[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2aec8c89-b09b-c173-de4e-6a6c1547d600@huaweicloud.com>
Date: Wed, 19 Jun 2024 09:52:46 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, hch@...radead.org, brauner@...nel.org,
david@...morbit.com, chandanbabu@...nel.org, jack@...e.cz,
yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com
Subject: Re: [PATCH -next v6 0/2] iomap/xfs: fix stale data exposure when
truncating realtime inodes
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@...nel.org>
>
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
>>
>>
Powered by blists - more mailing lists