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

Darrick J. Wong
2024-02-23T17:26:47Z

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
>
>