On Tue, Jun 18, 2024 at 10:21:10PM +0800, Zhang Yi wrote:
> Changes since v5:
> - Drop all the code about zeroing out the whole allocation unitsize
> on truncate down in xfs_setattr_size() as Christoph suggested, let's
> just fix this issue for RT file by converting tail blocks to
> unwritten now, and we could think about forced aligned extent and
> atomic write later until it needs, so only pick patch 6 and 8 in
> previous version, do some minor git log changes.
This mostly makes sense, let's see how it fares with overnight fstests.
For now, this is a provisional
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> Changes since v4:
> - Drop the first patch in v4 "iomap: zeroing needs to be pagecache
> aware" since this series is not strongly depends on it, that patch
> still needs furtuer analyse and also should add to handle the case of
> a pending COW extent that extends over a data fork hole. This is a
> big job, so let's fix the exposure stale data issue and brings back
> the changes in iomap_write_end() first, don't block the ext4 buffered
> iomap conversion.
> - In patch 1, drop the 'ifndef rem_u64'.
> - In patch 4, factor out a helper xfs_setattr_truncate_data() to handle
> the zero out, update i_size, write back and drop pagecache on
> truncate.
> - In patch 5, switch to use xfs_inode_alloc_unitsize() in
> xfs_itruncate_extents_flags().
> - In patch 6, changes to reserve blocks for rtextsize > 1 realtime
> inodes on truncate down.
> - In patch 7, drop the unwritten convert threshold, always convert tail
> blocks to unwritten on truncate down realtime inodes.
> - Add patch 8 to bring back 'commit 943bc0882ceb ("iomap: don't
> increase i_size if it's not a write operation")'.
>
> Changes since v3:
> - Factor out a new helper to get the remainder in math64.h as Darrick
> suggested.
> - Adjust the truncating order to prevent too much redundant blocking
> writes as Dave suggested.
> - Improve to convert the tail extent to unwritten when truncating down
> an inode with large rtextsize as Darrick and Dave suggested.
>
> Since 'commit 943bc0882ceb ("iomap: don't increase i_size if it's not a
> write operation")' merged, Chandan reported a stale data exposure issue
> when running fstests generic/561 on xfs with realtime device [1]. This
> issue has been fix in 6.10 by revert this commit through commit
> '0841ea4a3b41 ("iomap: keep on increasing i_size in iomap_write_end()")',
> but the real problem is xfs_setattr_size() doesn't zero out enough range
> when truncate down a realtime inode. So this series fix this problem by
> just converting the tail blocks to unwritten when truncate down realtime
> inodes, then we could bring commit 943bc0882ceb back.
>
> I've tested this series on fstests (1) with reflink=0, (2) with
> reflink=1, (3) with 28K RT device, no new failures detected, and it
> passed generic/561 on RT device over 300+ rounds, please let me know if
> we need any other test.
>
> [1] https://lore.kernel.org/linux-xfs/87ttj8ircu.fsf@debian-BULLSEYE-live-builder-AMD64/
>
> Thanks,
> Yi.
>
> Zhang Yi (2):
> xfs: reserve blocks for truncating large realtime inode
> iomap: don't increase i_size in iomap_write_end()
>
> fs/iomap/buffered-io.c | 53 +++++++++++++++++++++++-------------------
> fs/xfs/xfs_iops.c | 15 +++++++++++-
> 2 files changed, 43 insertions(+), 25 deletions(-)
>
> --
> 2.39.2
>
>
