Re: [PATCH 3/4] xfs: don't retry repairs harder when EAGAIN is returned

[Dave Chinner <david@fromorbit.com>]

On Sun, Oct 02, 2022 at 11:19:55AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Repair functions will not return EAGAIN -- if they were not able to
> obtain resources, they should return EDEADLOCK (like the rest of online
> fsck) to signal that we need to grab all the resources and try again.
> Hence we don't need to deal with this case except as a debugging
> assertion.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/scrub/repair.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
> index 92c661b98892..f6c4cb013346 100644
> --- a/fs/xfs/scrub/repair.c
> +++ b/fs/xfs/scrub/repair.c
> @@ -61,7 +61,6 @@ xrep_attempt(
>  		sc->flags |= XREP_ALREADY_FIXED;
>  		return -EAGAIN;
>  	case -EDEADLOCK:
> -	case -EAGAIN:
>  		/* Tell the caller to try again having grabbed all the locks. */
>  		if (!(sc->flags & XCHK_TRY_HARDER)) {
>  			sc->flags |= XCHK_TRY_HARDER;
> @@ -73,6 +72,10 @@ xrep_attempt(
>  		 * so report back to userspace.
>  		 */
>  		return -EFSCORRUPTED;
> +	case -EAGAIN:
> +		/* Repair functions should return EDEADLOCK, not EAGAIN. */
> +		ASSERT(0);
> +		fallthrough;
>  	default:
>  		return error;
>  	}

I'd do this rather than ASSERT(0) and fall through:

	default:
		ASSERT(error != -EAGAIN);
		return error;
	}

but that's just personal preference. Change it if you want, either
way works so:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com