xlog_recover_process_efi{s}() functions are completing the
second half of xfs_bmap_finish() which frees extents. If this
operation fails, the EFI will stay on the AIL and prevents
the xfs_ail_push all_sync() from completing and the mount will
fail to unmount.
Rather than have a special log recovery flag XFS_EFI_RECOVERED
to decrement the EFI/EFD counter, call the same decrement function
from the log recovery routine that is called then the EFI is added
to the AIL from a log write.
Remove all other unprocessed EFIs from the log recovery AIL
when one is discovered in error.
Signed-off-by: Mark Tinguely <tinguely@xxxxxxx>
---
v2 per Brian's feedback:
decrement the EFI counter again when removing EFI entries after the
entry with the error.
add comment in xlog_recover_process_efis() where we delete the EFIs.
v1:
Rewritten with suggestions from Dave.
Note: calling xfs_efi_item_unpin() seemed more explainatory than calling
the helper __xfs_efi_release().
fs/xfs/xfs_extfree_item.c | 9 +++------
fs/xfs/xfs_log_recover.c | 37 ++++++++++++++++++++++++-------------
fs/xfs/xfs_trans.h | 1 +
3 files changed, 28 insertions(+), 19 deletions(-)
Index: b/fs/xfs/xfs_extfree_item.c
===================================================================
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -134,9 +134,10 @@ xfs_efi_item_pin(
* 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. Otherwise coordinate with xfs_efi_release()
- * to determine who gets to free the EFI.
+ * to determine who gets to free the EFI. Call from log recovery of EFI
+ * entries so the EFD or error handling will remove the entry.
*/
-STATIC void
+void
xfs_efi_item_unpin(
struct xfs_log_item *lip,
int remove)
@@ -313,10 +314,6 @@ xfs_efi_release(xfs_efi_log_item_t *efip
{
ASSERT(atomic_read(&efip->efi_next_extent) >= nextents);
if (atomic_sub_and_test(nextents, &efip->efi_next_extent)) {
- /* recovery needs us to drop the EFI reference, too */
- if (test_bit(XFS_EFI_RECOVERED, &efip->efi_flags))
- __xfs_efi_release(efip);
-
__xfs_efi_release(efip);
/* efip may now have been freed, do not reference it again. */
}
Index: b/fs/xfs/xfs_log_recover.c
===================================================================
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3634,6 +3634,7 @@ xlog_recover_process_data(
/*
* Process an extent free intent item that was recovered from
* the log. We need to free the extents that it describes.
+ * The caller will release this and any following EFIs upon error.
*/
STATIC int
xlog_recover_process_efi(
@@ -3648,6 +3649,13 @@ xlog_recover_process_efi(
xfs_fsblock_t startblock_fsb;
ASSERT(!test_bit(XFS_EFI_RECOVERED, &efip->efi_flags));
+ set_bit(XFS_EFI_RECOVERED, &efip->efi_flags);
+
+ /*
+ * Decrement the EFI/EFD counter so the EFI is removed after
+ * processing the EFD or error handling in the caller.
+ */
+ xfs_efi_item_unpin(&efip->efi_item, 0);
/*
* First check the validity of the extents described by the
@@ -3662,12 +3670,6 @@ xlog_recover_process_efi(
(extp->ext_len == 0) ||
(startblock_fsb >= mp->m_sb.sb_dblocks) ||
(extp->ext_len >= mp->m_sb.sb_agblocks)) {
- /*
- * This will pull the EFI from the AIL and
- * free the memory associated with it.
- */
- set_bit(XFS_EFI_RECOVERED, &efip->efi_flags);
- xfs_efi_release(efip, efip->efi_format.efi_nextents);
return XFS_ERROR(EIO);
}
}
@@ -3687,7 +3689,6 @@ xlog_recover_process_efi(
extp->ext_len);
}
- set_bit(XFS_EFI_RECOVERED, &efip->efi_flags);
error = xfs_trans_commit(tp, 0);
return error;
@@ -3718,8 +3719,8 @@ STATIC int
xlog_recover_process_efis(
struct xlog *log)
{
- xfs_log_item_t *lip;
- xfs_efi_log_item_t *efip;
+ struct xfs_log_item *lip;
+ struct xfs_efi_log_item *efip;
int error = 0;
struct xfs_ail_cursor cur;
struct xfs_ail *ailp;
@@ -3750,13 +3751,23 @@ xlog_recover_process_efis(
}
spin_unlock(&ailp->xa_lock);
- error = xlog_recover_process_efi(log->l_mp, efip);
- spin_lock(&ailp->xa_lock);
+ if (error) {
+ /*
+ * An error happened while processing a previous
+ * EFI entry. Since the extents are freed in order,
+ * skip the remaining unprocessed EFIs. To do this
+ * the EFI entry counter must be decremented once
+ * here and the entry will be decremented and removed
+ * with the following xfs_efi_release().
+ */
+ xfs_efi_item_unpin(&efip->efi_item, 0);
+ } else
+ error = xlog_recover_process_efi(log->l_mp, efip);
if (error)
- goto out;
+ xfs_efi_release(efip, efip->efi_format.efi_nextents);
+ spin_lock(&ailp->xa_lock);
lip = xfs_trans_ail_cursor_next(ailp, &cur);
}
-out:
xfs_trans_ail_cursor_done(ailp, &cur);
spin_unlock(&ailp->xa_lock);
return error;
Index: b/fs/xfs/xfs_trans.h
===================================================================
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -216,6 +216,7 @@ void xfs_trans_ijoin(struct xfs_trans *
void xfs_trans_log_buf(xfs_trans_t *, struct xfs_buf *, uint, uint);
void xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
struct xfs_efi_log_item *xfs_trans_get_efi(xfs_trans_t *, uint);
+void xfs_efi_item_unpin(struct xfs_log_item *, int);
void xfs_efi_release(struct xfs_efi_log_item *, uint);
void xfs_trans_log_efi_extent(xfs_trans_t *,
struct xfs_efi_log_item *,
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
