xfs
[Top] [All Lists]

[PATCH 1/9] xfs: fix EFI transaction cancellation.

To: xfs@xxxxxxxxxxx
Subject: [PATCH 1/9] xfs: fix EFI transaction cancellation.
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 13 Dec 2010 15:32:15 +1100
In-reply-to: <1292214743-18073-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1292214743-18073-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

XFS_EFI_CANCELED has not been set in the code base since
xfs_efi_cancel() was removed back in 2006 by commit
065d312e15902976d256ddaf396a7950ec0350a8 ("[XFS] Remove unused
iop_abort log item operation), and even then xfs_efi_cancel() was
never called. I haven't tracked it back further than that (beyond
git history), but it indicates that the handling of EFIs in
cancelled transactions has been broken for a long time.

Basically, when we get an IOP_UNPIN(lip, 1); call from
xfs_trans_uncommit() (i.e. remove == 1), if we don't free the log
item descriptor we leak it. Fix the behviour to be correct and kill
the XFS_EFI_CANCELED flag.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/xfs_extfree_item.c |   20 +++++++++-----------
 fs/xfs/xfs_extfree_item.h |    1 -
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index a55e687..5997efa 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -99,10 +99,11 @@ xfs_efi_item_pin(
 }
 
 /*
- * While EFIs cannot really be pinned, the unpin operation is the
- * last place at which the EFI is manipulated during a transaction.
- * Here we coordinate with xfs_efi_cancel() to determine who gets to
- * free the EFI.
+ * While EFIs cannot really be pinned, the unpin operation is the last place at
+ * which the EFI is manipulated during a transaction.  If we are being asked to
+ * remove the EFI it's because the transaction has been cancelled and by
+ * definition that means the EFI cannot be in the AIL so remove it from the
+ * transaction and free it.
  */
 STATIC void
 xfs_efi_item_unpin(
@@ -113,17 +114,14 @@ xfs_efi_item_unpin(
        struct xfs_ail          *ailp = lip->li_ailp;
 
        spin_lock(&ailp->xa_lock);
-       if (efip->efi_flags & XFS_EFI_CANCELED) {
-               if (remove)
-                       xfs_trans_del_item(lip);
-
-               /* xfs_trans_ail_delete() drops the AIL lock. */
-               xfs_trans_ail_delete(ailp, lip);
+       if (remove) {
+               ASSERT(!(lip->li_flags & XFS_LI_IN_AIL));
+               xfs_trans_del_item(lip);
                xfs_efi_item_free(efip);
        } else {
                efip->efi_flags |= XFS_EFI_COMMITTED;
-               spin_unlock(&ailp->xa_lock);
        }
+       spin_unlock(&ailp->xa_lock);
 }
 
 /*
diff --git a/fs/xfs/xfs_extfree_item.h b/fs/xfs/xfs_extfree_item.h
index 0d22c56..f7834ec 100644
--- a/fs/xfs/xfs_extfree_item.h
+++ b/fs/xfs/xfs_extfree_item.h
@@ -115,7 +115,6 @@ typedef struct xfs_efd_log_format_64 {
  */
 #define        XFS_EFI_RECOVERED       0x1
 #define        XFS_EFI_COMMITTED       0x2
-#define        XFS_EFI_CANCELED        0x4
 
 /*
  * This is the "extent free intention" log item.  It is used
-- 
1.7.2.3

<Prev in Thread] Current Thread [Next in Thread>