[PATCH 02/14] xfs: clean up log space grant functions

[Dave Chinner <david@fromorbit.com>]

From: Dave Chinner <dchinner@redhat.com>

xlog_grant_log_space and xlog_regrant_log_write_space both have very
similar structure. Both have a "wait on non-empty queue" section at
the start, followed by a "wait for space" loop of which the contents
are almost identical to the initial non-empty queue section.

In both cases, the non-empty queue wait can be folded into the wait
for space loop, simply by entering the loop if the queue is not
empty and the current ticket is not on the queue. If we trigger the
non-empty queue case, we always add ourselves to the queue, and
hence the second and subsequent loops are always driven by the "wait
for space" test.

IOWs, both wait conditions can be folded into the one loop, removing
a bunch of duplicated code and making it simpler to modify in
future patches.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/linux-2.6/xfs_trace.h |   12 +--
 fs/xfs/xfs_log.c             |  171 +++++++++++++++---------------------------
 2 files changed, 64 insertions(+), 119 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_trace.h b/fs/xfs/linux-2.6/xfs_trace.h
index f400668..0884f93 100644
--- a/fs/xfs/linux-2.6/xfs_trace.h
+++ b/fs/xfs/linux-2.6/xfs_trace.h
@@ -831,17 +831,13 @@ DEFINE_LOGGRANT_EVENT(xfs_log_umount_write);
 DEFINE_LOGGRANT_EVENT(xfs_log_grant_enter);
 DEFINE_LOGGRANT_EVENT(xfs_log_grant_exit);
 DEFINE_LOGGRANT_EVENT(xfs_log_grant_error);
-DEFINE_LOGGRANT_EVENT(xfs_log_grant_sleep1);
-DEFINE_LOGGRANT_EVENT(xfs_log_grant_wake1);
-DEFINE_LOGGRANT_EVENT(xfs_log_grant_sleep2);
-DEFINE_LOGGRANT_EVENT(xfs_log_grant_wake2);
+DEFINE_LOGGRANT_EVENT(xfs_log_grant_sleep);
+DEFINE_LOGGRANT_EVENT(xfs_log_grant_wake);
 DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_enter);
 DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_exit);
 DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_error);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_sleep1);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_wake1);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_sleep2);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_wake2);
+DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_sleep);
+DEFINE_LOGGRANT_EVENT(xfs_log_regrant_write_wake);
 DEFINE_LOGGRANT_EVENT(xfs_log_regrant_reserve_enter);
 DEFINE_LOGGRANT_EVENT(xfs_log_regrant_reserve_exit);
 DEFINE_LOGGRANT_EVENT(xfs_log_regrant_reserve_sub);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 1b82735..399e559 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -68,14 +68,14 @@ STATIC void xlog_state_switch_iclogs(xlog_t		*log,
 STATIC void xlog_state_want_sync(xlog_t	*log, xlog_in_core_t *iclog);
 
 /* local functions to manipulate grant head */
-STATIC int  xlog_grant_log_space(xlog_t		*log,
-				 xlog_ticket_t	*xtic);
+STATIC int  xlog_grant_log_space(struct log		*log,
+				 struct xlog_ticket	*xtic);
 STATIC void xlog_grant_push_ail(xfs_mount_t	*mp,
 				int		need_bytes);
 STATIC void xlog_regrant_reserve_log_space(xlog_t	 *log,
 					   xlog_ticket_t *ticket);
-STATIC int xlog_regrant_write_log_space(xlog_t		*log,
-					 xlog_ticket_t  *ticket);
+STATIC int xlog_regrant_write_log_space(struct log		*log,
+					struct xlog_ticket	*xtic);
 STATIC void xlog_ungrant_log_space(xlog_t	 *log,
 				   xlog_ticket_t *ticket);
 
@@ -2498,52 +2498,27 @@ restart:
  * the needed reservation is satisfied.
  */
 STATIC int
-xlog_grant_log_space(xlog_t	   *log,
-		     xlog_ticket_t *tic)
+xlog_grant_log_space(
+	struct log		*log,
+	struct xlog_ticket	*tic)
 {
-	int		 free_bytes;
-	int		 need_bytes;
+	int			 free_bytes;
+	int			 need_bytes;
 #ifdef DEBUG
-	xfs_lsn_t	 tail_lsn;
-#endif
+	xfs_lsn_t		 tail_lsn;
 
-
-#ifdef DEBUG
 	if (log->l_flags & XLOG_ACTIVE_RECOVERY)
 		panic("grant Recovery problem");
 #endif
 
 	/* Is there space or do we need to sleep? */
 	spin_lock(&log->l_grant_lock);
-
 	trace_xfs_log_grant_enter(log, tic);
+	ASSERT(list_empty(&tic->t_queue));
 
-	/* something is already sleeping; insert new transaction at end */
-	if (!list_empty(&log->l_reserveq)) {
-		list_add_tail(&tic->t_queue, &log->l_reserveq);
-
-		trace_xfs_log_grant_sleep1(log, tic);
-
-		/*
-		 * Gotta check this before going to sleep, while we're
-		 * holding the grant lock.
-		 */
-		if (XLOG_FORCED_SHUTDOWN(log))
-			goto error_return;
-
-		XFS_STATS_INC(xs_sleep_logspace);
-		sv_wait(&tic->t_wait, PINOD|PLTWAIT, &log->l_grant_lock, s);
-		/*
-		 * If we got an error, and the filesystem is shutting down,
-		 * we'll catch it down below. So just continue...
-		 */
-		trace_xfs_log_grant_wake1(log, tic);
-		spin_lock(&log->l_grant_lock);
-	}
+	need_bytes = tic->t_unit_res;
 	if (tic->t_flags & XFS_LOG_PERM_RESERV)
-		need_bytes = tic->t_unit_res*tic->t_ocnt;
-	else
-		need_bytes = tic->t_unit_res;
+		need_bytes *= tic->t_ocnt;
 
 redo:
 	if (XLOG_FORCED_SHUTDOWN(log))
@@ -2551,25 +2526,25 @@ redo:
 
 	free_bytes = xlog_space_left(log, log->l_grant_reserve_cycle,
 				     log->l_grant_reserve_bytes);
-	if (free_bytes < need_bytes) {
+	/*
+	 * If there is not enough space or there is queued waiter and we
+	 * are not already on the queue, we need to wait.
+	 */
+	if (free_bytes < need_bytes ||
+	    (!list_empty(&log->l_reserveq) && list_empty(&tic->t_queue))) {
 		if (list_empty(&tic->t_queue))
 			list_add_tail(&tic->t_queue, &log->l_reserveq);
 
-		trace_xfs_log_grant_sleep2(log, tic);
-
 		spin_unlock(&log->l_grant_lock);
 		xlog_grant_push_ail(log->l_mp, need_bytes);
 		spin_lock(&log->l_grant_lock);
 
 		XFS_STATS_INC(xs_sleep_logspace);
+		trace_xfs_log_grant_sleep(log, tic);
 		sv_wait(&tic->t_wait, PINOD|PLTWAIT, &log->l_grant_lock, s);
 
 		spin_lock(&log->l_grant_lock);
-		if (XLOG_FORCED_SHUTDOWN(log))
-			goto error_return;
-
-		trace_xfs_log_grant_wake2(log, tic);
-
+		trace_xfs_log_grant_wake(log, tic);
 		goto redo;
 	}
 
@@ -2608,21 +2583,21 @@ redo:
 	tic->t_cnt = 0; /* ungrant will give back unit_res * t_cnt. */
 	spin_unlock(&log->l_grant_lock);
 	return XFS_ERROR(EIO);
-}	/* xlog_grant_log_space */
+}
 
 
 /*
  * Replenish the byte reservation required by moving the grant write head.
- *
- *
  */
 STATIC int
-xlog_regrant_write_log_space(xlog_t	   *log,
-			     xlog_ticket_t *tic)
+xlog_regrant_write_log_space(
+	struct log		*log,
+	struct xlog_ticket	*tic)
 {
-	int		free_bytes, need_bytes;
+	int			free_bytes;
+	int			need_bytes;
 #ifdef DEBUG
-	xfs_lsn_t	tail_lsn;
+	xfs_lsn_t		tail_lsn;
 #endif
 
 	tic->t_curr_res = tic->t_unit_res;
@@ -2637,81 +2612,55 @@ xlog_regrant_write_log_space(xlog_t	   *log,
 #endif
 
 	spin_lock(&log->l_grant_lock);
-
 	trace_xfs_log_regrant_write_enter(log, tic);
+	ASSERT(list_empty(&tic->t_queue));
 
-	if (XLOG_FORCED_SHUTDOWN(log))
-		goto error_return;
-
-	/* If there are other waiters on the queue then give them a
-	 * chance at logspace before us. Wake up the first waiters,
-	 * if we do not wake up all the waiters then go to sleep waiting
-	 * for more free space, otherwise try to get some space for
-	 * this transaction.
-	 */
 	need_bytes = tic->t_unit_res;
-	if (!list_empty(&log->l_writeq)) {
-		struct xlog_ticket *ntic;
-		free_bytes = xlog_space_left(log, log->l_grant_write_cycle,
-					     log->l_grant_write_bytes);
-		list_for_each_entry(ntic, &log->l_writeq, t_queue) {
-			ASSERT(ntic->t_flags & XLOG_TIC_PERM_RESERV);
-
-			if (free_bytes < ntic->t_unit_res)
-				break;
-			free_bytes -= ntic->t_unit_res;
-			sv_signal(&ntic->t_wait);
-		}
-
-		if (ntic != list_first_entry(&log->l_writeq,
-						struct xlog_ticket, t_queue)) {
-			if (list_empty(&tic->t_queue))
-				list_add_tail(&tic->t_queue, &log->l_writeq);
-
-			trace_xfs_log_regrant_write_sleep1(log, tic);
-
-			spin_unlock(&log->l_grant_lock);
-			xlog_grant_push_ail(log->l_mp, need_bytes);
-			spin_lock(&log->l_grant_lock);
-
-			XFS_STATS_INC(xs_sleep_logspace);
-			sv_wait(&tic->t_wait, PINOD|PLTWAIT,
-				&log->l_grant_lock, s);
-
-			/* If we're shutting down, this tic is already
-			 * off the queue */
-			spin_lock(&log->l_grant_lock);
-			if (XLOG_FORCED_SHUTDOWN(log))
-				goto error_return;
-
-			trace_xfs_log_regrant_write_wake1(log, tic);
-		}
-	}
-
 redo:
 	if (XLOG_FORCED_SHUTDOWN(log))
 		goto error_return;
 
 	free_bytes = xlog_space_left(log, log->l_grant_write_cycle,
 				     log->l_grant_write_bytes);
-	if (free_bytes < need_bytes) {
-		if (list_empty(&tic->t_queue))
+	/*
+	 * If there is not enough space or there is queued waiter and we
+	 * are not already on the queue, we need to wait.
+	 */
+	if (free_bytes < need_bytes ||
+	    (!list_empty(&log->l_writeq) && list_empty(&tic->t_queue))) {
+		if (list_empty(&tic->t_queue)) {
+			/*
+			 * give existing waiters a chance at logspace before
+			 * us.  If we woke all the waiters, then immediately
+			 * retry the space, otherwise sleep first.
+			 */
+			struct xlog_ticket	*ntic;
+			int			woke_all = 1;
+			list_for_each_entry(ntic, &log->l_writeq, t_queue) {
+				ASSERT(ntic->t_flags & XLOG_TIC_PERM_RESERV);
+
+				if (free_bytes < ntic->t_unit_res) {
+					woke_all = 0;
+					break;
+				}
+				free_bytes -= ntic->t_unit_res;
+				sv_signal(&ntic->t_wait);
+			}
 			list_add_tail(&tic->t_queue, &log->l_writeq);
+			if (woke_all)
+				goto redo;
+		}
+
 		spin_unlock(&log->l_grant_lock);
 		xlog_grant_push_ail(log->l_mp, need_bytes);
 		spin_lock(&log->l_grant_lock);
 
 		XFS_STATS_INC(xs_sleep_logspace);
-		trace_xfs_log_regrant_write_sleep2(log, tic);
-
+		trace_xfs_log_regrant_write_sleep(log, tic);
 		sv_wait(&tic->t_wait, PINOD|PLTWAIT, &log->l_grant_lock, s);
 
-		/* If we're shutting down, this tic is already off the queue */
 		spin_lock(&log->l_grant_lock);
-		if (XLOG_FORCED_SHUTDOWN(log))
-			goto error_return;
-
-		trace_xfs_log_regrant_write_wake2(log, tic);
+		trace_xfs_log_regrant_write_wake(log, tic);
 		goto redo;
 	}
 
@@ -2747,7 +2696,7 @@ redo:
 	tic->t_cnt = 0; /* ungrant will give back unit_res * t_cnt. */
 	spin_unlock(&log->l_grant_lock);
 	return XFS_ERROR(EIO);
-}	/* xlog_regrant_write_log_space */
+}
 
 
 /* The first cnt-1 times through here we don't need to
-- 
1.7.2.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs