Re: [PATCH v3 11/17] xfs: use delete helper for items expected to be in AIL

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Apr 29, 2020 at 01:21:47PM -0400, Brian Foster wrote:
> Various intent log items call xfs_trans_ail_remove() with a log I/O
> error shutdown type, but this helper historically checks whether an
> item is in the AIL before calling xfs_trans_ail_delete(). This means
> the shutdown check is essentially a no-op for users of
> xfs_trans_ail_remove().
> 
> It is possible that some items might not be AIL resident when the
> AIL remove attempt occurs, but this should be isolated to cases
> where the filesystem has already shutdown. For example, this
> includes abort of the transaction committing the intent and I/O
> error of the iclog buffer committing the intent to the log.
> Therefore, update these callsites to use xfs_trans_ail_delete() to
> provide AIL state validation for the common path of items being
> released and removed when associated done items commit to the
> physical log.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Seems pretty straightforward
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_bmap_item.c     | 2 +-
>  fs/xfs/xfs_extfree_item.c  | 2 +-
>  fs/xfs/xfs_refcount_item.c | 2 +-
>  fs/xfs/xfs_rmap_item.c     | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index ee6f4229cebc..909221a4a8ab 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -51,7 +51,7 @@ xfs_bui_release(
>  {
>  	ASSERT(atomic_read(&buip->bui_refcount) > 0);
>  	if (atomic_dec_and_test(&buip->bui_refcount)) {
> -		xfs_trans_ail_remove(&buip->bui_item, SHUTDOWN_LOG_IO_ERROR);
> +		xfs_trans_ail_delete(&buip->bui_item, SHUTDOWN_LOG_IO_ERROR);
>  		xfs_bui_item_free(buip);
>  	}
>  }
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 6ea847f6e298..cd98eba24884 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -55,7 +55,7 @@ xfs_efi_release(
>  {
>  	ASSERT(atomic_read(&efip->efi_refcount) > 0);
>  	if (atomic_dec_and_test(&efip->efi_refcount)) {
> -		xfs_trans_ail_remove(&efip->efi_item, SHUTDOWN_LOG_IO_ERROR);
> +		xfs_trans_ail_delete(&efip->efi_item, SHUTDOWN_LOG_IO_ERROR);
>  		xfs_efi_item_free(efip);
>  	}
>  }
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 8eeed73928cd..712939a015a9 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -50,7 +50,7 @@ xfs_cui_release(
>  {
>  	ASSERT(atomic_read(&cuip->cui_refcount) > 0);
>  	if (atomic_dec_and_test(&cuip->cui_refcount)) {
> -		xfs_trans_ail_remove(&cuip->cui_item, SHUTDOWN_LOG_IO_ERROR);
> +		xfs_trans_ail_delete(&cuip->cui_item, SHUTDOWN_LOG_IO_ERROR);
>  		xfs_cui_item_free(cuip);
>  	}
>  }
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 4911b68f95dd..ff949b32c051 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -50,7 +50,7 @@ xfs_rui_release(
>  {
>  	ASSERT(atomic_read(&ruip->rui_refcount) > 0);
>  	if (atomic_dec_and_test(&ruip->rui_refcount)) {
> -		xfs_trans_ail_remove(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR);
> +		xfs_trans_ail_delete(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR);
>  		xfs_rui_item_free(ruip);
>  	}
>  }
> -- 
> 2.21.1
>