[PATCH 03/11] xfs: fix efi/efd error handling to avoid fs shutdown hangs

[Dave Chinner]

On Thu, Aug 06, 2015 at 01:44:24PM -0400, Brian Foster wrote:
....
> Update the EFI/EFD item handling code to use a more straightforward and
> reliable approach to error handling. If an error occurs after the EFI
> transaction is committed and before the EFD is constructed, release the
> EFI explicitly from xfs_bmap_finish(). If the EFI transaction is
> cancelled, release the EFI in the unlock handler.
> 
> Once the EFD is constructed, it is responsible for releasing the EFI
> under any circumstances (including whether the EFI item aborts due to
> log I/O error).

Can you document these rules in a comment at the top of
fs/xfs/xfs_extfree_item.c?

> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 42c9b05..aceb54f 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -61,9 +61,15 @@ __xfs_efi_release(
>  
>  	if (atomic_dec_and_test(&efip->efi_refcount)) {
>  		spin_lock(&ailp->xa_lock);
> -		/* xfs_trans_ail_delete() drops the AIL lock. */
> -		xfs_trans_ail_delete(ailp, &efip->efi_item,
> -				     SHUTDOWN_LOG_IO_ERROR);
> +		/*
> +		 * We don't know whether the EFI made it to the AIL. Remove it
> +		 * if so. Note that xfs_trans_ail_delete() drops the AIL lock.
> +		 */
> +		if (efip->efi_item.li_flags & XFS_LI_IN_AIL)
> +			xfs_trans_ail_delete(ailp, &efip->efi_item,
> +					     SHUTDOWN_LOG_IO_ERROR);
> +		else
> +			spin_unlock(&ailp->xa_lock);
>  		xfs_efi_item_free(efip);
>  	}

We now have a lot of this pattern:

	spin_lock(&ailp->xa_lock);
	if (lip->li_flags & XFS_LI_IN_AIL)
		xfs_trans_ail_delete(ailp, lip, shutdown_reason);
	else
		spin_unlock(&ailp->xa_lock);

occurring in the code. Can you look into adding a helper function
for this, say xfs_trans_ail_remove()? (separate patch, of course!)

The rest of the patch looks fine - getting rid of almost all of
those aborted flag special cases is a big win :)

Cheers,

Dave.
-- 
Dave Chinner
david at fromorbit.com