xfs: unlock items before allowing the CIL to commit

When we commit a transaction using delayed logging, we need to
unlock the items in the transaciton before we unlock the CIL context
and allow it to be checkpointed. If we unlock them after we release
the CIl context lock, the CIL can checkpoint and complete before
we free the log items. This breaks stale buffer item unlock and
unpin processing as there is an implicit assumption that the unlock
will occur before the unpin.

Also, some log items need to store the LSN of the transaction commit
in the item (inodes and EFIs) and so can race with other transaction
completions if we don't prevent the CIL from checkpointing before
the unlock occurs.

Cc: <stable@kernel.org>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 31e4ea2d19acfc08f069813dffa367e6b2420973..ef8e7d9f445d535e5b9b74a378e05b37bd294449 100644 (file)
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -377,9 +377,23 @@ xfs_log_commit_cil(
        xfs_log_done(mp, tp->t_ticket, NULL, log_flags);
        xfs_trans_unreserve_and_mod_sb(tp);
 
+       /*
+        * Once all the items of the transaction have been copied to the CIL,
+        * the items can be unlocked and freed.
+        *
+        * This needs to be done before we drop the CIL context lock because we
+        * have to update state in the log items and unlock them before they go
+        * to disk. If we don't, then the CIL checkpoint can race with us and
+        * we can run checkpoint completion before we've updated and unlocked
+        * the log items. This affects (at least) processing of stale buffers,
+        * inodes and EFIs.
+        */
+       xfs_trans_free_items(tp, *commit_lsn, 0);
+
        /* check for background commit before unlock */
        if (log->l_cilp->xc_ctx->space_used > XLOG_CIL_SPACE_LIMIT(log))
                push = 1;
+
        up_read(&log->l_cilp->xc_ctx_lock);
 
        /*

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index fdca7416c754636a26dae9a312003932f7306d9f..1c47edaea0d28f4def851e87f664bd9339ac19b7 100644 (file)
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1167,7 +1167,7 @@ xfs_trans_del_item(
  * Unlock all of the items of a transaction and free all the descriptors
  * of that transaction.
  */
-STATIC void
+void
 xfs_trans_free_items(
        struct xfs_trans        *tp,
        xfs_lsn_t               commit_lsn,
@@ -1653,9 +1653,6 @@ xfs_trans_commit_cil(
                return error;
 
        current_restore_flags_nested(&tp->t_pflags, PF_FSTRANS);
-
-       /* xfs_trans_free_items() unlocks them first */
-       xfs_trans_free_items(tp, *commit_lsn, 0);
        xfs_trans_free(tp);
        return 0;
 }

diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
index e2d93d8ead7b68b9b6ab74b1869a3f8e10721981..62da86c90de53bb36b3fd2928d539ae9fc339478 100644 (file)
--- a/fs/xfs/xfs_trans_priv.h
+++ b/fs/xfs/xfs_trans_priv.h
@@ -25,7 +25,8 @@ struct xfs_trans;
 
 void   xfs_trans_add_item(struct xfs_trans *, struct xfs_log_item *);
 void   xfs_trans_del_item(struct xfs_log_item *);
-
+void   xfs_trans_free_items(struct xfs_trans *tp, xfs_lsn_t commit_lsn,
+                               int flags);
 void   xfs_trans_item_committed(struct xfs_log_item *lip,
                                xfs_lsn_t commit_lsn, int aborted);
 void   xfs_trans_unreserve_and_mod_sb(struct xfs_trans *tp);