* Re: [PATCH v4 12/13] ext4: move pagecache_isize_extended() out of active handle
From: Jan Kara @ 2026-04-01 17:21 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260327102939.1095257-13-yi.zhang@huaweicloud.com>
On Fri 27-03-26 18:29:38, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In ext4_alloc_file_blocks(), pagecache_isize_extended() is called under
> an active handle and may also hold folio lock if the block size is
> smaller than the folio size. This also breaks the "folio lock ->
> transaction start" lock ordering for the upcoming iomap buffered I/O
> path.
>
> Therefore, move pagecache_isize_extended() outside of an active handle.
> Additionally, it is unnecessary to update the file length during each
> iteration of the allocation loop. Instead, update the file length only
> to the position where the allocation is successful. Postpone updating
> the inode size until after the allocation loop completes or is
> interrupted due to an error.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good! Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents.c | 62 +++++++++++++++++++++++++++++------------------
> 1 file changed, 39 insertions(+), 23 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7abe47f923c0..f13f604b1f67 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4553,7 +4553,7 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
> ext4_lblk_t len_lblk;
> struct ext4_map_blocks map;
> unsigned int credits;
> - loff_t epos, old_size = i_size_read(inode);
> + loff_t epos = 0, old_size = i_size_read(inode);
> unsigned int blkbits = inode->i_blkbits;
> bool alloc_zero = false;
>
> @@ -4618,44 +4618,60 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
> ext4_journal_stop(handle);
> break;
> }
> + ext4_update_inode_fsync_trans(handle, inode, 1);
> + ret = ext4_journal_stop(handle);
> + if (unlikely(ret))
> + break;
> +
> /*
> * allow a full retry cycle for any remaining allocations
> */
> retries = 0;
> - epos = EXT4_LBLK_TO_B(inode, map.m_lblk + ret);
> - if (new_size) {
> - if (epos > new_size)
> - epos = new_size;
> - ext4_update_inode_size(inode, epos);
> - if (epos > old_size)
> - pagecache_isize_extended(inode, old_size, epos);
> - }
> - ret2 = ext4_mark_inode_dirty(handle, inode);
> - ext4_update_inode_fsync_trans(handle, inode, 1);
> - ret3 = ext4_journal_stop(handle);
> - ret2 = ret3 ? ret3 : ret2;
> - if (unlikely(ret2))
> - break;
>
> if (alloc_zero &&
> (map.m_flags & (EXT4_MAP_MAPPED | EXT4_MAP_UNWRITTEN))) {
> - ret2 = ext4_issue_zeroout(inode, map.m_lblk, map.m_pblk,
> - map.m_len);
> - if (likely(!ret2))
> - ret2 = ext4_convert_unwritten_extents(NULL,
> + ret = ext4_issue_zeroout(inode, map.m_lblk, map.m_pblk,
> + map.m_len);
> + if (likely(!ret))
> + ret = ext4_convert_unwritten_extents(NULL,
> inode, (loff_t)map.m_lblk << blkbits,
> (loff_t)map.m_len << blkbits);
> - if (ret2)
> + if (ret)
> break;
> }
>
> - map.m_lblk += ret;
> - map.m_len = len_lblk = len_lblk - ret;
> + map.m_lblk += map.m_len;
> + map.m_len = len_lblk = len_lblk - map.m_len;
> + epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
> }
> +
> if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
> goto retry;
>
> - return ret > 0 ? ret2 : ret;
> + if (!epos || !new_size)
> + return ret;
> +
> + /*
> + * Allocate blocks, update the file size to match the size of the
> + * already successfully allocated blocks.
> + */
> + if (epos > new_size)
> + epos = new_size;
> +
> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> + if (IS_ERR(handle))
> + return ret ? ret : PTR_ERR(handle);
> +
> + ext4_update_inode_size(inode, epos);
> + ret2 = ext4_mark_inode_dirty(handle, inode);
> + ext4_update_inode_fsync_trans(handle, inode, 1);
> + ret3 = ext4_journal_stop(handle);
> + ret2 = ret3 ? ret3 : ret2;
> +
> + if (epos > old_size)
> + pagecache_isize_extended(inode, old_size, epos);
> +
> + return ret ? ret : ret2;
> }
>
> static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len);
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 11/13] ext4: remove ctime/mtime update from ext4_alloc_file_blocks()
From: Jan Kara @ 2026-04-01 17:09 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260327102939.1095257-12-yi.zhang@huaweicloud.com>
On Fri 27-03-26 18:29:37, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> The ctime and mtime update is already handled by file_modified() in
> ext4_fallocate(), the caller of ext4_alloc_file_blocks(). So remove the
> redundant calls to inode_set_ctime_current() and inode_set_mtime_to_ts()
> in ext4_alloc_file_blocks().
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index f4009544f762..7abe47f923c0 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4623,13 +4623,10 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
> */
> retries = 0;
> epos = EXT4_LBLK_TO_B(inode, map.m_lblk + ret);
> - inode_set_ctime_current(inode);
> if (new_size) {
> if (epos > new_size)
> epos = new_size;
> - if (ext4_update_inode_size(inode, epos) & 0x1)
> - inode_set_mtime_to_ts(inode,
> - inode_get_ctime(inode));
> + ext4_update_inode_size(inode, epos);
> if (epos > old_size)
> pagecache_isize_extended(inode, old_size, epos);
> }
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 10/13] ext4: unify SYNC mode checks in fallocate paths
From: Jan Kara @ 2026-04-01 17:06 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260327102939.1095257-11-yi.zhang@huaweicloud.com>
On Fri 27-03-26 18:29:36, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In the ext4 fallocate call chain, SYNC mode handling is inconsistent:
> some places check the inode state, while others check the open file
> descriptor state. Unify these checks by evaluating both conditions
> to ensure consistent behavior across all fallocate operations.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Ah, OK, you unify it here :). Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents.c | 9 +++++----
> fs/ext4/inode.c | 2 +-
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 16386f499138..f4009544f762 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4753,7 +4753,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> goto out_handle;
>
> ext4_update_inode_fsync_trans(handle, inode, 1);
> - if (file->f_flags & O_SYNC)
> + if ((file->f_flags & O_SYNC) || IS_SYNC(inode))
> ext4_handle_sync(handle);
>
> out_handle:
> @@ -4791,7 +4791,8 @@ static long ext4_do_fallocate(struct file *file, loff_t offset,
> if (ret)
> goto out;
>
> - if (file->f_flags & O_SYNC && EXT4_SB(inode->i_sb)->s_journal) {
> + if (((file->f_flags & O_SYNC) || IS_SYNC(inode)) &&
> + EXT4_SB(inode->i_sb)->s_journal) {
> ret = ext4_fc_commit(EXT4_SB(inode->i_sb)->s_journal,
> EXT4_I(inode)->i_sync_tid);
> }
> @@ -5564,7 +5565,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
> goto out_handle;
>
> ext4_update_inode_fsync_trans(handle, inode, 1);
> - if (IS_SYNC(inode))
> + if ((file->f_flags & O_SYNC) || IS_SYNC(inode))
> ext4_handle_sync(handle);
>
> out_handle:
> @@ -5688,7 +5689,7 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
> goto out_handle;
>
> ext4_update_inode_fsync_trans(handle, inode, 1);
> - if (IS_SYNC(inode))
> + if ((file->f_flags & O_SYNC) || IS_SYNC(inode))
> ext4_handle_sync(handle);
>
> out_handle:
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index b934ad86a96d..3b150e643ef5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4501,7 +4501,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> goto out_handle;
>
> ext4_update_inode_fsync_trans(handle, inode, 1);
> - if (IS_SYNC(inode))
> + if ((file->f_flags & O_SYNC) || IS_SYNC(inode))
> ext4_handle_sync(handle);
> out_handle:
> ext4_journal_stop(handle);
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 09/13] ext4: ensure zeroed partial blocks are persisted in SYNC mode
From: Jan Kara @ 2026-04-01 17:06 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
jack, ojaswin, ritesh.list, libaokun, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260327102939.1095257-10-yi.zhang@huaweicloud.com>
On Fri 27-03-26 18:29:35, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In ext4_zero_range() and ext4_punch_hole(), when operating in SYNC mode
> and zeroing a partial block, only data=journal modes guarantee that the
> zeroed data is synchronously persisted after the operation completes.
> For data=ordered/writeback mode and non-journal modes, this guarantee is
> missing.
>
> Introduce a partial_zero parameter to explicitly trigger writeback for
> all scenarios where a partial block is zeroed, ensuring the zeroed data
> is durably persisted.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
> fs/ext4/ext4.h | 2 +-
> fs/ext4/extents.c | 9 ++++++++-
> fs/ext4/inode.c | 19 ++++++++++++++-----
> 3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 859ae05339ad..bfe86479a83c 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3101,7 +3101,7 @@ extern int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
> int pextents);
> extern int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end);
> extern int ext4_zero_partial_blocks(struct inode *inode, loff_t lstart,
> - loff_t length);
> + loff_t length, bool *did_zero);
> extern vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf);
> extern qsize_t *ext4_get_reserved_space(struct inode *inode);
> extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 002b1ec8cee2..16386f499138 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4673,6 +4673,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> loff_t align_start, align_end, new_size = 0;
> loff_t end = offset + len;
> unsigned int blocksize = i_blocksize(inode);
> + bool partial_zeroed = false;
> int ret, flags;
>
> trace_ext4_zero_range(inode, offset, len, mode);
> @@ -4728,9 +4729,15 @@ static long ext4_zero_range(struct file *file, loff_t offset,
> return ret;
>
> /* Zero out partial block at the edges of the range */
> - ret = ext4_zero_partial_blocks(inode, offset, len);
> + ret = ext4_zero_partial_blocks(inode, offset, len, &partial_zeroed);
> if (ret)
> return ret;
> + if (((file->f_flags & O_SYNC) || IS_SYNC(inode)) && partial_zeroed) {
> + ret = filemap_write_and_wait_range(inode->i_mapping, offset,
> + end - 1);
> + if (ret)
> + return ret;
> + }
Shouldn't we handle this somewhat below, where we do:
if (file->f_flags & O_SYNC)
ext4_handle_sync(handle);
It would be logical to keep these together... Similarly for
ext4_punch_hole() below. An yes, the check when calling ext4_handle_sync()
should be extended with an IS_SYNC() check, which is a preexisting bug.
Honza
>
> handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> if (IS_ERR(handle)) {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 8aa4369e3150..b934ad86a96d 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4227,7 +4227,8 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> return 0;
> }
>
> -int ext4_zero_partial_blocks(struct inode *inode, loff_t lstart, loff_t length)
> +int ext4_zero_partial_blocks(struct inode *inode, loff_t lstart, loff_t length,
> + bool *did_zero)
> {
> struct super_block *sb = inode->i_sb;
> unsigned partial_start, partial_end;
> @@ -4244,20 +4245,21 @@ int ext4_zero_partial_blocks(struct inode *inode, loff_t lstart, loff_t length)
> /* Handle partial zero within the single block */
> if (start == end &&
> (partial_start || (partial_end != sb->s_blocksize - 1))) {
> - err = ext4_block_zero_range(inode, lstart, length, NULL, NULL);
> + err = ext4_block_zero_range(inode, lstart, length, did_zero,
> + NULL);
> return err;
> }
> /* Handle partial zero out on the start of the range */
> if (partial_start) {
> err = ext4_block_zero_range(inode, lstart, sb->s_blocksize,
> - NULL, NULL);
> + did_zero, NULL);
> if (err)
> return err;
> }
> /* Handle partial zero out on the end of the range */
> if (partial_end != sb->s_blocksize - 1)
> err = ext4_block_zero_range(inode, byte_end - partial_end,
> - partial_end + 1, NULL, NULL);
> + partial_end + 1, did_zero, NULL);
> return err;
> }
>
> @@ -4406,6 +4408,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> loff_t end = offset + length;
> handle_t *handle;
> unsigned int credits;
> + bool partial_zeroed = false;
> int ret;
>
> trace_ext4_punch_hole(inode, offset, length, 0);
> @@ -4441,9 +4444,15 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> if (ret)
> return ret;
>
> - ret = ext4_zero_partial_blocks(inode, offset, length);
> + ret = ext4_zero_partial_blocks(inode, offset, length, &partial_zeroed);
> if (ret)
> return ret;
> + if (((file->f_flags & O_SYNC) || IS_SYNC(inode)) && partial_zeroed) {
> + ret = filemap_write_and_wait_range(inode->i_mapping, offset,
> + end - 1);
> + if (ret)
> + return ret;
> + }
>
> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> credits = ext4_chunk_trans_extent(inode, 0);
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v6 21/22] xfs: add fsverity traces
From: Andrey Albershteyn @ 2026-04-01 13:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Andrey Albershteyn, linux-xfs, fsverity, linux-fsdevel, ebiggers,
linux-ext4, linux-f2fs-devel, linux-btrfs, djwong
In-Reply-To: <20260401063146.GD24459@lst.de>
On 2026-04-01 08:31:46, Christoph Hellwig wrote:
> Wouldn't most of these trace points make more sense in the core
> fsverity code before calling into the file system?
>
> Otherwise they look good to me.
>
oh the _file_corrupt one seems to be not used anymore with new
fserror interface
The _read/_write ones were pretty handy for debugging issues when
block size < page size when XFS read block, so, fsverity request a single page but
filesystem fetched more blocks, but with the current implementation
it's not relevant anymore
The only one which still make sense is get_descriptor, which
probably could be core fsverity one
I will drop this patch, this doesn't seem to worth it for single
trace point
--
- Andrey
^ permalink raw reply
* Re: [PATCH 15/42] fat: Sync and invalidate metadata buffers from fat_evict_inode()
From: OGAWA Hirofumi @ 2026-04-01 12:50 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise
In-Reply-To: <dvo2jvc6upmzrgqxx3n6er3e2gvujheongtzkihzmllwxqiyuq@h3qrrnwxlwo4>
Jan Kara <jack@suse.cz> writes:
>> Hm, metadata block is shared by several inodes. So earlier flush
>> makes fewer chance to combining multiple dirties.
>>
>> For example,
>> create dir-A
>> reclaimed and flushed dir-A
>> add new entries to dir-A
>> lost chance to combining re-dirty of dir-A
>
> Yes, but for this to be possible you would have to:
> 1) stop using dir-A between dir create & file creates
> 2) create enough memory pressure to cycle the dentry of dir-A through the
> LRU and reclaim it
> 3) continue memory pressure to cycle the inode for dir-A through the LRU
> and reclaim it
>
> So the amount of work that already has to happen to trigger flushing of
> a single block is so large that IMHO that flush will be lost in the noise.
>
>> >> Anyway, with it, reclaimed
>> >> inode metadata will be flushed forcibly and frequently (yeah, may not be
>> >> significant though. but I can't see the benefit for users from this
>> >> change.), and lost to chance combining multiple time of dirty while copy
>> >> many files.
>> >
>> > The benefit for users is 24 bytes saved for the majority of inodes that are
>> > there in the system - all the virtual inodes on sysfs / proc filesystem,
>> > all tmpfs inodes, all XFS inodes, all ext4 inodes when using journal (once I
>> > optimize ext4 code a bit), etc. So actually quite a bit of kernel memory
>> > saved in common configurations.
>> >
>> > Another win is that with metadata buffer head tracking now separated, I can
>> > modify that code (which will require growing the tracking structure) to
>> > properly track buffer head containing the inode and flush it on fsync(2).
>> > Currently there's a race that if flush worker writes out inode before
>> > fsync(2), then fsync(2) does not writeout the buffer containing the inode
>> > at all and thus data is not really persistent. This is actually my initial
>> > motivation for this refactoring since growing inode for everybody to fix
>> > data consistency issues of FAT/ext2/udf isn't popular these days...
>>
>> Agree, it is good. I'm only saying about the flushing earlier. To
>> implement it, is the flush earlier really necessary?
>
> Yes, to separate metadata buffer head tracking into a separate structure we
> must remove the handling of buffer head list from generic inode reclaim (as
> the filesystem has no way to provide the separate tracking structure there).
> Of course we could add a filesystem hook to inode reclaim to allow for
> handling of metadata bhs but:
>
> a) I'd rather do that in a way that is usable also for other issues
> filesystems have with inode reclaim as I mentioned in this thread before
>
> b) I don't think it's warranted for FAT etc. at this point as I don't think
> the possible overhead of metadata bh flushing on inode reclaim will be a
> problem in practice.
>
> But of course we can reevaluate if my gut feeling is wrong and someone
> comes with a workload which significantly regresses due to these changes.
OK. I'm still thinking we should go the way to reduce the amplification
for performance and storage lifetime if possible, not increasing.
However discussion looks like enough for us, and looks like we just
voted to different priority.
Thanks.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* Re: [PATCH 15/42] fat: Sync and invalidate metadata buffers from fat_evict_inode()
From: Jan Kara @ 2026-04-01 10:36 UTC (permalink / raw)
To: OGAWA Hirofumi
Cc: Jan Kara, linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise
In-Reply-To: <87tstvc90b.fsf@mail.parknet.co.jp>
On Wed 01-04-26 18:41:56, OGAWA Hirofumi wrote:
> Jan Kara <jack@suse.cz> writes:
>
> >> I think it would happen with normal operation, for example, copy many
> >> files more than total memory. I think this would be much common than
> >> write=>close=>open=>fsync in your example.
> >
> > When you copy a lot of files which are large in total, I agree the flushing
> > can be triggered. But I don't think it will trigger any excessive IO
> > because the metadata blocks being flushed aren't redirtied after the inode
> > is evicted. So blocks may be written out earlier but I don't think they
> > will be written out more times. For FAT for example you track only
> > directory blocks in these lists so when directory inode will be getting
> > evicted, you may see earlier writeout of dirty directory blocks but that's
> > all.
>
> Hm, metadata block is shared by several inodes. So earlier flush
> makes fewer chance to combining multiple dirties.
>
> For example,
> create dir-A
> reclaimed and flushed dir-A
> add new entries to dir-A
> lost chance to combining re-dirty of dir-A
Yes, but for this to be possible you would have to:
1) stop using dir-A between dir create & file creates
2) create enough memory pressure to cycle the dentry of dir-A through the
LRU and reclaim it
3) continue memory pressure to cycle the inode for dir-A through the LRU
and reclaim it
So the amount of work that already has to happen to trigger flushing of
a single block is so large that IMHO that flush will be lost in the noise.
> >> Anyway, with it, reclaimed
> >> inode metadata will be flushed forcibly and frequently (yeah, may not be
> >> significant though. but I can't see the benefit for users from this
> >> change.), and lost to chance combining multiple time of dirty while copy
> >> many files.
> >
> > The benefit for users is 24 bytes saved for the majority of inodes that are
> > there in the system - all the virtual inodes on sysfs / proc filesystem,
> > all tmpfs inodes, all XFS inodes, all ext4 inodes when using journal (once I
> > optimize ext4 code a bit), etc. So actually quite a bit of kernel memory
> > saved in common configurations.
> >
> > Another win is that with metadata buffer head tracking now separated, I can
> > modify that code (which will require growing the tracking structure) to
> > properly track buffer head containing the inode and flush it on fsync(2).
> > Currently there's a race that if flush worker writes out inode before
> > fsync(2), then fsync(2) does not writeout the buffer containing the inode
> > at all and thus data is not really persistent. This is actually my initial
> > motivation for this refactoring since growing inode for everybody to fix
> > data consistency issues of FAT/ext2/udf isn't popular these days...
>
> Agree, it is good. I'm only saying about the flushing earlier. To
> implement it, is the flush earlier really necessary?
Yes, to separate metadata buffer head tracking into a separate structure we
must remove the handling of buffer head list from generic inode reclaim (as
the filesystem has no way to provide the separate tracking structure there).
Of course we could add a filesystem hook to inode reclaim to allow for
handling of metadata bhs but:
a) I'd rather do that in a way that is usable also for other issues
filesystems have with inode reclaim as I mentioned in this thread before
b) I don't think it's warranted for FAT etc. at this point as I don't think
the possible overhead of metadata bh flushing on inode reclaim will be a
problem in practice.
But of course we can reevaluate if my gut feeling is wrong and someone
comes with a workload which significantly regresses due to these changes.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 15/42] fat: Sync and invalidate metadata buffers from fat_evict_inode()
From: OGAWA Hirofumi @ 2026-04-01 9:41 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise
In-Reply-To: <gkwyh4z45xglmq7jmam4yfvkecivipr23trzjkbyfolm6pdzre@d5huocjf55ew>
Jan Kara <jack@suse.cz> writes:
>> I think it would happen with normal operation, for example, copy many
>> files more than total memory. I think this would be much common than
>> write=>close=>open=>fsync in your example.
>
> When you copy a lot of files which are large in total, I agree the flushing
> can be triggered. But I don't think it will trigger any excessive IO
> because the metadata blocks being flushed aren't redirtied after the inode
> is evicted. So blocks may be written out earlier but I don't think they
> will be written out more times. For FAT for example you track only
> directory blocks in these lists so when directory inode will be getting
> evicted, you may see earlier writeout of dirty directory blocks but that's
> all.
Hm, metadata block is shared by several inodes. So earlier flush
makes fewer chance to combining multiple dirties.
For example,
create dir-A
reclaimed and flushed dir-A
add new entries to dir-A
lost chance to combining re-dirty of dir-A
>> Anyway, with it, reclaimed
>> inode metadata will be flushed forcibly and frequently (yeah, may not be
>> significant though. but I can't see the benefit for users from this
>> change.), and lost to chance combining multiple time of dirty while copy
>> many files.
>
> The benefit for users is 24 bytes saved for the majority of inodes that are
> there in the system - all the virtual inodes on sysfs / proc filesystem,
> all tmpfs inodes, all XFS inodes, all ext4 inodes when using journal (once I
> optimize ext4 code a bit), etc. So actually quite a bit of kernel memory
> saved in common configurations.
>
> Another win is that with metadata buffer head tracking now separated, I can
> modify that code (which will require growing the tracking structure) to
> properly track buffer head containing the inode and flush it on fsync(2).
> Currently there's a race that if flush worker writes out inode before
> fsync(2), then fsync(2) does not writeout the buffer containing the inode
> at all and thus data is not really persistent. This is actually my initial
> motivation for this refactoring since growing inode for everybody to fix
> data consistency issues of FAT/ext2/udf isn't popular these days...
Agree, it is good. I'm only saying about the flushing earlier. To
implement it, is the flush earlier really necessary?
Thanks.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* Re: [PATCH 15/42] fat: Sync and invalidate metadata buffers from fat_evict_inode()
From: Jan Kara @ 2026-04-01 9:11 UTC (permalink / raw)
To: OGAWA Hirofumi
Cc: Jan Kara, linux-fsdevel, linux-block, Christian Brauner, Al Viro,
linux-ext4, Ted Tso, Tigran A. Aivazian, David Sterba,
Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
linux-aio, Benjamin LaHaise
In-Reply-To: <87wlyss2ny.fsf@mail.parknet.co.jp>
On Tue 31-03-26 19:40:01, OGAWA Hirofumi wrote:
> Jan Kara <jack@suse.cz> writes:
> >> It is including trade off write amplification vs reliability (i.e. may
> >> not call fsync()), for example. So I think we should not add it easily.
> >
> > I expect in practice you'll hardly be able to observe the difference as
> > inodes usually get quite a while to be reclaimed at which point the dirty
> > buffers would be already flushed by background writeback. I don't see how
> > this change would lead specifically to "write amplification" - that would
> > mean frequent redirtying of the same metadata buffer of an inode
> > interleaved with frequent reclaims of the inode and I don't see how that
> > would happen in a realistic setting.
> >
> > If someone comes with a realistic workload which would suffer significant
> > regression from this change, then of course we should address it. I have
> > plans for adding an interface for filesystems to expose the information
> > that inode has some pending dirty metadata and a way to flush them from
> > flush worker because that is a common need a lot of filesystems has and
> > doing the flushing from .evict isn't always doable due to locking
> > constraints.
>
> I think it would happen with normal operation, for example, copy many
> files more than total memory. I think this would be much common than
> write=>close=>open=>fsync in your example.
When you copy a lot of files which are large in total, I agree the flushing
can be triggered. But I don't think it will trigger any excessive IO
because the metadata blocks being flushed aren't redirtied after the inode
is evicted. So blocks may be written out earlier but I don't think they
will be written out more times. For FAT for example you track only
directory blocks in these lists so when directory inode will be getting
evicted, you may see earlier writeout of dirty directory blocks but that's
all.
> Anyway, with it, reclaimed
> inode metadata will be flushed forcibly and frequently (yeah, may not be
> significant though. but I can't see the benefit for users from this
> change.), and lost to chance combining multiple time of dirty while copy
> many files.
The benefit for users is 24 bytes saved for the majority of inodes that are
there in the system - all the virtual inodes on sysfs / proc filesystem,
all tmpfs inodes, all XFS inodes, all ext4 inodes when using journal (once I
optimize ext4 code a bit), etc. So actually quite a bit of kernel memory
saved in common configurations.
Another win is that with metadata buffer head tracking now separated, I can
modify that code (which will require growing the tracking structure) to
properly track buffer head containing the inode and flush it on fsync(2).
Currently there's a race that if flush worker writes out inode before
fsync(2), then fsync(2) does not writeout the buffer containing the inode
at all and thus data is not really persistent. This is actually my initial
motivation for this refactoring since growing inode for everybody to fix
data consistency issues of FAT/ext2/udf isn't popular these days...
> > I'm still thinking about details but this has to be a properly
> > abstracted interface all filesystems can use and not a special hack for a
> > handful of old filesystems.
>
> Sounds great. How about we delay this behavior change until this
> interface?
I would prefer not since that would delay fixing the data consistency
issues and I expect that reclaim discussion not to be very fast to
conclude.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v5 3/5] ext4: fix the error handling process in extents_kunit_init).
From: Ojaswin Mujoo @ 2026-04-01 7:34 UTC (permalink / raw)
To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, jack
In-Reply-To: <20260330133035.287842-4-yebin@huaweicloud.com>
On Mon, Mar 30, 2026 at 09:30:33PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> The error processing in extents_kunit_init() is improper, causing
> resource leakage.
> Reconstruct the error handling process to prevent potential resource
> leaks
>
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
Looks good now, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Regards,
ojaswin
^ permalink raw reply
* Re: [PATCH v6 00/22] fs-verity support for XFS with post EOF merkle tree
From: Christoph Hellwig @ 2026-04-01 6:32 UTC (permalink / raw)
To: Andrey Albershteyn, ebiggers
Cc: linux-xfs, fsverity, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong, david
In-Reply-To: <20260331212827.2631020-1-aalbersh@kernel.org>
Eric,
can we get you to chime in on the core fsverity changes, especially
the hole handling?
^ permalink raw reply
* Re: [PATCH v6 22/22] xfs: enable ro-compat fs-verity flag
From: Christoph Hellwig @ 2026-04-01 6:32 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-23-aalbersh@kernel.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v6 21/22] xfs: add fsverity traces
From: Christoph Hellwig @ 2026-04-01 6:31 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-22-aalbersh@kernel.org>
Wouldn't most of these trace points make more sense in the core
fsverity code before calling into the file system?
Otherwise they look good to me.
^ permalink raw reply
* Re: [PATCH v6 07/22] iomap: teach iomap to read files with fsverity
From: Christoph Hellwig @ 2026-04-01 6:30 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-8-aalbersh@kernel.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v6 06/22] iomap: introduce IOMAP_F_FSVERITY and teach writeback to handle fsverity
From: Christoph Hellwig @ 2026-04-01 6:28 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-7-aalbersh@kernel.org>
> + /*
> + * fsverity metadata is stored past i_size, we need to read it instead of
Overly long line.
Otherwise looks good.
^ permalink raw reply
* Re: [PATCH v6 02/22] fsverity: expose ensure_fsverity_info()
From: Christoph Hellwig @ 2026-04-01 6:27 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-3-aalbersh@kernel.org>
> +/**
> + * fsverity_ensure_verity_info() - create verity info if it's not in memory yet
> + * @inode: the inode for which verity info should be created
> + *
> + * Ensure this inode has verity info attached to it. Read fsverity descriptor
> + * and creates verity based on that. Inodes opened outside of
> + * file_operations->open will not have any verity info attached. This
> + * info is required for any fsverity related operations.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int fsverity_ensure_verity_info(struct inode *inode);
kerneldoc comments go next to the definition, not the prototype.
^ permalink raw reply
* Re: [PATCH v6 16/22] xfs: remove unwritten extents after preallocations in fsverity metadata
From: Darrick J. Wong @ 2026-03-31 23:36 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs
In-Reply-To: <20260331212827.2631020-17-aalbersh@kernel.org>
On Tue, Mar 31, 2026 at 11:28:17PM +0200, Andrey Albershteyn wrote:
> XFS preallocates spaces during writes. In normal I/O this space, if
> unused, is removed by truncate. For files with fsverity XFS does not use
> truncate as fsverity metadata is stored past EOF.
>
> After we're done with writing fsverity metadata iterate over extents in
> that region and remove any unwritten ones. These would be left overs in
> the holes in the merkle tree and past fsverity descriptor.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
/me is ok with this
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_fsverity.c | 67 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
> index 5a6a48fcf843..b193009a1bdb 100644
> --- a/fs/xfs/xfs_fsverity.c
> +++ b/fs/xfs/xfs_fsverity.c
> @@ -21,6 +21,8 @@
> #include "xfs_iomap.h"
> #include "xfs_error.h"
> #include "xfs_health.h"
> +#include "xfs_bmap.h"
> +#include "xfs_bmap_util.h"
> #include <linux/fsverity.h>
> #include <linux/iomap.h>
> #include <linux/pagemap.h>
> @@ -173,6 +175,63 @@ xfs_fsverity_delete_metadata(
> return error;
> }
>
> +static int
> +xfs_fsverity_cancel_unwritten(
> + struct xfs_inode *ip,
> + xfs_fileoff_t start,
> + xfs_fileoff_t end)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + xfs_fileoff_t offset_fsb = XFS_B_TO_FSB(mp, start);
> + xfs_fileoff_t end_fsb = XFS_B_TO_FSB(mp, end);
> + struct xfs_bmbt_irec imap;
> + int nimaps;
> + int error = 0;
> + int done;
> +
> +
> + while (offset_fsb < end_fsb) {
> + nimaps = 1;
> +
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0,
> + 0, &tp);
> + if (error)
> + return error;
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
> + &imap, &nimaps, 0);
> + if (error)
> + goto out_cancel;
> +
> + if (nimaps == 0)
> + goto out_cancel;
> +
> + if (imap.br_state == XFS_EXT_UNWRITTEN) {
> + xfs_trans_ijoin(tp, ip, 0);
> +
> + error = xfs_bunmapi(tp, ip, imap.br_startoff,
> + imap.br_blockcount, 0, 1, &done);
> + if (error)
> + goto out_cancel;
> +
> + error = xfs_trans_commit(tp);
> + } else {
> + xfs_trans_cancel(tp);
> + }
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> + offset_fsb = imap.br_startoff + imap.br_blockcount;
> + }
> +
> + return error;
> +out_cancel:
> + xfs_trans_cancel(tp);
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> + return error;
> +}
> +
>
> /*
> * Prepare to enable fsverity by clearing old metadata.
> @@ -248,6 +307,14 @@ xfs_fsverity_end_enable(
> if (error)
> goto out;
>
> + /*
> + * Remove unwritten extents left by COW preallocations and write
> + * preallocation in the merkle tree holes and past descriptor
> + */
> + error = xfs_fsverity_cancel_unwritten(ip, range_start, LLONG_MAX);
> + if (error)
> + goto out;
> +
> /*
> * Proactively drop any delayed allocations in COW fork, the fsverity
> * files are read-only
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH v6 15/22] xfs: add fs-verity support
From: Darrick J. Wong @ 2026-03-31 23:35 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs
In-Reply-To: <20260331212827.2631020-16-aalbersh@kernel.org>
On Tue, Mar 31, 2026 at 11:28:16PM +0200, Andrey Albershteyn wrote:
> Add integration with fs-verity. XFS stores fs-verity descriptor and
> Merkle tree in the inode data fork at first block aligned to 64k past
> EOF.
>
> The Merkle tree reading/writing is done through iomap interface. The
> data itself is read to the inode's page cache. When XFS reads from this
> region iomap doesn't call into fsverity to verify it against Merkle
> tree. For data, verification is done at ioend completion in a workqueue.
>
> When fs-verity is enabled on an inode, the XFS_IVERITY_CONSTRUCTION
> flag is set meaning that the Merkle tree is being build. The
> initialization ends with storing of verity descriptor and setting
> inode on-disk flag (XFS_DIFLAG2_VERITY). Lastly, the
> XFS_IVERITY_CONSTRUCTION is dropped and I_VERITY is set on inode.
>
> The descriptor is stored in a new block aligned to 64k after the last
> Merkle tree block. The size of the descriptor is stored at the end of
> the last descriptor block (descriptor can be multiple blocks).
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks good to me still, and thanks for confirming my infogatherings on
the previous revision!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_bmap_util.c | 8 +
> fs/xfs/xfs_fsverity.c | 349 ++++++++++++++++++++++++++++++++++++++++-
> fs/xfs/xfs_fsverity.h | 2 +
> fs/xfs/xfs_message.c | 4 +
> fs/xfs/xfs_message.h | 1 +
> fs/xfs/xfs_mount.h | 2 +
> fs/xfs/xfs_super.c | 7 +
> 7 files changed, 372 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 0ab00615f1ad..18348f4fd2aa 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -31,6 +31,7 @@
> #include "xfs_rtbitmap.h"
> #include "xfs_rtgroup.h"
> #include "xfs_zone_alloc.h"
> +#include <linux/fsverity.h>
>
> /* Kernel only BMAP related definitions and functions */
>
> @@ -553,6 +554,13 @@ xfs_can_free_eofblocks(
> if (last_fsb <= end_fsb)
> return false;
>
> + /*
> + * Nothing to clean on fsverity inodes as they don't use prealloc and
> + * there no delalloc as only written data is fsverity metadata
> + */
> + if (IS_VERITY(VFS_I(ip)))
> + return false;
> +
> /*
> * Check if there is an post-EOF extent to free. If there are any
> * delalloc blocks attached to the inode (data fork delalloc
> diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
> index b983e20bb5e1..5a6a48fcf843 100644
> --- a/fs/xfs/xfs_fsverity.c
> +++ b/fs/xfs/xfs_fsverity.c
> @@ -4,14 +4,26 @@
> */
> #include "xfs_platform.h"
> #include "xfs_format.h"
> -#include "xfs_inode.h"
> #include "xfs_shared.h"
> #include "xfs_trans_resv.h"
> #include "xfs_mount.h"
> #include "xfs_fsverity.h"
> +#include "xfs_da_format.h"
> +#include "xfs_da_btree.h"
> +#include "xfs_inode.h"
> +#include "xfs_log_format.h"
> +#include "xfs_bmap_util.h"
> +#include "xfs_log_format.h"
> +#include "xfs_trans.h"
> +#include "xfs_trace.h"
> +#include "xfs_quota.h"
> #include "xfs_fsverity.h"
> +#include "xfs_iomap.h"
> +#include "xfs_error.h"
> +#include "xfs_health.h"
> #include <linux/fsverity.h>
> #include <linux/iomap.h>
> +#include <linux/pagemap.h>
>
> loff_t
> xfs_fsverity_metadata_offset(
> @@ -28,3 +40,338 @@ xfs_fsverity_is_file_data(
> return fsverity_active(VFS_IC(ip)) &&
> offset < xfs_fsverity_metadata_offset(ip);
> }
> +
> +/*
> + * Retrieve the verity descriptor.
> + */
> +static int
> +xfs_fsverity_get_descriptor(
> + struct inode *inode,
> + void *buf,
> + size_t buf_size)
> +{
> + struct xfs_inode *ip = XFS_I(inode);
> + struct xfs_mount *mp = ip->i_mount;
> + __be32 d_desc_size;
> + u32 desc_size;
> + u64 desc_size_pos;
> + int error;
> + u64 desc_pos;
> + struct xfs_bmbt_irec rec;
> + int is_empty;
> + uint32_t blocksize = i_blocksize(VFS_I(ip));
> + xfs_fileoff_t last_block_offset;
> +
> + ASSERT(inode->i_flags & S_VERITY);
> + error = xfs_bmap_last_extent(NULL, ip, XFS_DATA_FORK, &rec, &is_empty);
> + if (error)
> + return error;
> +
> + if (is_empty)
> + return -ENODATA;
> +
> + last_block_offset =
> + XFS_FSB_TO_B(mp, rec.br_startoff + rec.br_blockcount);
> + if (last_block_offset < xfs_fsverity_metadata_offset(ip))
> + return -ENODATA;
> +
> + desc_size_pos = last_block_offset - sizeof(__be32);
> + error = fsverity_pagecache_read(inode, (char *)&d_desc_size,
> + sizeof(d_desc_size), desc_size_pos);
> + if (error)
> + return error;
> +
> + desc_size = be32_to_cpu(d_desc_size);
> + if (XFS_IS_CORRUPT(mp, desc_size > FS_VERITY_MAX_DESCRIPTOR_SIZE))
> + return -ERANGE;
> + if (XFS_IS_CORRUPT(mp, desc_size > desc_size_pos))
> + return -ERANGE;
> +
> + if (!buf_size)
> + return desc_size;
> +
> + if (XFS_IS_CORRUPT(mp, desc_size > buf_size))
> + return -ERANGE;
> +
> + desc_pos = round_down(desc_size_pos - desc_size, blocksize);
> + error = fsverity_pagecache_read(inode, buf, desc_size, desc_pos);
> + if (error)
> + return error;
> +
> + return desc_size;
> +}
> +
> +static int
> +xfs_fsverity_write_descriptor(
> + struct file *file,
> + const void *desc,
> + u32 desc_size,
> + u64 merkle_tree_size)
> +{
> + int error;
> + struct inode *inode = file_inode(file);
> + struct xfs_inode *ip = XFS_I(inode);
> + unsigned int blksize = ip->i_mount->m_attr_geo->blksize;
> + u64 tree_last_block =
> + xfs_fsverity_metadata_offset(ip) + merkle_tree_size;
> + u64 desc_pos =
> + round_up(tree_last_block, XFS_FSVERITY_START_ALIGN);
> + u64 desc_end = desc_pos + desc_size;
> + __be32 desc_size_disk = cpu_to_be32(desc_size);
> + u64 desc_size_pos =
> + round_up(desc_end + sizeof(desc_size_disk), blksize) -
> + sizeof(desc_size_disk);
> +
> + error = iomap_fsverity_write(file, desc_size_pos, sizeof(__be32),
> + (const void *)&desc_size_disk,
> + &xfs_buffered_write_iomap_ops,
> + &xfs_iomap_write_ops);
> + if (error)
> + return error;
> +
> + return iomap_fsverity_write(file, desc_pos, desc_size, desc,
> + &xfs_buffered_write_iomap_ops,
> + &xfs_iomap_write_ops);
> +}
> +
> +/*
> + * Try to remove all the fsverity metadata after a failed enablement.
> + */
> +static int
> +xfs_fsverity_delete_metadata(
> + struct xfs_inode *ip)
> +{
> + struct xfs_trans *tp;
> + struct xfs_mount *mp = ip->i_mount;
> + int error;
> +
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
> + if (error)
> + return error;
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, ip, 0);
> +
> + /*
> + * We removing post EOF data, no need to update i_size as fsverity
> + * didn't move i_size in the first place
> + */
> + error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, XFS_ISIZE(ip));
> + if (error)
> + goto err_cancel;
> +
> + error = xfs_trans_commit(tp);
> + if (error)
> + goto err_cancel;
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> + return error;
> +
> +err_cancel:
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> + xfs_trans_cancel(tp);
> + return error;
> +}
> +
> +
> +/*
> + * Prepare to enable fsverity by clearing old metadata.
> + */
> +static int
> +xfs_fsverity_begin_enable(
> + struct file *filp)
> +{
> + struct inode *inode = file_inode(filp);
> + struct xfs_inode *ip = XFS_I(inode);
> + int error;
> +
> + xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
> +
> + if (IS_DAX(inode))
> + return -EINVAL;
> +
> + if (inode->i_size > XFS_FSVERITY_LARGEST_FILE)
> + return -EFBIG;
> +
> + /*
> + * Flush pagecache before building Merkle tree. Inode is locked and no
> + * further writes will happen to the file except fsverity metadata
> + */
> + error = filemap_write_and_wait(inode->i_mapping);
> + if (error)
> + return error;
> +
> + if (xfs_iflags_test_and_set(ip, XFS_VERITY_CONSTRUCTION))
> + return -EBUSY;
> +
> + error = xfs_qm_dqattach(ip);
> + if (error)
> + return error;
> +
> + return xfs_fsverity_delete_metadata(ip);
> +}
> +
> +/*
> + * Complete (or fail) the process of enabling fsverity.
> + */
> +static int
> +xfs_fsverity_end_enable(
> + struct file *file,
> + const void *desc,
> + size_t desc_size,
> + u64 merkle_tree_size)
> +{
> + struct inode *inode = file_inode(file);
> + struct xfs_inode *ip = XFS_I(inode);
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + int error = 0;
> + loff_t range_start = xfs_fsverity_metadata_offset(ip);
> +
> + xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
> +
> + /* fs-verity failed, just cleanup */
> + if (desc == NULL)
> + goto out;
> +
> + error = xfs_fsverity_write_descriptor(file, desc, desc_size,
> + merkle_tree_size);
> + if (error)
> + goto out;
> +
> + /*
> + * Wait for Merkle tree get written to disk before setting on-disk inode
> + * flag and clearing XFS_VERITY_CONSTRUCTION
> + */
> + error = filemap_write_and_wait_range(inode->i_mapping, range_start,
> + LLONG_MAX);
> + if (error)
> + goto out;
> +
> + /*
> + * Proactively drop any delayed allocations in COW fork, the fsverity
> + * files are read-only
> + */
> + if (xfs_is_cow_inode(ip))
> + xfs_bmap_punch_delalloc_range(ip, XFS_COW_FORK, 0, LLONG_MAX,
> + NULL);
> +
> + /*
> + * Set fsverity inode flag
> + */
> + error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_ichange,
> + 0, 0, false, &tp);
> + if (error)
> + goto out;
> +
> + /*
> + * Ensure that we've persisted the verity information before we enable
> + * it on the inode and tell the caller we have sealed the inode.
> + */
> + ip->i_diflags2 |= XFS_DIFLAG2_VERITY;
> +
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> + xfs_trans_set_sync(tp);
> +
> + error = xfs_trans_commit(tp);
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> + if (!error)
> + inode->i_flags |= S_VERITY;
> +
> +out:
> + if (error) {
> + int error2;
> +
> + error2 = xfs_fsverity_delete_metadata(ip);
> + if (error2)
> + xfs_alert(ip->i_mount,
> +"ino 0x%llx failed to clean up new fsverity metadata, err %d",
> + ip->i_ino, error2);
> + }
> +
> + xfs_iflags_clear(ip, XFS_VERITY_CONSTRUCTION);
> + return error;
> +}
> +
> +/*
> + * Retrieve a merkle tree block.
> + */
> +static struct page *
> +xfs_fsverity_read_merkle(
> + struct inode *inode,
> + pgoff_t index)
> +{
> + index += xfs_fsverity_metadata_offset(XFS_I(inode)) >> PAGE_SHIFT;
> +
> + return generic_read_merkle_tree_page(inode, index);
> +}
> +
> +/*
> + * Retrieve a merkle tree block.
> + */
> +static void
> +xfs_fsverity_readahead_merkle_tree(
> + struct inode *inode,
> + pgoff_t index,
> + unsigned long nr_pages)
> +{
> + index += xfs_fsverity_metadata_offset(XFS_I(inode)) >> PAGE_SHIFT;
> +
> + generic_readahead_merkle_tree(inode, index, nr_pages);
> +}
> +
> +/*
> + * Write a merkle tree block.
> + */
> +static int
> +xfs_fsverity_write_merkle(
> + struct file *file,
> + const void *buf,
> + u64 pos,
> + unsigned int size,
> + const u8 *zero_digest,
> + unsigned int digest_size)
> +{
> + struct inode *inode = file_inode(file);
> + struct xfs_inode *ip = XFS_I(inode);
> + loff_t position = pos +
> + xfs_fsverity_metadata_offset(ip);
> + const char *p;
> + unsigned int i;
> +
> + if (position + size > inode->i_sb->s_maxbytes)
> + return -EFBIG;
> +
> + /*
> + * If this is a block full of hashes of zeroed blocks, don't bother
> + * storing the block. We can synthesize them later.
> + *
> + * However, do this only in case Merkle tree block == fs block size.
> + * Iomap synthesizes these blocks based on holes in the merkle tree. We
> + * won't be able to tell if something need to be synthesizes for the
> + * range in the fs block. For example, for 4k filesystem block
> + *
> + * [ 1k | zero hashes | zero hashes | 1k ]
> + *
> + * Iomap won't know about these empty blocks.
> + */
> + for (i = 0, p = buf; i < size; i += digest_size, p += digest_size)
> + if (memcmp(p, zero_digest, digest_size))
> + break;
> + if (i == size && size == ip->i_mount->m_sb.sb_blocksize)
> + return 0;
> +
> + return iomap_fsverity_write(file, position, size, buf,
> + &xfs_buffered_write_iomap_ops,
> + &xfs_iomap_write_ops);
> +}
> +
> +const struct fsverity_operations xfs_fsverity_ops = {
> + .begin_enable_verity = xfs_fsverity_begin_enable,
> + .end_enable_verity = xfs_fsverity_end_enable,
> + .get_verity_descriptor = xfs_fsverity_get_descriptor,
> + .read_merkle_tree_page = xfs_fsverity_read_merkle,
> + .readahead_merkle_tree = xfs_fsverity_readahead_merkle_tree,
> + .write_merkle_tree_block = xfs_fsverity_write_merkle,
> +};
> diff --git a/fs/xfs/xfs_fsverity.h b/fs/xfs/xfs_fsverity.h
> index ec77ba571106..6a981e20a75b 100644
> --- a/fs/xfs/xfs_fsverity.h
> +++ b/fs/xfs/xfs_fsverity.h
> @@ -6,8 +6,10 @@
> #define __XFS_FSVERITY_H__
>
> #include "xfs_platform.h"
> +#include <linux/fsverity.h>
>
> #ifdef CONFIG_FS_VERITY
> +extern const struct fsverity_operations xfs_fsverity_ops;
> loff_t xfs_fsverity_metadata_offset(const struct xfs_inode *ip);
> bool xfs_fsverity_is_file_data(const struct xfs_inode *ip, loff_t offset);
> #else
> diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
> index fd297082aeb8..9818d8f8f239 100644
> --- a/fs/xfs/xfs_message.c
> +++ b/fs/xfs/xfs_message.c
> @@ -153,6 +153,10 @@ xfs_warn_experimental(
> .opstate = XFS_OPSTATE_WARNED_ZONED,
> .name = "zoned RT device",
> },
> + [XFS_EXPERIMENTAL_FSVERITY] = {
> + .opstate = XFS_OPSTATE_WARNED_FSVERITY,
> + .name = "fsverity",
> + },
> };
> ASSERT(feat >= 0 && feat < XFS_EXPERIMENTAL_MAX);
> BUILD_BUG_ON(ARRAY_SIZE(features) != XFS_EXPERIMENTAL_MAX);
> diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
> index 49b0ef40d299..083403944f11 100644
> --- a/fs/xfs/xfs_message.h
> +++ b/fs/xfs/xfs_message.h
> @@ -94,6 +94,7 @@ enum xfs_experimental_feat {
> XFS_EXPERIMENTAL_SHRINK,
> XFS_EXPERIMENTAL_LARP,
> XFS_EXPERIMENTAL_ZONED,
> + XFS_EXPERIMENTAL_FSVERITY,
>
> XFS_EXPERIMENTAL_MAX,
> };
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 07f6aa3c3f26..84d7cfb5e2c7 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -583,6 +583,8 @@ __XFS_HAS_FEAT(nouuid, NOUUID)
> #define XFS_OPSTATE_WARNED_ZONED 19
> /* (Zoned) GC is in progress */
> #define XFS_OPSTATE_ZONEGC_RUNNING 20
> +/* Kernel has logged a warning about fsverity support */
> +#define XFS_OPSTATE_WARNED_FSVERITY 21
>
> #define __XFS_IS_OPSTATE(name, NAME) \
> static inline bool xfs_is_ ## name (struct xfs_mount *mp) \
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f8de44443e81..d9d442009610 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -30,6 +30,7 @@
> #include "xfs_filestream.h"
> #include "xfs_quota.h"
> #include "xfs_sysfs.h"
> +#include "xfs_fsverity.h"
> #include "xfs_ondisk.h"
> #include "xfs_rmap_item.h"
> #include "xfs_refcount_item.h"
> @@ -1686,6 +1687,9 @@ xfs_fs_fill_super(
> sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> #endif
> sb->s_op = &xfs_super_operations;
> +#ifdef CONFIG_FS_VERITY
> + sb->s_vop = &xfs_fsverity_ops;
> +#endif
>
> /*
> * Delay mount work if the debug hook is set. This is debug
> @@ -1939,6 +1943,9 @@ xfs_fs_fill_super(
> if (error)
> goto out_filestream_unmount;
>
> + if (xfs_has_verity(mp))
> + xfs_warn_experimental(mp, XFS_EXPERIMENTAL_FSVERITY);
> +
> root = igrab(VFS_I(mp->m_rootip));
> if (!root) {
> error = -ENOENT;
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH v6 14/22] xfs: use read ioend for fsverity data verification
From: Darrick J. Wong @ 2026-03-31 23:34 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs
In-Reply-To: <20260331212827.2631020-15-aalbersh@kernel.org>
On Tue, Mar 31, 2026 at 11:28:15PM +0200, Andrey Albershteyn wrote:
> Use read ioends for fsverity verification. Do not issues fsverity
> metadata I/O through the same workqueue due to risk of a deadlock by a
> filled workqueue.
>
> Pass fsverity_info from iomap context down to the ioend as hashtable
> lookups are expensive.
>
> Add a simple helper to check that this is not fsverity metadata but file
> data that needs verification.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks fine to me still,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_aops.c | 46 ++++++++++++++++++++++++++++++++++---------
> fs/xfs/xfs_fsverity.c | 9 +++++++++
> fs/xfs/xfs_fsverity.h | 6 ++++++
> 3 files changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 9503252a0fa4..ecb07f250956 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -24,6 +24,7 @@
> #include "xfs_rtgroup.h"
> #include "xfs_fsverity.h"
> #include <linux/bio-integrity.h>
> +#include <linux/fsverity.h>
>
> struct xfs_writepage_ctx {
> struct iomap_writepage_ctx ctx;
> @@ -171,6 +172,23 @@ xfs_end_ioend_write(
> memalloc_nofs_restore(nofs_flag);
> }
>
> +/*
> + * IO read completion.
> + */
> +static void
> +xfs_end_ioend_read(
> + struct iomap_ioend *ioend)
> +{
> + struct xfs_inode *ip = XFS_I(ioend->io_inode);
> +
> + if (!ioend->io_bio.bi_status &&
> + xfs_fsverity_is_file_data(ip, ioend->io_offset))
> + fsverity_verify_bio(ioend->io_vi,
> + &ioend->io_bio);
> + iomap_finish_ioends(ioend,
> + blk_status_to_errno(ioend->io_bio.bi_status));
> +}
> +
> /*
> * Finish all pending IO completions that require transactional modifications.
> *
> @@ -205,8 +223,7 @@ xfs_end_io(
> list_del_init(&ioend->io_list);
> iomap_ioend_try_merge(ioend, &tmp);
> if (bio_op(&ioend->io_bio) == REQ_OP_READ)
> - iomap_finish_ioends(ioend,
> - blk_status_to_errno(ioend->io_bio.bi_status));
> + xfs_end_ioend_read(ioend);
> else
> xfs_end_ioend_write(ioend);
> cond_resched();
> @@ -232,9 +249,14 @@ xfs_end_bio(
> }
>
> spin_lock_irqsave(&ip->i_ioend_lock, flags);
> - if (list_empty(&ip->i_ioend_list))
> - WARN_ON_ONCE(!queue_work(mp->m_unwritten_workqueue,
> + if (list_empty(&ip->i_ioend_list)) {
> + if (IS_ENABLED(CONFIG_FS_VERITY) && ioend->io_vi &&
> + ioend->io_offset < xfs_fsverity_metadata_offset(ip))
> + fsverity_enqueue_verify_work(&ip->i_ioend_work);
> + else
> + WARN_ON_ONCE(!queue_work(mp->m_unwritten_workqueue,
> &ip->i_ioend_work));
> + }
> list_add_tail(&ioend->io_list, &ip->i_ioend_list);
> spin_unlock_irqrestore(&ip->i_ioend_lock, flags);
> }
> @@ -764,9 +786,13 @@ xfs_bio_submit_read(
> struct iomap_read_folio_ctx *ctx)
> {
> struct bio *bio = ctx->read_ctx;
> + struct iomap_ioend *ioend;
>
> /* defer read completions to the ioend workqueue */
> - iomap_init_ioend(iter->inode, bio, ctx->read_ctx_file_offset, 0);
> + ioend = iomap_init_ioend(iter->inode, bio, ctx->read_ctx_file_offset,
> + 0);
> + ioend->io_vi = ctx->vi;
> +
> bio->bi_end_io = xfs_end_bio;
> submit_bio(bio);
> }
> @@ -779,11 +805,13 @@ static const struct iomap_read_ops xfs_iomap_read_ops = {
>
> static inline const struct iomap_read_ops *
> xfs_get_iomap_read_ops(
> - const struct address_space *mapping)
> + const struct address_space *mapping,
> + loff_t position)
> {
> struct xfs_inode *ip = XFS_I(mapping->host);
>
> - if (bdev_has_integrity_csum(xfs_inode_buftarg(ip)->bt_bdev))
> + if (bdev_has_integrity_csum(xfs_inode_buftarg(ip)->bt_bdev) ||
> + xfs_fsverity_is_file_data(ip, position))
> return &xfs_iomap_read_ops;
> return &iomap_bio_read_ops;
> }
> @@ -795,7 +823,7 @@ xfs_vm_read_folio(
> {
> struct iomap_read_folio_ctx ctx = { .cur_folio = folio };
>
> - ctx.ops = xfs_get_iomap_read_ops(folio->mapping);
> + ctx.ops = xfs_get_iomap_read_ops(folio->mapping, folio_pos(folio));
> iomap_read_folio(&xfs_read_iomap_ops, &ctx, NULL);
> return 0;
> }
> @@ -806,7 +834,7 @@ xfs_vm_readahead(
> {
> struct iomap_read_folio_ctx ctx = { .rac = rac };
>
> - ctx.ops = xfs_get_iomap_read_ops(rac->mapping),
> + ctx.ops = xfs_get_iomap_read_ops(rac->mapping, readahead_pos(rac));
> iomap_readahead(&xfs_read_iomap_ops, &ctx, NULL);
> }
>
> diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
> index 6e6a8636a577..b983e20bb5e1 100644
> --- a/fs/xfs/xfs_fsverity.c
> +++ b/fs/xfs/xfs_fsverity.c
> @@ -19,3 +19,12 @@ xfs_fsverity_metadata_offset(
> {
> return round_up(i_size_read(VFS_IC(ip)), XFS_FSVERITY_START_ALIGN);
> }
> +
> +bool
> +xfs_fsverity_is_file_data(
> + const struct xfs_inode *ip,
> + loff_t offset)
> +{
> + return fsverity_active(VFS_IC(ip)) &&
> + offset < xfs_fsverity_metadata_offset(ip);
> +}
> diff --git a/fs/xfs/xfs_fsverity.h b/fs/xfs/xfs_fsverity.h
> index 5771db2cd797..ec77ba571106 100644
> --- a/fs/xfs/xfs_fsverity.h
> +++ b/fs/xfs/xfs_fsverity.h
> @@ -9,12 +9,18 @@
>
> #ifdef CONFIG_FS_VERITY
> loff_t xfs_fsverity_metadata_offset(const struct xfs_inode *ip);
> +bool xfs_fsverity_is_file_data(const struct xfs_inode *ip, loff_t offset);
> #else
> static inline loff_t xfs_fsverity_metadata_offset(const struct xfs_inode *ip)
> {
> WARN_ON_ONCE(1);
> return ULLONG_MAX;
> }
> +static inline bool xfs_fsverity_is_file_data(const struct xfs_inode *ip,
> + loff_t offset)
> +{
> + return false;
> +}
> #endif /* CONFIG_FS_VERITY */
>
> #endif /* __XFS_FSVERITY_H__ */
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH v6 08/22] iomap: introduce iomap_fsverity_write() for writing fsverity metadata
From: Darrick J. Wong @ 2026-03-31 23:32 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs
In-Reply-To: <20260331212827.2631020-9-aalbersh@kernel.org>
On Tue, Mar 31, 2026 at 11:28:09PM +0200, Andrey Albershteyn wrote:
> This is just a wrapper around iomap_file_buffered_write() to create
> necessary iterator over metadata.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks good to me.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/iomap/buffered-io.c | 25 +++++++++++++++++++++++++
> include/linux/iomap.h | 3 +++
> 2 files changed, 28 insertions(+)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 7ac319618f8e..0f89225dc3d7 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1287,6 +1287,31 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i,
> }
> EXPORT_SYMBOL_GPL(iomap_file_buffered_write);
>
> +int iomap_fsverity_write(struct file *file, loff_t pos, size_t length,
> + const void *buf, const struct iomap_ops *ops,
> + const struct iomap_write_ops *write_ops)
> +{
> + int ret;
> + struct iov_iter iiter;
> + struct kvec kvec = {
> + .iov_base = (void *)buf,
> + .iov_len = length,
> + };
> + struct kiocb iocb = {
> + .ki_filp = file,
> + .ki_ioprio = get_current_ioprio(),
> + .ki_pos = pos,
> + };
> +
> + iov_iter_kvec(&iiter, WRITE, &kvec, 1, length);
> +
> + ret = iomap_file_buffered_write(&iocb, &iiter, ops, write_ops, NULL);
> + if (ret < 0)
> + return ret;
> + return ret == length ? 0 : -EIO;
> +}
> +EXPORT_SYMBOL_GPL(iomap_fsverity_write);
> +
> static void iomap_write_delalloc_ifs_punch(struct inode *inode,
> struct folio *folio, loff_t start_byte, loff_t end_byte,
> struct iomap *iomap, iomap_punch_t punch)
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 4d9202cae29f..83586f09f365 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -359,6 +359,9 @@ static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter)
> ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
> const struct iomap_ops *ops,
> const struct iomap_write_ops *write_ops, void *private);
> +int iomap_fsverity_write(struct file *file, loff_t pos, size_t length,
> + const void *buf, const struct iomap_ops *ops,
> + const struct iomap_write_ops *write_ops);
> void iomap_read_folio(const struct iomap_ops *ops,
> struct iomap_read_folio_ctx *ctx, void *private);
> void iomap_readahead(const struct iomap_ops *ops,
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH v6 07/22] iomap: teach iomap to read files with fsverity
From: Darrick J. Wong @ 2026-03-31 23:30 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs
In-Reply-To: <20260331212827.2631020-8-aalbersh@kernel.org>
On Tue, Mar 31, 2026 at 11:28:08PM +0200, Andrey Albershteyn wrote:
> Obtain fsverity info for folios with file data and fsverity metadata.
> Filesystem can pass vi down to ioend and then to fsverity for
> verification. This is different from other filesystems ext4, f2fs, btrfs
> supporting fsverity, these filesystems don't need fsverity_info for
> reading fsverity metadata. While reading merkle tree iomap requires
> fsverity info to synthesize hashes for zeroed data block.
>
> fsverity metadata has two kinds of holes - ones in merkle tree and one
> after fsverity descriptor.
>
> Merkle tree holes are blocks full of hashes of zeroed data blocks. These
> are not stored on the disk but synthesized on the fly. This saves a bit
> of space for sparse files. Due to this iomap also need to lookup
> fsverity_info for folios with fsverity metadata. ->vi has a hash of the
> zeroed data block which will be used to fill the merkle tree block.
>
> The hole past descriptor is interpreted as end of metadata region. As we
> don't have EOF here we use this hole as an indication that rest of the
> folio is empty. This patch marks rest of the folio beyond fsverity
> descriptor as uptodate.
>
> For file data, fsverity needs to verify consistency of the whole file
> against the root hash, hashes of holes are included in the merkle tree.
> Verify them too.
>
> Issue reading of fsverity merkle tree on the fsverity inodes. This way
> metadata will be available at I/O completion time.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Still looks fine, thanks for reducing the patch count :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/iomap/buffered-io.c | 41 +++++++++++++++++++++++++++++++++++++++--
> include/linux/iomap.h | 2 ++
> 2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index a80fcb598cc8..7ac319618f8e 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -9,6 +9,7 @@
> #include <linux/swap.h>
> #include <linux/migrate.h>
> #include <linux/fserror.h>
> +#include <linux/fsverity.h>
> #include "internal.h"
> #include "trace.h"
>
> @@ -561,9 +562,27 @@ static int iomap_read_folio_iter(struct iomap_iter *iter,
> if (plen == 0)
> return 0;
>
> - /* zero post-eof blocks as the page may be mapped */
> - if (iomap_block_needs_zeroing(iter, pos)) {
> + /*
> + * Handling of fsverity "holes". We hit this for two case:
> + * 1. No need to go further, the hole after fsverity
> + * descriptor is the end of the fsverity metadata.
> + *
> + * 2. This folio contains merkle tree blocks which need to be
> + * synthesized. If we already have fsverity info (ctx->vi)
> + * synthesize these blocks.
> + */
> + if ((iomap->flags & IOMAP_F_FSVERITY) &&
> + iomap->type == IOMAP_HOLE) {
> + if (ctx->vi)
> + fsverity_fill_zerohash(folio, poff, plen,
> + ctx->vi);
> + iomap_set_range_uptodate(folio, poff, plen);
> + } else if (iomap_block_needs_zeroing(iter, pos)) {
> + /* zero post-eof blocks as the page may be mapped */
> folio_zero_range(folio, poff, plen);
> + if (ctx->vi &&
> + !fsverity_verify_blocks(ctx->vi, folio, plen, poff))
> + return -EIO;
> iomap_set_range_uptodate(folio, poff, plen);
> } else {
> if (!*bytes_submitted)
> @@ -614,6 +633,15 @@ void iomap_read_folio(const struct iomap_ops *ops,
>
> trace_iomap_readpage(iter.inode, 1);
>
> + /*
> + * Fetch fsverity_info for both data and fsverity metadata, as iomap
> + * needs zeroed hash for merkle tree block synthesis
> + */
> + ctx->vi = fsverity_get_info(iter.inode);
> + if (ctx->vi && iter.pos < i_size_read(iter.inode))
> + fsverity_readahead(ctx->vi, folio->index,
> + folio_nr_pages(folio));
> +
> while ((ret = iomap_iter(&iter, ops)) > 0)
> iter.status = iomap_read_folio_iter(&iter, ctx,
> &bytes_submitted);
> @@ -681,6 +709,15 @@ void iomap_readahead(const struct iomap_ops *ops,
>
> trace_iomap_readahead(rac->mapping->host, readahead_count(rac));
>
> + /*
> + * Fetch fsverity_info for both data and fsverity metadata, as iomap
> + * needs zeroed hash for merkle tree block synthesis
> + */
> + ctx->vi = fsverity_get_info(iter.inode);
> + if (ctx->vi && iter.pos < i_size_read(iter.inode))
> + fsverity_readahead(ctx->vi, readahead_index(rac),
> + readahead_count(rac));
> +
> while (iomap_iter(&iter, ops) > 0)
> iter.status = iomap_readahead_iter(&iter, ctx,
> &cur_bytes_submitted);
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 4506a99d5285..4d9202cae29f 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -435,6 +435,7 @@ struct iomap_ioend {
> loff_t io_offset; /* offset in the file */
> sector_t io_sector; /* start sector of ioend */
> void *io_private; /* file system private data */
> + struct fsverity_info *io_vi; /* fsverity info */
> struct bio io_bio; /* MUST BE LAST! */
> };
>
> @@ -509,6 +510,7 @@ struct iomap_read_folio_ctx {
> struct readahead_control *rac;
> void *read_ctx;
> loff_t read_ctx_file_offset;
> + struct fsverity_info *vi;
> };
>
> struct iomap_read_ops {
> --
> 2.51.2
>
>
^ permalink raw reply
* [PATCH v6 22/22] xfs: enable ro-compat fs-verity flag
From: Andrey Albershteyn @ 2026-03-31 21:28 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Andrey Albershteyn, hch, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-1-aalbersh@kernel.org>
Finalize fs-verity integration in XFS by making kernel fs-verity
aware with ro-compat flag.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
[djwong: add spaces]
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_format.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 4dff29659e40..0ce46c234b9c 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -378,8 +378,9 @@ xfs_sb_has_compat_feature(
#define XFS_SB_FEAT_RO_COMPAT_ALL \
(XFS_SB_FEAT_RO_COMPAT_FINOBT | \
XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
- XFS_SB_FEAT_RO_COMPAT_REFLINK| \
- XFS_SB_FEAT_RO_COMPAT_INOBTCNT)
+ XFS_SB_FEAT_RO_COMPAT_REFLINK | \
+ XFS_SB_FEAT_RO_COMPAT_INOBTCNT | \
+ XFS_SB_FEAT_RO_COMPAT_VERITY)
#define XFS_SB_FEAT_RO_COMPAT_UNKNOWN ~XFS_SB_FEAT_RO_COMPAT_ALL
static inline bool
xfs_sb_has_ro_compat_feature(
--
2.51.2
^ permalink raw reply related
* [PATCH v6 21/22] xfs: add fsverity traces
From: Andrey Albershteyn @ 2026-03-31 21:28 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Andrey Albershteyn, hch, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-1-aalbersh@kernel.org>
Even though fsverity has traces, debugging issues with varying block
sizes could be a bit less transparent without read/write traces.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_fsverity.c | 6 ++++++
fs/xfs/xfs_trace.h | 46 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
index ecc66ee8bac5..6db8113da52f 100644
--- a/fs/xfs/xfs_fsverity.c
+++ b/fs/xfs/xfs_fsverity.c
@@ -64,6 +64,8 @@ xfs_fsverity_get_descriptor(
uint32_t blocksize = i_blocksize(VFS_I(ip));
xfs_fileoff_t last_block_offset;
+ trace_xfs_fsverity_get_descriptor(ip);
+
ASSERT(inode->i_flags & S_VERITY);
error = xfs_bmap_last_extent(NULL, ip, XFS_DATA_FORK, &rec, &is_empty);
if (error)
@@ -377,6 +379,7 @@ xfs_fsverity_read_merkle(
pgoff_t index)
{
index += xfs_fsverity_metadata_offset(XFS_I(inode)) >> PAGE_SHIFT;
+ trace_xfs_fsverity_read_merkle(XFS_I(inode), index, PAGE_SIZE);
return generic_read_merkle_tree_page(inode, index);
}
@@ -391,6 +394,7 @@ xfs_fsverity_readahead_merkle_tree(
unsigned long nr_pages)
{
index += xfs_fsverity_metadata_offset(XFS_I(inode)) >> PAGE_SHIFT;
+ trace_xfs_fsverity_read_merkle(XFS_I(inode), index, PAGE_SIZE);
generic_readahead_merkle_tree(inode, index, nr_pages);
}
@@ -414,6 +418,8 @@ xfs_fsverity_write_merkle(
const char *p;
unsigned int i;
+ trace_xfs_fsverity_write_merkle(XFS_I(inode), position, size);
+
if (position + size > inode->i_sb->s_maxbytes)
return -EFBIG;
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 5e8190fe2be9..d05f79ec92db 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -6440,6 +6440,52 @@ TRACE_EVENT(xfs_verify_media_error,
__entry->error)
);
+TRACE_EVENT(xfs_fsverity_get_descriptor,
+ TP_PROTO(struct xfs_inode *ip),
+ TP_ARGS(ip),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(xfs_ino_t, ino)
+ ),
+ TP_fast_assign(
+ __entry->dev = VFS_I(ip)->i_sb->s_dev;
+ __entry->ino = ip->i_ino;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino)
+);
+
+DECLARE_EVENT_CLASS(xfs_fsverity_class,
+ TP_PROTO(struct xfs_inode *ip, u64 pos, size_t length),
+ TP_ARGS(ip, pos, length),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(xfs_ino_t, ino)
+ __field(u64, pos)
+ __field(size_t, length)
+ ),
+ TP_fast_assign(
+ __entry->dev = VFS_I(ip)->i_sb->s_dev;
+ __entry->ino = ip->i_ino;
+ __entry->pos = pos;
+ __entry->length = length;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx pos 0x%llx length 0x%zx",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino,
+ __entry->pos,
+ __entry->length)
+)
+
+#define DEFINE_FSVERITY_EVENT(name) \
+DEFINE_EVENT(xfs_fsverity_class, name, \
+ TP_PROTO(struct xfs_inode *ip, u64 pos, size_t length), \
+ TP_ARGS(ip, pos, length))
+DEFINE_FSVERITY_EVENT(xfs_fsverity_read_merkle);
+DEFINE_FSVERITY_EVENT(xfs_fsverity_write_merkle);
+DEFINE_FSVERITY_EVENT(xfs_fsverity_file_corrupt);
+
#endif /* _TRACE_XFS_H */
#undef TRACE_INCLUDE_PATH
--
2.51.2
^ permalink raw reply related
* [PATCH v6 20/22] xfs: introduce health state for corrupted fsverity metadata
From: Andrey Albershteyn @ 2026-03-31 21:28 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Andrey Albershteyn, hch, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong
In-Reply-To: <20260331212827.2631020-1-aalbersh@kernel.org>
Report corrupted fsverity descriptor through health system.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/xfs/libxfs/xfs_fs.h | 1 +
fs/xfs/libxfs/xfs_health.h | 4 +++-
fs/xfs/xfs_fsverity.c | 13 ++++++++++---
fs/xfs/xfs_health.c | 1 +
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index ebf17a0b0722..cece31ecee81 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -422,6 +422,7 @@ struct xfs_bulkstat {
#define XFS_BS_SICK_SYMLINK (1 << 6) /* symbolic link remote target */
#define XFS_BS_SICK_PARENT (1 << 7) /* parent pointers */
#define XFS_BS_SICK_DIRTREE (1 << 8) /* directory tree structure */
+#define XFS_BS_SICK_FSVERITY (1 << 9) /* fsverity metadata */
/*
* Project quota id helpers (previously projid was 16bit only
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index 1d45cf5789e8..932b447190da 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -104,6 +104,7 @@ struct xfs_rtgroup;
/* Don't propagate sick status to ag health summary during inactivation */
#define XFS_SICK_INO_FORGET (1 << 12)
#define XFS_SICK_INO_DIRTREE (1 << 13) /* directory tree structure */
+#define XFS_SICK_INO_FSVERITY (1 << 14) /* fsverity metadata */
/* Primary evidence of health problems in a given group. */
#define XFS_SICK_FS_PRIMARY (XFS_SICK_FS_COUNTERS | \
@@ -140,7 +141,8 @@ struct xfs_rtgroup;
XFS_SICK_INO_XATTR | \
XFS_SICK_INO_SYMLINK | \
XFS_SICK_INO_PARENT | \
- XFS_SICK_INO_DIRTREE)
+ XFS_SICK_INO_DIRTREE | \
+ XFS_SICK_INO_FSVERITY)
#define XFS_SICK_INO_ZAPPED (XFS_SICK_INO_BMBTD_ZAPPED | \
XFS_SICK_INO_BMBTA_ZAPPED | \
diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
index b193009a1bdb..ecc66ee8bac5 100644
--- a/fs/xfs/xfs_fsverity.c
+++ b/fs/xfs/xfs_fsverity.c
@@ -84,16 +84,23 @@ xfs_fsverity_get_descriptor(
return error;
desc_size = be32_to_cpu(d_desc_size);
- if (XFS_IS_CORRUPT(mp, desc_size > FS_VERITY_MAX_DESCRIPTOR_SIZE))
+ if (XFS_IS_CORRUPT(mp, desc_size > FS_VERITY_MAX_DESCRIPTOR_SIZE)) {
+ xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
return -ERANGE;
- if (XFS_IS_CORRUPT(mp, desc_size > desc_size_pos))
+ }
+
+ if (XFS_IS_CORRUPT(mp, desc_size > desc_size_pos)) {
+ xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
return -ERANGE;
+ }
if (!buf_size)
return desc_size;
- if (XFS_IS_CORRUPT(mp, desc_size > buf_size))
+ if (XFS_IS_CORRUPT(mp, desc_size > buf_size)) {
+ xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
return -ERANGE;
+ }
desc_pos = round_down(desc_size_pos - desc_size, blocksize);
error = fsverity_pagecache_read(inode, buf, desc_size, desc_pos);
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 239b843e83d4..be66760fb120 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -625,6 +625,7 @@ static const struct ioctl_sick_map ino_map[] = {
{ XFS_SICK_INO_DIR_ZAPPED, XFS_BS_SICK_DIR },
{ XFS_SICK_INO_SYMLINK_ZAPPED, XFS_BS_SICK_SYMLINK },
{ XFS_SICK_INO_DIRTREE, XFS_BS_SICK_DIRTREE },
+ { XFS_SICK_INO_FSVERITY, XFS_BS_SICK_FSVERITY },
};
/* Fill out bulkstat health info. */
--
2.51.2
^ permalink raw reply related
* [PATCH v6 19/22] xfs: check and repair the verity inode flag state
From: Andrey Albershteyn @ 2026-03-31 21:28 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Darrick J. Wong, hch, linux-ext4, linux-f2fs-devel, linux-btrfs,
Andrey Albershteyn
In-Reply-To: <20260331212827.2631020-1-aalbersh@kernel.org>
From: "Darrick J. Wong" <djwong@kernel.org>
If an inode has the incore verity iflag set, make sure that we can
actually activate fsverity on that inode. If activation fails due to
a fsverity metadata validation error, clear the flag. The usage model
for fsverity requires that any program that cares about verity state is
required to call statx/getflags to check that the flag is set after
opening the file, so clearing the flag will not compromise that model.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/xfs/scrub/attr.c | 7 +++++
fs/xfs/scrub/common.c | 53 +++++++++++++++++++++++++++++++++++++
fs/xfs/scrub/common.h | 2 ++
fs/xfs/scrub/inode.c | 7 +++++
fs/xfs/scrub/inode_repair.c | 36 +++++++++++++++++++++++++
5 files changed, 105 insertions(+)
diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 390ac2e11ee0..daf7962c2374 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -649,6 +649,13 @@ xchk_xattr(
if (!xfs_inode_hasattr(sc->ip))
return -ENOENT;
+ /*
+ * If this is a verity file that won't activate, we cannot check the
+ * merkle tree geometry.
+ */
+ if (xchk_inode_verity_broken(sc->ip))
+ xchk_set_incomplete(sc);
+
/* Allocate memory for xattr checking. */
error = xchk_setup_xattr_buf(sc, 0);
if (error == -ENOMEM)
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 20e63069088b..6cc6bea9c554 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -45,6 +45,8 @@
#include "scrub/health.h"
#include "scrub/tempfile.h"
+#include <linux/fsverity.h>
+
/* Common code for the metadata scrubbers. */
/*
@@ -1743,3 +1745,54 @@ xchk_inode_count_blocks(
return xfs_bmap_count_blocks(sc->tp, sc->ip, whichfork, nextents,
count);
}
+
+/*
+ * If this inode has S_VERITY set on it, read the verity info. If the reading
+ * fails with anything other than ENOMEM, the file is corrupt, which we can
+ * detect later with fsverity_active.
+ *
+ * Callers must hold the IOLOCK and must not hold the ILOCK of sc->ip because
+ * activation reads inode data.
+ */
+int
+xchk_inode_setup_verity(
+ struct xfs_scrub *sc)
+{
+ int error;
+
+ if (!fsverity_active(VFS_I(sc->ip)))
+ return 0;
+
+ error = fsverity_ensure_verity_info(VFS_I(sc->ip));
+ switch (error) {
+ case 0:
+ /* fsverity is active */
+ break;
+ case -ENODATA:
+ case -EMSGSIZE:
+ case -EINVAL:
+ case -EFSCORRUPTED:
+ case -EFBIG:
+ /*
+ * The nonzero errno codes above are the error codes that can
+ * be returned from fsverity on metadata validation errors.
+ */
+ return 0;
+ default:
+ /* runtime errors */
+ return error;
+ }
+
+ return 0;
+}
+
+/*
+ * Is this a verity file that failed to activate? Callers must have tried to
+ * activate fsverity via xchk_inode_setup_verity.
+ */
+bool
+xchk_inode_verity_broken(
+ struct xfs_inode *ip)
+{
+ return fsverity_active(VFS_I(ip)) && !fsverity_get_info(VFS_I(ip));
+}
diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index f2ecc68538f0..aa16d310bd6d 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -264,6 +264,8 @@ int xchk_inode_is_allocated(struct xfs_scrub *sc, xfs_agino_t agino,
bool *inuse);
int xchk_inode_count_blocks(struct xfs_scrub *sc, int whichfork,
xfs_extnum_t *nextents, xfs_filblks_t *count);
+int xchk_inode_setup_verity(struct xfs_scrub *sc);
+bool xchk_inode_verity_broken(struct xfs_inode *ip);
bool xchk_inode_is_dirtree_root(const struct xfs_inode *ip);
bool xchk_inode_is_sb_rooted(const struct xfs_inode *ip);
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 948d04dcba2a..8ce6917e22b4 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -36,6 +36,10 @@ xchk_prepare_iscrub(
xchk_ilock(sc, XFS_IOLOCK_EXCL);
+ error = xchk_inode_setup_verity(sc);
+ if (error)
+ return error;
+
error = xchk_trans_alloc(sc, 0);
if (error)
return error;
@@ -833,6 +837,9 @@ xchk_inode(
if (S_ISREG(VFS_I(sc->ip)->i_mode))
xchk_inode_check_reflink_iflag(sc, sc->ip->i_ino);
+ if (xchk_inode_verity_broken(sc->ip))
+ xchk_ino_set_corrupt(sc, sc->sm->sm_ino);
+
xchk_inode_check_unlinked(sc);
xchk_inode_xref(sc, sc->ip->i_ino, &di);
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 9738b9ce3f2d..3761e3922466 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -573,6 +573,8 @@ xrep_dinode_flags(
dip->di_nrext64_pad = 0;
else if (dip->di_version >= 3)
dip->di_v3_pad = 0;
+ if (!xfs_has_verity(mp) || !S_ISREG(mode))
+ flags2 &= ~XFS_DIFLAG2_VERITY;
if (flags2 & XFS_DIFLAG2_METADATA) {
xfs_failaddr_t fa;
@@ -1613,6 +1615,10 @@ xrep_dinode_core(
if (iget_error)
return iget_error;
+ error = xchk_inode_setup_verity(sc);
+ if (error)
+ return error;
+
error = xchk_trans_alloc(sc, 0);
if (error)
return error;
@@ -2032,6 +2038,27 @@ xrep_inode_unlinked(
return 0;
}
+/*
+ * If this file is a fsverity file, xchk_prepare_iscrub or xrep_dinode_core
+ * should have activated it. If it's still not active, then there's something
+ * wrong with the verity descriptor and we should turn it off.
+ */
+STATIC int
+xrep_inode_verity(
+ struct xfs_scrub *sc)
+{
+ struct inode *inode = VFS_I(sc->ip);
+
+ if (xchk_inode_verity_broken(sc->ip)) {
+ sc->ip->i_diflags2 &= ~XFS_DIFLAG2_VERITY;
+ inode->i_flags &= ~S_VERITY;
+
+ xfs_trans_log_inode(sc->tp, sc->ip, XFS_ILOG_CORE);
+ }
+
+ return 0;
+}
+
/* Repair an inode's fields. */
int
xrep_inode(
@@ -2081,6 +2108,15 @@ xrep_inode(
return error;
}
+ /*
+ * Disable fsverity if it cannot be activated. Activation failure
+ * prohibits the file from being opened, so there cannot be another
+ * program with an open fd to what it thinks is a verity file.
+ */
+ error = xrep_inode_verity(sc);
+ if (error)
+ return error;
+
/* Reconnect incore unlinked list */
error = xrep_inode_unlinked(sc);
if (error)
--
2.51.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox