[PATCH] xfs: order log tail accounting before grant head returns

[PATCH] xfs: order log tail accounting before grant head returns

[Cen Zhang]

The log reservation fast path computes available space from l_tail_space
and a grant head without taking the AIL lock.  CIL completion moves a
committed checkpoint's consumed bytes from grant-head accounting to
l_tail_space by advancing the AIL head/tail accounting before subtracting
the same amount from both grant heads.

That transfer must not be observed as a smaller grant head with the old
l_tail_space value.  Such a mixed snapshot makes the free-space calculation
spuriously larger by the transferred amount and can allow a reservation
that should have waited.

The existing smp_wmb()/smp_rmb() comments describe that intent, but the
reader samples l_tail_space after the read barrier and before the grant
head.  In addition, the writer orders a normal store against unordered
atomic64_sub() operations, for which the atomic documentation specifies
smp_mb__before_atomic().

Put the write-side barrier immediately before the grant-head atomic
subtractions.  Sample the grant head before l_tail_space on the read side.
This makes observing returned grant space imply visibility of the matching
tail-space accounting update.

Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/xfs/xfs_log.c     | 21 +++++++++++++--------
 fs/xfs/xfs_log_cil.c |  1 -
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index f807f8f4f7058..de56c707027d9 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -113,17 +113,21 @@ xlog_grant_return_space(
 {
 	int64_t		diff = xlog_lsn_sub(log, new_head, old_head);
 
+	/*
+	 * Publish the tail-space update before dropping the grant heads so
+	 * readers cannot observe returned grant space without the matching
+	 * tail-space accounting.
+	 */
+	smp_mb__before_atomic();
 	xlog_grant_sub_space(&log->l_reserve_head, diff);
 	xlog_grant_sub_space(&log->l_write_head, diff);
 }
 
 /*
  * Return the space in the log between the tail and the head.  In the case where
- * we have overrun available reservation space, return 0. The memory barrier
- * pairs with the smp_wmb() in xlog_cil_ail_insert() to ensure that grant head
- * vs tail space updates are seen in the correct order and hence avoid
- * transients as space is transferred from the grant heads to the AIL on commit
- * completion.
+ * we have overrun available reservation space, return 0.  Sample the grant
+ * head before l_tail_space so that seeing returned grant space implies seeing
+ * the matching tail-space update from xlog_grant_return_space().
  */
 static uint64_t
 xlog_grant_space_left(
@@ -131,10 +135,11 @@ xlog_grant_space_left(
 	struct xlog_grant_head	*head)
 {
 	int64_t			free_bytes;
+	s64			grant;
 
-	smp_rmb();	/* paired with smp_wmb in xlog_cil_ail_insert() */
-	free_bytes = log->l_logsize - READ_ONCE(log->l_tail_space) -
-			atomic64_read(&head->grant);
+	grant = atomic64_read(&head->grant);
+	smp_rmb();	/* paired with smp_mb__before_atomic in xlog_grant_return_space */
+	free_bytes = log->l_logsize - READ_ONCE(log->l_tail_space) - grant;
 	if (free_bytes > 0)
 		return free_bytes;
 	return 0;
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index edc368938f305..ea0efabdc9990 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -876,7 +876,6 @@ xlog_cil_ail_insert(
 	 * available on return, only for it to disappear again immediately as
 	 * the AIL head update accounts in the log tail space.
 	 */
-	smp_wmb();	/* paired with smp_rmb in xlog_grant_space_left */
 	xlog_grant_return_space(ailp->ail_log, old_head, ailp->ail_head_lsn);
 
 	/* unpin all the log items */