Re: [PATCH 10/10] xfs: reinstate delalloc for RT inodes (if sb_rextsize == 1)

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

On Fri, Feb 23, 2024 at 08:15:06AM +0100, Christoph Hellwig wrote:
> Commit aff3a9edb708 ("xfs: Use preallocation for inodes with extsz
> hints") disabled delayed allocation for all inodes with extent size
> hints due a data exposure problem.  It turns out we fixed this data
> exposure problem since by always creating unwritten extents for
> delalloc conversions due to more data exposure problems, but the
> writeback path doesn't actually support extent size hints when
> converting delalloc these days, which probably isn't a problem given
> that people using the hints know what they get.
> 
> However due to the way how xfs_get_extsz_hint is implemented, it
> always claims an extent size hint for RT inodes even if the RT
> extent size is a single FSB.  Due to that the above commit effectively
> disabled delalloc support for RT inodes.
> 
> Switch xfs_get_extsz_hint to return 0 for this case and work around
> that in a few places to reinstate delalloc support for RT inodes on
> file systems with an sb_rextsize of 1.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_inode.c   | 3 ++-
>  fs/xfs/xfs_iomap.c   | 2 --
>  fs/xfs/xfs_iops.c    | 2 +-
>  fs/xfs/xfs_rtalloc.c | 2 ++
>  4 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 37ec247edc1332..9e12278d1b62cd 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -60,7 +60,8 @@ xfs_get_extsz_hint(
>  		return 0;
>  	if ((ip->i_diflags & XFS_DIFLAG_EXTSIZE) && ip->i_extsize)
>  		return ip->i_extsize;
> -	if (XFS_IS_REALTIME_INODE(ip))
> +	if (XFS_IS_REALTIME_INODE(ip) &&
> +	    ip->i_mount->m_sb.sb_rextsize > 1)
>  		return ip->i_mount->m_sb.sb_rextsize;
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index e6abe56d1f1f23..aea4e29ebd6785 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -992,8 +992,6 @@ xfs_buffered_write_iomap_begin(
>  		return xfs_direct_write_iomap_begin(inode, offset, count,
>  				flags, iomap, srcmap);
>  
> -	ASSERT(!XFS_IS_REALTIME_INODE(ip));
> -
>  	error = xfs_qm_dqattach(ip);
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index be102fd49560dc..ca60ba060fd5c9 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -521,7 +521,7 @@ xfs_stat_blksize(
>  	 * always return the realtime extent size.
>  	 */
>  	if (XFS_IS_REALTIME_INODE(ip))
> -		return XFS_FSB_TO_B(mp, xfs_get_extsz_hint(ip));
> +		return XFS_FSB_TO_B(mp, xfs_get_extsz_hint(ip) ? : 1);
>  
>  	/*
>  	 * Allow large block sizes to be reported to userspace programs if the
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 2f85567f3d756b..9c7fba175b9025 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1340,6 +1340,8 @@ xfs_bmap_rtalloc(
>  	int			error;
>  
>  	align = xfs_get_extsz_hint(ap->ip);
> +	if (!align)
> +		align = 1;
>  retry:
>  	error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
>  					align, 1, ap->eof, 0,
> -- 
> 2.39.2
> 
>