Re: [PATCH 1/2] xfs: flush inodegc before swapon

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

On Wed, Feb 05, 2025 at 05:28:00PM +0100, Christoph Hellwig wrote:
> Fix the brand new xfstest that tries to swapon on a recently unshared
> file and use the chance to document the other bit of magic in this
> function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Cc: <stable@vger.kernel.org> # v5.19
Fixes: 5e672cd69f0a53 ("xfs: introduce xfs_inodegc_push()")

Perhaps?  This isn't really a hard brokenness, but it could surprise a
user somewhere.

> ---
>  fs/xfs/xfs_aops.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 69b8c2d1937d..c792297aa0a3 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -21,6 +21,7 @@
>  #include "xfs_error.h"
>  #include "xfs_zone_alloc.h"
>  #include "xfs_rtgroup.h"
> +#include "xfs_icache.h"
>  
>  struct xfs_writepage_ctx {
>  	struct iomap_writepage_ctx ctx;
> @@ -685,7 +686,22 @@ xfs_iomap_swapfile_activate(
>  	struct file			*swap_file,
>  	sector_t			*span)
>  {
> -	sis->bdev = xfs_inode_buftarg(XFS_I(file_inode(swap_file)))->bt_bdev;
> +	struct xfs_inode		*ip = XFS_I(file_inode(swap_file));
> +
> +	/*
> +	 * Ensure inode GC has finished to remove unmapped extents, as the
> +	 * reflink bit is only cleared once all previously shared extents
> +	 * are unmapped.  Otherwise swapon could incorrectly fail on a
> +	 * very recently unshare file.

                         unshared

> +	 */
> +	xfs_inodegc_flush(ip->i_mount);
> +
> +	/*
> +	 * Direct the swap code to the correct block device when this file
> +	 * sits on the RT device.
> +	 */
> +	sis->bdev = xfs_inode_buftarg(ip)->bt_bdev;

With all that amended,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +
>  	return iomap_swapfile_activate(sis, swap_file, span,
>  			&xfs_read_iomap_ops);
>  }
> -- 
> 2.45.2
> 
>