Re: [PATCH 3/3] xfs: abort fstrim if kernel is suspending

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Sep 21, 2023 at 11:39:45AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> A recent ext4 patch posting from Jan Kara reminded me of a
> discussion a year ago about fstrim in progress preventing kernels
> from suspending. The fix is simple, we should do the same for XFS.
> 
> This removes the -ERESTARTSYS error return from this code, replacing
> it with either the last error seen or the number of blocks
> successfully trimmed up to the point where we detected the stop
> condition.

Kinda weird, actually, that FITRIM can do some discard work, hit an
error, and return the error without updating userspace about the work
that it /did/ get done.  Short writes work like that, so why not this?

Oh.  This is one of those syscalls that has no manpage, nor a
description in Documentation/, so the behavior of this is entirely
defined by calling code interpretation.  Yeah, not doing that today.

> References: https://bugzilla.kernel.org/show_bug.cgi?id=216322
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_discard.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
> index f16b254b5eaa..d5787991bb5b 100644
> --- a/fs/xfs/xfs_discard.c
> +++ b/fs/xfs/xfs_discard.c
> @@ -283,6 +283,12 @@ xfs_trim_gather_extents(
>  	return error;
>  }
>  
> +static bool
> +xfs_trim_should_stop(void)
> +{
> +	return fatal_signal_pending(current) || freezing(current);
> +}
> +
>  /*
>   * Iterate the free list gathering extents and discarding them. We need a cursor
>   * for the repeated iteration of gather/discard loop, so use the longest extent
> @@ -336,10 +342,9 @@ xfs_trim_extents(
>  		if (error)
>  			break;
>  
> -		if (fatal_signal_pending(current)) {
> -			error = -ERESTARTSYS;
> +		if (xfs_trim_should_stop())
>  			break;
> -		}
> +
>  	} while (tcur.ar_blockcount != 0);
>  
>  	return error;
> @@ -408,12 +413,12 @@ xfs_ioc_trim(
>  	for_each_perag_range(mp, agno, xfs_daddr_to_agno(mp, end), pag) {
>  		error = xfs_trim_extents(pag, start, end, minlen,
>  					  &blocks_trimmed);
> -		if (error) {
> +		if (error)
>  			last_error = error;
> -			if (error == -ERESTARTSYS) {
> -				xfs_perag_rele(pag);
> -				break;
> -			}
> +
> +		if (xfs_trim_should_stop()) {
> +			xfs_perag_rele(pag);
> +			break;
>  		}
>  	}
>  
> -- 
> 2.40.1
>