Re: [PATCH 15/30] xfs: move xfs_clear_li_failed out of xfs_ail_delete_one()

From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 15/30] xfs: move xfs_clear_li_failed out of xfs_ail_delete_one()
Date: Tue, 2 Jun 2020 13:47:44 -0700	[thread overview]
Message-ID: <20200602204744.GL8230@magnolia> (raw)
In-Reply-To: <20200601214251.4167140-16-david@fromorbit.com>

On Tue, Jun 02, 2020 at 07:42:36AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> xfs_ail_delete_one() is called directly from dquot and inode IO
> completion, as well as from the generic xfs_trans_ail_delete()
> function. Inodes are about to have their own failure handling, and
> dquots will in future, too. Pull the clearing of the LI_FAILED flag
> up into the callers so we can customise the code appropriately.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_dquot.c      | 6 +-----
>  fs/xfs/xfs_inode_item.c | 3 +--
>  fs/xfs/xfs_trans_ail.c  | 2 +-
>  3 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index d5984a926d1d0..76353c9a723ee 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -1070,16 +1070,12 @@ xfs_qm_dqflush_done(
>  	     test_bit(XFS_LI_FAILED, &lip->li_flags))) {
>  
>  		spin_lock(&ailp->ail_lock);
> +		xfs_clear_li_failed(lip);
>  		if (lip->li_lsn == qip->qli_flush_lsn) {
>  			/* xfs_ail_update_finish() drops the AIL lock */
>  			tail_lsn = xfs_ail_delete_one(ailp, lip);
>  			xfs_ail_update_finish(ailp, tail_lsn);
>  		} else {
> -			/*
> -			 * Clear the failed state since we are about to drop the
> -			 * flush lock
> -			 */
> -			xfs_clear_li_failed(lip);
>  			spin_unlock(&ailp->ail_lock);
>  		}
>  	}
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 86c783dec2bac..0ba75764a8dc5 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -690,12 +690,11 @@ xfs_iflush_done(
>  		/* this is an opencoded batch version of xfs_trans_ail_delete */
>  		spin_lock(&ailp->ail_lock);
>  		list_for_each_entry(lip, &tmp, li_bio_list) {
> +			xfs_clear_li_failed(lip);
>  			if (lip->li_lsn == INODE_ITEM(lip)->ili_flush_lsn) {
>  				xfs_lsn_t lsn = xfs_ail_delete_one(ailp, lip);
>  				if (!tail_lsn && lsn)
>  					tail_lsn = lsn;
> -			} else {
> -				xfs_clear_li_failed(lip);
>  			}
>  		}
>  		xfs_ail_update_finish(ailp, tail_lsn);
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index ac5019361a139..ac33f6393f99c 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -843,7 +843,6 @@ xfs_ail_delete_one(
>  
>  	trace_xfs_ail_delete(lip, mlip->li_lsn, lip->li_lsn);
>  	xfs_ail_delete(ailp, lip);
> -	xfs_clear_li_failed(lip);
>  	clear_bit(XFS_LI_IN_AIL, &lip->li_flags);
>  	lip->li_lsn = 0;
>  
> @@ -874,6 +873,7 @@ xfs_trans_ail_delete(
>  	}
>  
>  	/* xfs_ail_update_finish() drops the AIL lock */
> +	xfs_clear_li_failed(lip);
>  	tail_lsn = xfs_ail_delete_one(ailp, lip);
>  	xfs_ail_update_finish(ailp, tail_lsn);
>  }
> -- 
> 2.26.2.761.g0e0b3e54be
> 