xfs: ensure EFD trans aborts on log recovery extent free failure

Log recovery attempts to free extents with leftover EFIs in the AIL
after initial processing. If the extent free fails (e.g., due to
unrelated fs corruption), the transaction is cancelled, though it
might not be dirtied at the time. If this is the case, the EFD does
not abort and thus does not release the EFI. This can lead to hangs
as the EFI pins the AIL.

Update xlog_recover_process_efi() to log the EFD in the transaction
before xfs_free_extent() errors are handled to ensure the
transaction is dirty, aborts the EFD and releases the EFI on error.
Since this is a requirement for EFD processing (and consistent with
xfs_bmap_finish()), update the EFD logging helper to do the extent
free and unconditionally log the EFD. This encodes the required EFD
logging behavior into the helper and reduces the likelihood of
errors down the road.

[dchinner: re-add xfs_alloc.h to xfs_log_recover.c to fix build
 failure.]

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index fa65f67..73aab0d 100644 (file)
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -112,24 +112,17 @@ xfs_bmap_finish(
                return error;
        }
 
+       /*
+        * Get an EFD and free each extent in the list, logging to the EFD in
+        * the process. The remaining bmap free list is cleaned up by the caller
+        * on error.
+        */
        efd = xfs_trans_get_efd(*tp, efi, flist->xbf_count);
        for (free = flist->xbf_first; free != NULL; free = next) {
                next = free->xbfi_next;
 
-               /*
-                * Free the extent and log the EFD to dirty the transaction
-                * before handling errors. This ensures that the transaction is
-                * aborted, which:
-                *
-                * 1.) releases the EFI and frees the EFD
-                * 2.) shuts down the filesystem
-                *
-                * The bmap free list is cleaned up at a higher level.
-                */
-               error = xfs_free_extent(*tp, free->xbfi_startblock,
-                                       free->xbfi_blockcount);
-               xfs_trans_log_efd_extent(*tp, efd, free->xbfi_startblock,
-                                        free->xbfi_blockcount);
+               error = xfs_trans_free_extent(*tp, efd, free->xbfi_startblock,
+                                             free->xbfi_blockcount);
                if (error)
                        return error;
 

diff --cc fs/xfs/xfs_log_recover.c
Simple merge

diff --cc fs/xfs/xfs_trans.h
Simple merge

diff --cc fs/xfs/xfs_trans_extfree.c
Simple merge