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@xxxxxxxxxx>
Seems pretty straightforward
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--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
>
