On 2024/6/19 8:24, Darrick J. Wong wrote: > 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> > Thanks for the review. BTW, what kind of fstests configs do you usually run? I always run kvm-xfstests -g auto[1] before I submitting ext4 patches, I'd also like to run full tests and hope to find regressions before submitting xfs patches. [1] https://github.com/tytso/xfstests-bld Thanks, Yi. > > >> 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 >> >>
