* Re: [PATCH 05/10] ext4: rename ext4_block_zero_page_range() to ext4_block_zero_range()
From: Jan Kara @ 2026-03-23 16:49 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: <20260310014101.4140698-6-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:56, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Rename ext4_block_zero_page_range() to ext4_block_zero_range() since the
> "page" naming is no longer appropriate for current context. Also change
> its signature to take an inode pointer instead of an address_space. This
> aligns with the caller ext4_block_zero_eof() and
> ext4_zero_partial_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/inode.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index d63d455831b9..86f169df226a 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4147,11 +4147,9 @@ static int ext4_block_journalled_zero_range(handle_t *handle,
> * the end of the block it will be shortened to end of the block
> * that corresponds to 'from'
> */
> -static int ext4_block_zero_page_range(handle_t *handle,
> - struct address_space *mapping, loff_t from, loff_t length,
> - bool *did_zero)
> +static int ext4_block_zero_range(handle_t *handle, struct inode *inode,
> + loff_t from, loff_t length, bool *did_zero)
> {
> - struct inode *inode = mapping->host;
> unsigned blocksize = inode->i_sb->s_blocksize;
> unsigned int max = blocksize - (from & (blocksize - 1));
>
> @@ -4198,8 +4196,7 @@ int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> if (length > blocksize - offset)
> length = blocksize - offset;
>
> - err = ext4_block_zero_page_range(handle, inode->i_mapping, from, length,
> - &did_zero);
> + err = ext4_block_zero_range(handle, inode, from, length, &did_zero);
> if (err)
> return err;
>
> @@ -4210,7 +4207,6 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> loff_t lstart, loff_t length)
> {
> struct super_block *sb = inode->i_sb;
> - struct address_space *mapping = inode->i_mapping;
> unsigned partial_start, partial_end;
> ext4_fsblk_t start, end;
> loff_t byte_end = (lstart + length - 1);
> @@ -4225,22 +4221,22 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> /* Handle partial zero within the single block */
> if (start == end &&
> (partial_start || (partial_end != sb->s_blocksize - 1))) {
> - err = ext4_block_zero_page_range(handle, mapping,
> - lstart, length, NULL);
> + err = ext4_block_zero_range(handle, inode, lstart,
> + length, NULL);
> return err;
> }
> /* Handle partial zero out on the start of the range */
> if (partial_start) {
> - err = ext4_block_zero_page_range(handle, mapping, lstart,
> - sb->s_blocksize, NULL);
> + err = ext4_block_zero_range(handle, inode, lstart,
> + sb->s_blocksize, 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_page_range(handle, mapping,
> - byte_end - partial_end,
> - partial_end + 1, NULL);
> + err = ext4_block_zero_range(handle, inode,
> + byte_end - partial_end,
> + partial_end + 1, NULL);
> return err;
> }
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 04/10] ext4: factor out journalled block zeroing range
From: Jan Kara @ 2026-03-23 16:48 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: <20260310014101.4140698-5-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:55, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Refactor __ext4_block_zero_page_range() by separating the block zeroing
> operations for ordered data mode and journal data mode into two distinct
> functions:
>
> - ext4_block_do_zero_range(): handles non-journal data mode with
> ordered data support
> - ext4_block_journalled_zero_range(): handles journal data mode
>
> Also extract a common helper, ext4_block_get_zero_range(), to handle
> buffer head and folio retrieval, along with the associated error
> handling. This prepares for converting the partial block zero range to
> the iomap infrastructure.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Just one nit below. Otherwise feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 52c6a86ad9f9..d63d455831b9 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4002,13 +4002,12 @@ void ext4_set_aops(struct inode *inode)
> * ext4_punch_hole, etc) which needs to be properly zeroed out. Otherwise a
> * racing writeback can come later and flush the stale pagecache to disk.
> */
> -static int __ext4_block_zero_page_range(handle_t *handle,
> - struct address_space *mapping, loff_t from, loff_t length,
> - bool *did_zero)
> +static struct buffer_head *ext4_block_get_zero_range(struct inode *inode,
> + loff_t from, loff_t length)
I'd call this function ext4_load_tail_bh() and AFAICT the 'length' argument
is unused so it can be dropped.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 3/3] ext4: derive f_fsid from block device to avoid collisions
From: Darrick J. Wong @ 2026-03-23 16:44 UTC (permalink / raw)
To: Theodore Tso; +Cc: Anand Jain, linux-ext4, linux-btrfs, linux-xfs, hch
In-Reply-To: <20260323152943.GH6223@frogsfrogsfrogs>
On Mon, Mar 23, 2026 at 08:29:43AM -0700, Darrick J. Wong wrote:
> On Sun, Mar 22, 2026 at 11:16:24PM -0500, Theodore Tso wrote:
> > On Sat, Mar 21, 2026 at 07:55:19PM +0800, Anand Jain wrote:
> > > statfs() currently reports f_fsid derived from the on-disk UUID.
> > > Cloned block devices share the same UUID, so distinct ext4 instances
> > > can return identical f_fsid values. This leads to collisions in
> > > fanotify.
> > >
> > > Encode sb->s_dev into f_fsid instead of using the superblock UUID.
> > > This provides a per-device identifier and avoids conflicts when
> > > filesystem is cloned, matching the behavior with xfs.
> >
> > As I observed in [1] this leads to collisions when for removable block
> > devices which can be used to mount different file systems.
> >
> > [1] https://lore.kernel.org/all/20260322203151.GA98947@mac.lan/
> >
> > > Place this change behind the new mount option "-o nouuid" for ABI
> > > compatibility.
> >
> > I *really* hate this mount option. It's not at all obvious what it
> > means for a system administrator who hasn't had the context of reading
> > the e-mail discussion on this subject.
>
> I don't love 'nouuid' either, because it means something completely
> different in XFS. 'fsid_from_dev' or something would at least be
> clearer about what it's doing...
>
> > As I stated in [1], I think the f_fsid is a terrible interface that
> > was promulgated by history, and future usage should be strongly
> > discouraged, and the wise programmer won't use it because it has
> > significant compatibility issues.
> >
> > As such, my personal preference is that we not try to condition it on
> > a mount option, which in all likelihood almost no one will use, and
> > instead just change it so that we hash the file system's UUID and
> > block device number together and use that for ext4's f_fsid.
>
> ...but why not just set fsid to some approximation of the dev_t like
> XFS and be done with it?
>
> st->f_fsid = u64_to_fsid(huge_encode_dev(mp->m_ddev_targp->bt_dev))
>
> There are a few other single-bdev filesystems that do this.
/me reads the rest of both threads, realizes the answer is "because you
can eject media, mount another media with the same fs, and bizarrely
they produce the same fsid."
I'll point out that for the filesystems that base fsid off of a block
device, perhaps we should mix in the diskseq to prevent that problem?
But then someone said the magic words "IMA and fanotify also use fsid"
and /me backs away from yet another opportunity to argue about
open-coded file handles from legacy Unix.
--D
^ permalink raw reply
* Re: [PATCH 03/10] ext4: rename and extend ext4_block_truncate_page()
From: Jan Kara @ 2026-03-23 16:42 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: <20260310014101.4140698-4-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:54, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Rename ext4_block_truncate_page() to ext4_block_zero_eof() and extend
> its signature to accept an explicit 'end' offset instead of calculating
> the block boundary. This helper function now can replace all cases
> requiring zeroing of the partial EOF block, including the append
> buffered write paths in ext4_*_write_end().
>
> 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/ext4.h | 2 ++
> fs/ext4/extents.c | 4 ++--
> fs/ext4/inode.c | 43 +++++++++++++++++++++++--------------------
> 3 files changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 293f698b7042..c62459ef9796 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3099,6 +3099,8 @@ extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
> extern int ext4_chunk_trans_extent(struct inode *inode, int nrblocks);
> extern int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
> int pextents);
> +extern int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> + loff_t from, loff_t end);
> extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> loff_t lstart, loff_t lend);
> extern vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ae3804f36535..a265070c1b79 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4625,8 +4625,8 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> inode_get_ctime(inode));
> if (epos > old_size) {
> pagecache_isize_extended(inode, old_size, epos);
> - ext4_zero_partial_blocks(handle, inode,
> - old_size, epos - old_size);
> + ext4_block_zero_eof(handle, inode, old_size,
> + epos);
> }
> }
> ret2 = ext4_mark_inode_dirty(handle, inode);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index a737ce05e768..52c6a86ad9f9 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1458,7 +1458,7 @@ static int ext4_write_end(const struct kiocb *iocb,
>
> if (old_size < pos && !verity) {
> pagecache_isize_extended(inode, old_size, pos);
> - ext4_zero_partial_blocks(handle, inode, old_size, pos - old_size);
> + ext4_block_zero_eof(handle, inode, old_size, pos);
> }
> /*
> * Don't mark the inode dirty under folio lock. First, it unnecessarily
> @@ -1576,7 +1576,7 @@ static int ext4_journalled_write_end(const struct kiocb *iocb,
>
> if (old_size < pos && !verity) {
> pagecache_isize_extended(inode, old_size, pos);
> - ext4_zero_partial_blocks(handle, inode, old_size, pos - old_size);
> + ext4_block_zero_eof(handle, inode, old_size, pos);
> }
>
> if (size_changed) {
> @@ -3252,7 +3252,7 @@ static int ext4_da_do_write_end(struct address_space *mapping,
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> if (zero_len)
> - ext4_zero_partial_blocks(handle, inode, old_size, zero_len);
> + ext4_block_zero_eof(handle, inode, old_size, pos);
> ext4_mark_inode_dirty(handle, inode);
> ext4_journal_stop(handle);
>
> @@ -4132,29 +4132,32 @@ static int ext4_block_zero_page_range(handle_t *handle,
> }
>
> /*
> - * ext4_block_truncate_page() zeroes out a mapping from file offset `from'
> - * up to the end of the block which corresponds to `from'.
> - * This required during truncate. We need to physically zero the tail end
> - * of that block so it doesn't yield old data if the file is later grown.
> - * Return the zeroed length on success.
> + * Zero out a mapping from file offset 'from' up to the end of the block
> + * which corresponds to 'from' or to the given 'end' inside this block.
> + * This required during truncate up and performing append writes. We need
> + * to physically zero the tail end of that block so it doesn't yield old
> + * data if the file is grown. Return the zeroed length on success.
> */
> -static int ext4_block_truncate_page(handle_t *handle,
> - struct address_space *mapping, loff_t from)
> +int ext4_block_zero_eof(handle_t *handle, struct inode *inode,
> + loff_t from, loff_t end)
> {
> - unsigned length;
> - unsigned blocksize;
> - struct inode *inode = mapping->host;
> + unsigned int blocksize = i_blocksize(inode);
> + unsigned int offset;
> + loff_t length = end - from;
> bool did_zero = false;
> int err;
>
> + offset = from & (blocksize - 1);
> + if (!offset || from >= end)
> + return 0;
> /* If we are processing an encrypted inode during orphan list handling */
> if (IS_ENCRYPTED(inode) && !fscrypt_has_encryption_key(inode))
> return 0;
>
> - blocksize = i_blocksize(inode);
> - length = blocksize - (from & (blocksize - 1));
> + if (length > blocksize - offset)
> + length = blocksize - offset;
>
> - err = ext4_block_zero_page_range(handle, mapping, from, length,
> + err = ext4_block_zero_page_range(handle, inode->i_mapping, from, length,
> &did_zero);
> if (err)
> return err;
> @@ -4508,7 +4511,6 @@ int ext4_truncate(struct inode *inode)
> unsigned int credits;
> int err = 0, err2;
> handle_t *handle;
> - struct address_space *mapping = inode->i_mapping;
>
> /*
> * There is a possibility that we're either freeing the inode
> @@ -4551,8 +4553,9 @@ int ext4_truncate(struct inode *inode)
> goto out_trace;
> }
>
> + /* Zero to the end of the block containing i_size */
> if (inode->i_size & (inode->i_sb->s_blocksize - 1))
> - ext4_block_truncate_page(handle, mapping, inode->i_size);
> + ext4_block_zero_eof(handle, inode, inode->i_size, LLONG_MAX);
>
> /*
> * We add the inode to the orphan list, so that if this
> @@ -5911,8 +5914,8 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> inode_set_mtime_to_ts(inode,
> inode_set_ctime_current(inode));
> if (oldsize & (inode->i_sb->s_blocksize - 1))
> - ext4_block_truncate_page(handle,
> - inode->i_mapping, oldsize);
> + ext4_block_zero_eof(handle, inode,
> + oldsize, LLONG_MAX);
> }
>
> if (shrink)
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 02/10] ext4: ext4_block_truncate_page() returns zeroed length on success
From: Jan Kara @ 2026-03-23 16:34 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: <20260310014101.4140698-3-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:53, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Return the actual zeroed length instead of 0 on success. This prepares
> for the upcoming iomap buffered I/O conversion by exposing zeroed length
> information to callers.
>
> 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/inode.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 1e037a314dab..a737ce05e768 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4136,6 +4136,7 @@ static int ext4_block_zero_page_range(handle_t *handle,
> * up to the end of the block which corresponds to `from'.
> * This required during truncate. We need to physically zero the tail end
> * of that block so it doesn't yield old data if the file is later grown.
> + * Return the zeroed length on success.
> */
> static int ext4_block_truncate_page(handle_t *handle,
> struct address_space *mapping, loff_t from)
> @@ -4143,6 +4144,8 @@ static int ext4_block_truncate_page(handle_t *handle,
> unsigned length;
> unsigned blocksize;
> struct inode *inode = mapping->host;
> + bool did_zero = false;
> + int err;
>
> /* If we are processing an encrypted inode during orphan list handling */
> if (IS_ENCRYPTED(inode) && !fscrypt_has_encryption_key(inode))
> @@ -4151,7 +4154,12 @@ static int ext4_block_truncate_page(handle_t *handle,
> blocksize = i_blocksize(inode);
> length = blocksize - (from & (blocksize - 1));
>
> - return ext4_block_zero_page_range(handle, mapping, from, length, NULL);
> + err = ext4_block_zero_page_range(handle, mapping, from, length,
> + &did_zero);
> + if (err)
> + return err;
> +
> + return did_zero ? length : 0;
> }
>
> int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 01/10] ext4: add did_zero output parameter to ext4_block_zero_page_range()
From: Jan Kara @ 2026-03-23 16:33 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: <20260310014101.4140698-2-yi.zhang@huaweicloud.com>
On Tue 10-03-26 09:40:52, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Add a bool *did_zero output parameter to ext4_block_zero_page_range()
> and __ext4_block_zero_page_range(). The parameter reports whether a
> partial block was zeroed out, which is needed for the upcoming iomap
> buffered I/O conversion.
>
> 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/inode.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 396dc3a5d16b..1e037a314dab 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4003,7 +4003,8 @@ void ext4_set_aops(struct inode *inode)
> * racing writeback can come later and flush the stale pagecache to disk.
> */
> static int __ext4_block_zero_page_range(handle_t *handle,
> - struct address_space *mapping, loff_t from, loff_t length)
> + struct address_space *mapping, loff_t from, loff_t length,
> + bool *did_zero)
> {
> unsigned int offset, blocksize, pos;
> ext4_lblk_t iblock;
> @@ -4091,6 +4092,8 @@ static int __ext4_block_zero_page_range(handle_t *handle,
> err = ext4_jbd2_inode_add_write(handle, inode, from,
> length);
> }
> + if (!err && did_zero)
> + *did_zero = true;
>
> unlock:
> folio_unlock(folio);
> @@ -4106,7 +4109,8 @@ static int __ext4_block_zero_page_range(handle_t *handle,
> * that corresponds to 'from'
> */
> static int ext4_block_zero_page_range(handle_t *handle,
> - struct address_space *mapping, loff_t from, loff_t length)
> + struct address_space *mapping, loff_t from, loff_t length,
> + bool *did_zero)
> {
> struct inode *inode = mapping->host;
> unsigned blocksize = inode->i_sb->s_blocksize;
> @@ -4120,10 +4124,11 @@ static int ext4_block_zero_page_range(handle_t *handle,
> length = max;
>
> if (IS_DAX(inode)) {
> - return dax_zero_range(inode, from, length, NULL,
> + return dax_zero_range(inode, from, length, did_zero,
> &ext4_iomap_ops);
> }
> - return __ext4_block_zero_page_range(handle, mapping, from, length);
> + return __ext4_block_zero_page_range(handle, mapping, from, length,
> + did_zero);
> }
>
> /*
> @@ -4146,7 +4151,7 @@ static int ext4_block_truncate_page(handle_t *handle,
> blocksize = i_blocksize(inode);
> length = blocksize - (from & (blocksize - 1));
>
> - return ext4_block_zero_page_range(handle, mapping, from, length);
> + return ext4_block_zero_page_range(handle, mapping, from, length, NULL);
> }
>
> int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> @@ -4169,13 +4174,13 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> if (start == end &&
> (partial_start || (partial_end != sb->s_blocksize - 1))) {
> err = ext4_block_zero_page_range(handle, mapping,
> - lstart, length);
> + lstart, length, NULL);
> return err;
> }
> /* Handle partial zero out on the start of the range */
> if (partial_start) {
> - err = ext4_block_zero_page_range(handle, mapping,
> - lstart, sb->s_blocksize);
> + err = ext4_block_zero_page_range(handle, mapping, lstart,
> + sb->s_blocksize, NULL);
> if (err)
> return err;
> }
> @@ -4183,7 +4188,7 @@ int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> if (partial_end != sb->s_blocksize - 1)
> err = ext4_block_zero_page_range(handle, mapping,
> byte_end - partial_end,
> - partial_end + 1);
> + partial_end + 1, NULL);
> return err;
> }
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 3/3] ext4: derive f_fsid from block device to avoid collisions
From: Anand Jain @ 2026-03-23 15:41 UTC (permalink / raw)
To: Theodore Tso
Cc: linux-ext4, linux-btrfs, linux-xfs, Christoph Hellwig, Anand Jain
In-Reply-To: <20260323041624.GA11453@mac.lan>
Thanks for the feedback. I'll try to address the points raised
here and in your earlier email [1].
[1] https://lore.kernel.org/all/20260322203151.GA98947@mac.lan/
This work originally came out of a Btrfs issue where a cloned
filesystem ended up using a dynamically generated, mount-time
UUID for sb->s_uuid instead of the on-disk UUID. As a result,
OverlayFS (with index enabled) started failing mount-recycle
tests [2] for the cloned filesystem.
[2]
https://lore.kernel.org/lkml/20251014015707.129013-1-andrealmeid@igalia.com/
While looking into that problem, I also noticed that different
filesystems derive f_fsid in inconsistent ways, and in practice
many of them base it on dev_t.
On the question of the 64-bit limit: although a 64-bit value
is not globally unique in the way a 128-bit UUID is, f_fsid
has historically been derived from dev_t. Since dev_t must be
unique within a running kernel instance, 64 bits are enough to
safely encode its effective ~32-bit dev_t without collisions.
The number of concurrently addressable block devices is also
bounded by the 12-bit major / 20-bit minor limits and
/proc/sys/fs/mount-max. IMO, within a single boot, 64 bits
should provide a collision-free identifier.
I've also submitted new test cases that validate expectations
around both sb->s_uuid and statfs::f_fsid here [3].
[3] https://lore.kernel.org/fstests/cover.1774090817.git.asj@kernel.org/
> As I observed in [1] this leads to collisions when for removable block
> devices which can be used to mount different file systems.
>
> [1] https://lore.kernel.org/all/20260322203151.GA98947@mac.lan/
I agree. A straightforward f_fsid = f(dev_t) will collide if a
removable device is swapped but ends up reusing the same dev_t.
Theoretically, I see this can be reproduced with XFS, and with
my current patchset on Ext4. That’s clearly a blocker, and I plan
to revise, btw Btrfs does well for this test scenario.
> And even as you've proposed to change
> things, it's not consistent across file systems. In particular, your
> proposed solution mixes s_uuid into btrfs-patched, but not
> ext4-patched. Why?
The discrepancy exists because Btrfs must distinguish subvolume
mounts as separate logical entities. For Btrfs, the derivation
requires f(s_uuid, root_id, dev_t) to ensure that two different
subvolumes on the same device report distinct f_fsid values.
For Ext4, a simpler f(s_uuid, dev_t) should suffice to ensure
both cross-device uniqueness and persistent across media swaps.
>> Place this change behind the new mount option "-o nouuid" for ABI
>> compatibility.
>
> I *really* hate this mount option. It's not at all obvious what it
> means for a system administrator who hasn't had the context of reading
> the e-mail discussion on this subject.
>
> As I stated in [1], I think the f_fsid is a terrible interface that
> was promulgated by history, and future usage should be strongly
> discouraged, and the wise programmer won't use it because it has
> significant compatibility issues.
>
> As such, my personal preference is that we not try to condition it on
> a mount option, which in all likelihood almost no one will use, and
> instead just change it so that we hash the file system's UUID and
> block device number together and use that for ext4's f_fsid.
The decision to gate this behind a mount option followed feedback
from Christoph Hellwig. The concern is binary compatibility:
applications that manually derive an ID based on existing behavior
might break if the kernel changes its derivation logic.
I agree that -o nouuid is a poor name. If we must keep the mount
option for ABI stability, I am open to better nomenclature.
If we agree that f_fsid is already a problematic interface and
should simply be fixed without any special options for example by
always hashing the filesystem UUID together with the block device
number for Ext4, that would be my preference.
Thanks, Anand
^ permalink raw reply
* Re: [PATCH v2 3/3] ext4: derive f_fsid from block device to avoid collisions
From: Darrick J. Wong @ 2026-03-23 15:29 UTC (permalink / raw)
To: Theodore Tso; +Cc: Anand Jain, linux-ext4, linux-btrfs, linux-xfs, hch
In-Reply-To: <20260323041624.GA11453@mac.lan>
On Sun, Mar 22, 2026 at 11:16:24PM -0500, Theodore Tso wrote:
> On Sat, Mar 21, 2026 at 07:55:19PM +0800, Anand Jain wrote:
> > statfs() currently reports f_fsid derived from the on-disk UUID.
> > Cloned block devices share the same UUID, so distinct ext4 instances
> > can return identical f_fsid values. This leads to collisions in
> > fanotify.
> >
> > Encode sb->s_dev into f_fsid instead of using the superblock UUID.
> > This provides a per-device identifier and avoids conflicts when
> > filesystem is cloned, matching the behavior with xfs.
>
> As I observed in [1] this leads to collisions when for removable block
> devices which can be used to mount different file systems.
>
> [1] https://lore.kernel.org/all/20260322203151.GA98947@mac.lan/
>
> > Place this change behind the new mount option "-o nouuid" for ABI
> > compatibility.
>
> I *really* hate this mount option. It's not at all obvious what it
> means for a system administrator who hasn't had the context of reading
> the e-mail discussion on this subject.
I don't love 'nouuid' either, because it means something completely
different in XFS. 'fsid_from_dev' or something would at least be
clearer about what it's doing...
> As I stated in [1], I think the f_fsid is a terrible interface that
> was promulgated by history, and future usage should be strongly
> discouraged, and the wise programmer won't use it because it has
> significant compatibility issues.
>
> As such, my personal preference is that we not try to condition it on
> a mount option, which in all likelihood almost no one will use, and
> instead just change it so that we hash the file system's UUID and
> block device number together and use that for ext4's f_fsid.
...but why not just set fsid to some approximation of the dev_t like
XFS and be done with it?
st->f_fsid = u64_to_fsid(huge_encode_dev(mp->m_ddev_targp->bt_dev))
There are a few other single-bdev filesystems that do this.
--D
> Thoughts, comments?
>
> - Ted
>
^ permalink raw reply
* Re: [PATCH] ext4: skip split extent recovery on corruption
From: Zhang Yi @ 2026-03-23 12:12 UTC (permalink / raw)
To: hongao, tytso, adilger.kernel, jack, ojaswin
Cc: linux-ext4, linux-kernel, syzbot+1ffa5d865557e51cb604
In-Reply-To: <FA074540F4E30F27+20260323015752.60064-1-hongao@uniontech.com>
On 3/23/2026 9:57 AM, hongao wrote:
> ext4_split_extent_at() retries after ext4_ext_insert_extent() fails by
> refinding the original extent and restoring its length. That recovery is
> only safe for transient resource failures such as -ENOSPC, -EDQUOT, and
> -ENOMEM.
>
> When ext4_ext_insert_extent() fails because the extent tree is already
> corrupted, ext4_find_extent() can return a leaf path without p_ext.
> ext4_split_extent_at() then dereferences path[depth].p_ext while trying to
> fix up the original extent length, causing a NULL pointer dereference while
> handling a pre-existing filesystem corruption.
>
> Do not enter the recovery path for corruption errors, and validate p_ext
> after refinding the extent before touching it. This keeps the recovery path
> limited to cases it can actually repair and turns the syzbot-triggered crash
> into a proper corruption report.
>
> Fixes: 716b9c23b862 ("ext4: refactor split and convert extents")
> Reported-by: syzbot+1ffa5d865557e51cb604@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1ffa5d865557e51cb604
> Signed-off-by: hongao <hongao@uniontech.com>
Thank you for the patch, this is a nice catch. This overall looks good
to me besides one minor suggestion below.
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ae3804f36535..aba9947fd151 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3239,6 +3239,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>
> insert_err = PTR_ERR(path);
> err = 0;
> + if (insert_err != -ENOSPC && insert_err != -EDQUOT &&
> + insert_err != -ENOMEM)
> + goto out_path;
>
> /*
> * Get a new path to try to zeroout or fix the extent length.
> @@ -3261,6 +3264,13 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>
> depth = ext_depth(inode);
> ex = path[depth].p_ext;
> + if (!ex) {
> + EXT4_ERROR_INODE(inode,
> + "bad extent address lblock: %lu, depth: %d pblock %lld",
> + (unsigned long)ee_block, depth, path[depth].p_block);
> + err = -EFSCORRUPTED;
> + goto out;
> + }
Should we move the entire code snippet before the previous
ext4_ext_get_access() call? Although this is not closely related to the
current patch.
>
> fix_extent_len:
> ex->ee_len = orig_ex.ee_len;
^ permalink raw reply
* Re: [PATCH] ext4: fix iloc.bh leak in ext4_fc_replay_inode() error paths
From: Zhang Yi @ 2026-03-23 11:05 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ojaswin, ritesh.list, Joseph Qi
In-Reply-To: <20260323060836.3452660-1-libaokun@linux.alibaba.com>
On 3/23/2026 2:08 PM, Baokun Li wrote:
> During code review, Joseph found that ext4_fc_replay_inode() calls
> ext4_get_fc_inode_loc() to get the inode location, which holds a
> reference to iloc.bh that must be released via brelse().
>
> However, several error paths jump to the 'out' label without
> releasing iloc.bh:
>
> - ext4_handle_dirty_metadata() failure
> - sync_dirty_buffer() failure
> - ext4_mark_inode_used() failure
> - ext4_iget() failure
>
> Fix this by introducing an 'out_brelse' label placed just before
> the existing 'out' label to ensure iloc.bh is always released.
>
> Additionally, make ext4_fc_replay_inode() propagate errors
> properly instead of always returning 0.
>
> Reported-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Looks good to me.
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> ---
> fs/ext4/fast_commit.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index f575751f1cae..f1df5581fa96 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1613,19 +1613,21 @@ static int ext4_fc_replay_inode(struct super_block *sb,
> /* Immediately update the inode on disk. */
> ret = ext4_handle_dirty_metadata(NULL, NULL, iloc.bh);
> if (ret)
> - goto out;
> + goto out_brelse;
> ret = sync_dirty_buffer(iloc.bh);
> if (ret)
> - goto out;
> + goto out_brelse;
> ret = ext4_mark_inode_used(sb, ino);
> if (ret)
> - goto out;
> + goto out_brelse;
>
> /* Given that we just wrote the inode on disk, this SHOULD succeed. */
> inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL);
> if (IS_ERR(inode)) {
> ext4_debug("Inode not found.");
> - return -EFSCORRUPTED;
> + inode = NULL;
> + ret = -EFSCORRUPTED;
> + goto out_brelse;
> }
>
> /*
> @@ -1642,13 +1644,14 @@ static int ext4_fc_replay_inode(struct super_block *sb,
> ext4_inode_csum_set(inode, ext4_raw_inode(&iloc), EXT4_I(inode));
> ret = ext4_handle_dirty_metadata(NULL, NULL, iloc.bh);
> sync_dirty_buffer(iloc.bh);
> +out_brelse:
> brelse(iloc.bh);
> out:
> iput(inode);
> if (!ret)
> blkdev_issue_flush(sb->s_bdev);
>
> - return 0;
> + return ret;
> }
>
> /*
^ permalink raw reply
* Re: [PATCH 04/41] ocfs2: Drop pointless sync_mapping_buffers() calls
From: Joseph Qi @ 2026-03-23 10:46 UTC (permalink / raw)
To: Jan Kara, linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Joel Becker, ocfs2-devel
In-Reply-To: <20260320134100.20731-45-jack@suse.cz>
On 3/20/26 9:40 PM, Jan Kara wrote:
> ocfs2 never calls mark_buffer_dirty_inode() and thus its metadata
> buffers list is always empty. Drop the pointless sync_mapping_buffers()
> calls.
>
> CC: Joel Becker <jlbec@evilplan.org>
> CC: Joseph Qi <joseph.qi@linux.alibaba.com>
> CC: ocfs2-devel@lists.linux.dev
> Signed-off-by: Jan Kara <jack@suse.cz>
Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> fs/ocfs2/dlmglue.c | 1 -
> fs/ocfs2/namei.c | 3 ---
> 2 files changed, 4 deletions(-)
>
> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index bd2ddb7d841d..7283bb2c5a31 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -3971,7 +3971,6 @@ static int ocfs2_data_convert_worker(struct ocfs2_lock_res *lockres,
> mlog(ML_ERROR, "Could not sync inode %llu for downconvert!",
> (unsigned long long)OCFS2_I(inode)->ip_blkno);
> }
> - sync_mapping_buffers(mapping);
> if (blocking == DLM_LOCK_EX) {
> truncate_inode_pages(mapping, 0);
> } else {
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 268b79339a51..1277666c77cd 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -1683,9 +1683,6 @@ static int ocfs2_rename(struct mnt_idmap *idmap,
> if (rename_lock)
> ocfs2_rename_unlock(osb);
>
> - if (new_inode)
> - sync_mapping_buffers(old_inode->i_mapping);
> -
> iput(new_inode);
>
> ocfs2_free_dir_lookup_result(&target_lookup_res);
^ permalink raw reply
* Re: [PATCH v2 0/41] fs: Move metadata bh tracking from address_space
From: Christian Brauner @ 2026-03-23 10:20 UTC (permalink / raw)
To: linux-fsdevel, Jan Kara
Cc: Christian Brauner, linux-block, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
On Fri, 20 Mar 2026 14:40:55 +0100, Jan Kara wrote:
> here is a next revision of the patchset cleaning up buffer head metadata
> tracking and use of address_space's private_list and private_lock. The patches
> have survived some testing with fstests and ltp however I didn't test AFFS and
> KVM guest_memfd changes so a help with testing those would be very welcome.
> Thanks.
>
> Changes since v1:
> * Fixed hugetlbfs handling of root directory
> * Reworked mapping_metadata_bhs handling functions to get the tracking
> structure as an argument so we now don't need iops method to fetch the struct
> from the inode
> * Reordered patches into more sensible order
> * Added patch to merge two mostly duplicate generic fsync implementations
> * Added Reviewed-by tags
> * Couple more minor changes that were requested during review
>
> [...]
x86_64 (gcc, debian, ovl-fstests) pass
x86_64 (gcc, debian, selftests) pass
x86_64 (gcc, debian, xfstests) pass
x86_64 (gcc, fedora, ovl-fstests) pass
x86_64 (gcc, fedora, selftests) pass
x86_64 (gcc, fedora, xfstests) pass
---
Applied to the vfs-7.1.bh.metadata branch of the vfs/vfs.git tree.
Patches in the vfs-7.1.bh.metadata branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.1.bh.metadata
[01/41] ext4: Use inode_has_buffers()
https://git.kernel.org/vfs/vfs/c/2caab145b54f
[02/41] gfs2: Don't zero i_private_data
https://git.kernel.org/vfs/vfs/c/8395055f2455
[03/41] ntfs3: Drop pointless sync_mapping_buffers() and invalidate_inode_buffers() calls
https://git.kernel.org/vfs/vfs/c/bed1ecade645
[04/41] ocfs2: Drop pointless sync_mapping_buffers() calls
https://git.kernel.org/vfs/vfs/c/c40f470d21ae
[05/41] bdev: Drop pointless invalidate_inode_buffers() call
https://git.kernel.org/vfs/vfs/c/4e001046c8a6
[06/41] ufs: Drop pointless invalidate_mapping_buffers() call
https://git.kernel.org/vfs/vfs/c/26d88dcdb54b
[07/41] exfat: Drop pointless invalidate_inode_buffers() call
https://git.kernel.org/vfs/vfs/c/8dbad3a0e39a
[08/41] udf: Switch to generic_buffers_fsync()
https://git.kernel.org/vfs/vfs/c/0892d39092b3
[09/41] minix: Switch to generic_buffers_fsync()
https://git.kernel.org/vfs/vfs/c/387a7a22307e
[10/41] bfs: Switch to generic_buffers_fsync()
https://git.kernel.org/vfs/vfs/c/a59c3be58777
[11/41] fat: Switch to generic_buffers_fsync_noflush()
https://git.kernel.org/vfs/vfs/c/e118e65dba18
[12/41] fs: Drop sync_mapping_buffers() from __generic_file_fsync()
https://git.kernel.org/vfs/vfs/c/0bdc542b3faa
[13/41] fat: Sync and invalidate metadata buffers from fat_evict_inode()
https://git.kernel.org/vfs/vfs/c/f2145333cd91
[14/41] udf: Sync and invalidate metadata buffers from udf_evict_inode()
https://git.kernel.org/vfs/vfs/c/525acf32a4ad
[15/41] minix: Sync and invalidate metadata buffers from minix_evict_inode()
https://git.kernel.org/vfs/vfs/c/60ef3750f238
[16/41] ext2: Sync and invalidate metadata buffers from ext2_evict_inode()
https://git.kernel.org/vfs/vfs/c/52e995b1474d
[17/41] ext4: Sync and invalidate metadata buffers from ext4_evict_inode()
https://git.kernel.org/vfs/vfs/c/36dc7f23446b
[18/41] bfs: Sync and invalidate metadata buffers from bfs_evict_inode()
https://git.kernel.org/vfs/vfs/c/aa2caecd2b38
[19/41] affs: Sync and invalidate metadata buffers from affs_evict_inode()
https://git.kernel.org/vfs/vfs/c/2779c362a490
[20/41] fs: Ignore inode metadata buffers in inode_lru_isolate()
https://git.kernel.org/vfs/vfs/c/95c6bfdb5d3e
[21/41] fs: Stop using i_private_data for metadata bh tracking
https://git.kernel.org/vfs/vfs/c/89f2eea7f6c3
[22/41] hugetlbfs: Stop using i_private_data
https://git.kernel.org/vfs/vfs/c/8f3bf5b0ce4e
[23/41] aio: Stop using i_private_data and i_private_lock
https://git.kernel.org/vfs/vfs/c/8514c0d15c45
[24/41] fs: Remove i_private_data
https://git.kernel.org/vfs/vfs/c/aa7c3819d2db
[25/41] kvm: Use private inode list instead of i_private_list
https://git.kernel.org/vfs/vfs/c/1e0cbe5bac95
[26/41] fs: Drop osync_buffers_list()
https://git.kernel.org/vfs/vfs/c/f07ad1722cda
[27/41] fs: Fold fsync_buffers_list() into sync_mapping_buffers()
https://git.kernel.org/vfs/vfs/c/e7cd907f2326
[28/41] fs: Move metadata bhs tracking to a separate struct
https://git.kernel.org/vfs/vfs/c/a3b0a90f1e93
[29/41] fs: Make bhs point to mapping_metadata_bhs
https://git.kernel.org/vfs/vfs/c/d8b6d9ff9552
[30/41] fs: Switch inode_has_buffers() to take mapping_metadata_bhs
https://git.kernel.org/vfs/vfs/c/9e5f33d8201e
[31/41] fs: Provide functions for handling mapping_metadata_bhs directly
https://git.kernel.org/vfs/vfs/c/a13a480c81b1
[32/41] ext2: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/107b7505d866
[33/41] affs: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/592eacdd5928
[34/41] bfs: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/c6661db8efc0
[35/41] fat: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/b5d84862f99d
[36/41] udf: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/7d37ac2bba4c
[37/41] minix: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/37da66baf00c
[38/41] ext4: Track metadata bhs in fs-private inode part
https://git.kernel.org/vfs/vfs/c/ebcf10f6f905
[39/41] fs: Drop mapping_metadata_bhs from address space
https://git.kernel.org/vfs/vfs/c/ecfcd39c0ab0
[40/41] fs: Drop i_private_list from address_space
https://git.kernel.org/vfs/vfs/c/b39f532b7a2e
[41/41] fs: Unify generic_file_fsync() with mmb methods
https://git.kernel.org/vfs/vfs/c/24b45fa837a4
^ permalink raw reply
* Re: [PATCH] ext4: fix iloc.bh leak in ext4_fc_replay_inode() error paths
From: Jan Kara @ 2026-03-23 10:08 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, Joseph Qi
In-Reply-To: <20260323060836.3452660-1-libaokun@linux.alibaba.com>
On Mon 23-03-26 14:08:36, Baokun Li wrote:
> During code review, Joseph found that ext4_fc_replay_inode() calls
> ext4_get_fc_inode_loc() to get the inode location, which holds a
> reference to iloc.bh that must be released via brelse().
>
> However, several error paths jump to the 'out' label without
> releasing iloc.bh:
>
> - ext4_handle_dirty_metadata() failure
> - sync_dirty_buffer() failure
> - ext4_mark_inode_used() failure
> - ext4_iget() failure
>
> Fix this by introducing an 'out_brelse' label placed just before
> the existing 'out' label to ensure iloc.bh is always released.
>
> Additionally, make ext4_fc_replay_inode() propagate errors
> properly instead of always returning 0.
>
> Reported-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/fast_commit.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index f575751f1cae..f1df5581fa96 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1613,19 +1613,21 @@ static int ext4_fc_replay_inode(struct super_block *sb,
> /* Immediately update the inode on disk. */
> ret = ext4_handle_dirty_metadata(NULL, NULL, iloc.bh);
> if (ret)
> - goto out;
> + goto out_brelse;
> ret = sync_dirty_buffer(iloc.bh);
> if (ret)
> - goto out;
> + goto out_brelse;
> ret = ext4_mark_inode_used(sb, ino);
> if (ret)
> - goto out;
> + goto out_brelse;
>
> /* Given that we just wrote the inode on disk, this SHOULD succeed. */
> inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL);
> if (IS_ERR(inode)) {
> ext4_debug("Inode not found.");
> - return -EFSCORRUPTED;
> + inode = NULL;
> + ret = -EFSCORRUPTED;
> + goto out_brelse;
> }
>
> /*
> @@ -1642,13 +1644,14 @@ static int ext4_fc_replay_inode(struct super_block *sb,
> ext4_inode_csum_set(inode, ext4_raw_inode(&iloc), EXT4_I(inode));
> ret = ext4_handle_dirty_metadata(NULL, NULL, iloc.bh);
> sync_dirty_buffer(iloc.bh);
> +out_brelse:
> brelse(iloc.bh);
> out:
> iput(inode);
> if (!ret)
> blkdev_issue_flush(sb->s_bdev);
>
> - return 0;
> + return ret;
> }
>
> /*
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] ext4: skip split extent recovery on corruption
From: Jan Kara @ 2026-03-23 10:02 UTC (permalink / raw)
To: hongao
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, linux-ext4,
linux-kernel, syzbot+1ffa5d865557e51cb604
In-Reply-To: <FA074540F4E30F27+20260323015752.60064-1-hongao@uniontech.com>
On Mon 23-03-26 09:57:52, hongao wrote:
> ext4_split_extent_at() retries after ext4_ext_insert_extent() fails by
> refinding the original extent and restoring its length. That recovery is
> only safe for transient resource failures such as -ENOSPC, -EDQUOT, and
> -ENOMEM.
>
> When ext4_ext_insert_extent() fails because the extent tree is already
> corrupted, ext4_find_extent() can return a leaf path without p_ext.
> ext4_split_extent_at() then dereferences path[depth].p_ext while trying to
> fix up the original extent length, causing a NULL pointer dereference while
> handling a pre-existing filesystem corruption.
>
> Do not enter the recovery path for corruption errors, and validate p_ext
> after refinding the extent before touching it. This keeps the recovery path
> limited to cases it can actually repair and turns the syzbot-triggered crash
> into a proper corruption report.
>
> Fixes: 716b9c23b862 ("ext4: refactor split and convert extents")
> Reported-by: syzbot+1ffa5d865557e51cb604@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1ffa5d865557e51cb604
> Signed-off-by: hongao <hongao@uniontech.com>
Thanks! The fix looks good to me! Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ae3804f36535..aba9947fd151 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3239,6 +3239,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>
> insert_err = PTR_ERR(path);
> err = 0;
> + if (insert_err != -ENOSPC && insert_err != -EDQUOT &&
> + insert_err != -ENOMEM)
> + goto out_path;
>
> /*
> * Get a new path to try to zeroout or fix the extent length.
> @@ -3261,6 +3264,13 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>
> depth = ext_depth(inode);
> ex = path[depth].p_ext;
> + if (!ex) {
> + EXT4_ERROR_INODE(inode,
> + "bad extent address lblock: %lu, depth: %d pblock %lld",
> + (unsigned long)ee_block, depth, path[depth].p_block);
> + err = -EFSCORRUPTED;
> + goto out;
> + }
>
> fix_extent_len:
> ex->ee_len = orig_ex.ee_len;
> --
> 2.51.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] ext4: skip split extent recovery on corruption
From: Ojaswin Mujoo @ 2026-03-23 9:44 UTC (permalink / raw)
To: hongao
Cc: tytso, adilger.kernel, jack, yi.zhang, linux-ext4, linux-kernel,
syzbot+1ffa5d865557e51cb604
In-Reply-To: <FA074540F4E30F27+20260323015752.60064-1-hongao@uniontech.com>
On Mon, Mar 23, 2026 at 09:57:52AM +0800, hongao wrote:
> ext4_split_extent_at() retries after ext4_ext_insert_extent() fails by
> refinding the original extent and restoring its length. That recovery is
> only safe for transient resource failures such as -ENOSPC, -EDQUOT, and
> -ENOMEM.
>
> When ext4_ext_insert_extent() fails because the extent tree is already
> corrupted, ext4_find_extent() can return a leaf path without p_ext.
> ext4_split_extent_at() then dereferences path[depth].p_ext while trying to
> fix up the original extent length, causing a NULL pointer dereference while
> handling a pre-existing filesystem corruption.
>
> Do not enter the recovery path for corruption errors, and validate p_ext
> after refinding the extent before touching it. This keeps the recovery path
> limited to cases it can actually repair and turns the syzbot-triggered crash
> into a proper corruption report.
>
> Fixes: 716b9c23b862 ("ext4: refactor split and convert extents")
> Reported-by: syzbot+1ffa5d865557e51cb604@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1ffa5d865557e51cb604
> Signed-off-by: hongao <hongao@uniontech.com>
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ae3804f36535..aba9947fd151 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3239,6 +3239,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>
> insert_err = PTR_ERR(path);
> err = 0;
> + if (insert_err != -ENOSPC && insert_err != -EDQUOT &&
> + insert_err != -ENOMEM)
> + goto out_path;
Hi Hongao,
Thanks for the fix. This makes sense to me since we anyways don't
zeroout for these cases and just return the error, the callers
should expect no guarantee that extent is in a stable state.
Are you able to replicate this issue btw as I don't see a syzbot
replicator for this.
>
> /*
> * Get a new path to try to zeroout or fix the extent length.
> @@ -3261,6 +3264,13 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>
> depth = ext_depth(inode);
> ex = path[depth].p_ext;
> + if (!ex) {
> + EXT4_ERROR_INODE(inode,
> + "bad extent address lblock: %lu, depth: %d pblock %lld",
> + (unsigned long)ee_block, depth, path[depth].p_block);
> + err = -EFSCORRUPTED;
> + goto out;
> + }
Looking at the syzbot report, it does seem this is possible, but looking
at ext4_ext_insert_extent() it is not immediately obvious. Do you know
what codepath can lead us to path being valid but p_ext being NULL.
Regards,
ojaswin
>
> fix_extent_len:
> ex->ee_len = orig_ex.ee_len;
> --
> 2.51.0
>
^ permalink raw reply
* [PATCH] ext4: fix iloc.bh leak in ext4_fc_replay_inode() error paths
From: Baokun Li @ 2026-03-23 6:08 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
libaokun, Joseph Qi
During code review, Joseph found that ext4_fc_replay_inode() calls
ext4_get_fc_inode_loc() to get the inode location, which holds a
reference to iloc.bh that must be released via brelse().
However, several error paths jump to the 'out' label without
releasing iloc.bh:
- ext4_handle_dirty_metadata() failure
- sync_dirty_buffer() failure
- ext4_mark_inode_used() failure
- ext4_iget() failure
Fix this by introducing an 'out_brelse' label placed just before
the existing 'out' label to ensure iloc.bh is always released.
Additionally, make ext4_fc_replay_inode() propagate errors
properly instead of always returning 0.
Reported-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/fast_commit.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index f575751f1cae..f1df5581fa96 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1613,19 +1613,21 @@ static int ext4_fc_replay_inode(struct super_block *sb,
/* Immediately update the inode on disk. */
ret = ext4_handle_dirty_metadata(NULL, NULL, iloc.bh);
if (ret)
- goto out;
+ goto out_brelse;
ret = sync_dirty_buffer(iloc.bh);
if (ret)
- goto out;
+ goto out_brelse;
ret = ext4_mark_inode_used(sb, ino);
if (ret)
- goto out;
+ goto out_brelse;
/* Given that we just wrote the inode on disk, this SHOULD succeed. */
inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL);
if (IS_ERR(inode)) {
ext4_debug("Inode not found.");
- return -EFSCORRUPTED;
+ inode = NULL;
+ ret = -EFSCORRUPTED;
+ goto out_brelse;
}
/*
@@ -1642,13 +1644,14 @@ static int ext4_fc_replay_inode(struct super_block *sb,
ext4_inode_csum_set(inode, ext4_raw_inode(&iloc), EXT4_I(inode));
ret = ext4_handle_dirty_metadata(NULL, NULL, iloc.bh);
sync_dirty_buffer(iloc.bh);
+out_brelse:
brelse(iloc.bh);
out:
iput(inode);
if (!ret)
blkdev_issue_flush(sb->s_bdev);
- return 0;
+ return ret;
}
/*
--
2.43.7
^ permalink raw reply related
* Re: [PATCH v2 3/3] ext4: derive f_fsid from block device to avoid collisions
From: Theodore Tso @ 2026-03-23 4:16 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-ext4, linux-btrfs, linux-xfs, hch
In-Reply-To: <33e8eb64c304a4d42b60f608c26497bf9a2e9e19.1774092915.git.asj@kernel.org>
On Sat, Mar 21, 2026 at 07:55:19PM +0800, Anand Jain wrote:
> statfs() currently reports f_fsid derived from the on-disk UUID.
> Cloned block devices share the same UUID, so distinct ext4 instances
> can return identical f_fsid values. This leads to collisions in
> fanotify.
>
> Encode sb->s_dev into f_fsid instead of using the superblock UUID.
> This provides a per-device identifier and avoids conflicts when
> filesystem is cloned, matching the behavior with xfs.
As I observed in [1] this leads to collisions when for removable block
devices which can be used to mount different file systems.
[1] https://lore.kernel.org/all/20260322203151.GA98947@mac.lan/
> Place this change behind the new mount option "-o nouuid" for ABI
> compatibility.
I *really* hate this mount option. It's not at all obvious what it
means for a system administrator who hasn't had the context of reading
the e-mail discussion on this subject.
As I stated in [1], I think the f_fsid is a terrible interface that
was promulgated by history, and future usage should be strongly
discouraged, and the wise programmer won't use it because it has
significant compatibility issues.
As such, my personal preference is that we not try to condition it on
a mount option, which in all likelihood almost no one will use, and
instead just change it so that we hash the file system's UUID and
block device number together and use that for ext4's f_fsid.
Thoughts, comments?
- Ted
^ permalink raw reply
* [PATCH] ext4: skip split extent recovery on corruption
From: hongao @ 2026-03-23 1:57 UTC (permalink / raw)
To: tytso, adilger.kernel, jack, yi.zhang, ojaswin
Cc: linux-ext4, linux-kernel, hongao, syzbot+1ffa5d865557e51cb604
ext4_split_extent_at() retries after ext4_ext_insert_extent() fails by
refinding the original extent and restoring its length. That recovery is
only safe for transient resource failures such as -ENOSPC, -EDQUOT, and
-ENOMEM.
When ext4_ext_insert_extent() fails because the extent tree is already
corrupted, ext4_find_extent() can return a leaf path without p_ext.
ext4_split_extent_at() then dereferences path[depth].p_ext while trying to
fix up the original extent length, causing a NULL pointer dereference while
handling a pre-existing filesystem corruption.
Do not enter the recovery path for corruption errors, and validate p_ext
after refinding the extent before touching it. This keeps the recovery path
limited to cases it can actually repair and turns the syzbot-triggered crash
into a proper corruption report.
Fixes: 716b9c23b862 ("ext4: refactor split and convert extents")
Reported-by: syzbot+1ffa5d865557e51cb604@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1ffa5d865557e51cb604
Signed-off-by: hongao <hongao@uniontech.com>
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ae3804f36535..aba9947fd151 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3239,6 +3239,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
insert_err = PTR_ERR(path);
err = 0;
+ if (insert_err != -ENOSPC && insert_err != -EDQUOT &&
+ insert_err != -ENOMEM)
+ goto out_path;
/*
* Get a new path to try to zeroout or fix the extent length.
@@ -3261,6 +3264,13 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
depth = ext_depth(inode);
ex = path[depth].p_ext;
+ if (!ex) {
+ EXT4_ERROR_INODE(inode,
+ "bad extent address lblock: %lu, depth: %d pblock %lld",
+ (unsigned long)ee_block, depth, path[depth].p_block);
+ err = -EFSCORRUPTED;
+ goto out;
+ }
fix_extent_len:
ex->ee_len = orig_ex.ee_len;
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 0/3] fix s_uuid and f_fsid consistency for cloned filesystems
From: Theodore Tso @ 2026-03-22 20:31 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, linux-ext4
In-Reply-To: <cover.1772095546.git.asj@kernel.org>
On Thu, Feb 26, 2026 at 10:23:32PM +0800, Anand Jain wrote:
>
> | s_uuid f_fsid
> --------------|---------------------------
> EXT4 | same same
> Btrfs | random random
> XFS | same f(devt)
> EXT4-patched | same f(devt)
> Btrfs-patched | same f(s_uuid,rootid,devt)
I don't *object* to changing ext4 reports since having something that
is unique is probably better. However, my bigger concern is using
f_fsid in the first place. It's only 64 bits, and that's really not
enough to gaurantee uniqueness. And even as you've proposed to change
things, it's not consistent across file systems. In particular, your
proposed solution mixes s_uuid into btrfs-patched, but not
ext4-patched. Why?
> Problem
> -------
> Btrfs currently never duplicates s_uuid or f_fsid for cloned filesystems.
> When an fsid collision is detected at mount time, btrfs generates a new
> in-memory fsid (temp_fsid), but this is ephemeral — it changes on every
> mount. This has two consequences:
>
> 1. IMA (Integrity Measurement Architecture) cannot reliably track the
> filesystem across mount-cycle, since the f_fsid it sees keeps changing.
> This does not scale. Whereas on the otherhand if you have same s_uuid
> on multiple filesystems, monitoring per distint filesystem is lost.
The problem with using f(dev_t) for IMA is that if you have a
removable device (e.g., an SD card), reporting f_fsid as purely being
a function of dev_t means that the if an SD card ejected, and replaced
with another, the fsid_t will manifestly *not* be unique. So in that
sense, replace f(dev_t) with f(s_uuid) would be worse if you think
"file system unique id" should be unique in the case of removable
storage devices.
If the audit log includes mounts and unmounts, then this might not be
fatal. But if less-than intelligent system administrator or LLM tries
to analyize an audit log using tools like "grep", it would be pretty
easy for someone to get misled.
I know you were primarily interested in cloned file systems, but I
think we also need to take into account other cases, including ones
where there might be more the file system associated with a block
device over time.
> 2. If we instead allow cloned filesystems to share the same f_fsid (as
> ext4 currently does), fanotify loses the ability to distinguish
> between distinct filesystem instances. FAN_EVENT_INFO_TYPE_FID events
> will fail to resolve to the correct mountpoint when f_fsid values
> are identical across clones.
My personal opinion is that f_fsuid is just a terrible interface, the
fact that IMA and fanotify used this is regrettable. I understand why
it happened, because there wasn't anything better, and for many use
cases, it's good enough. But not all.
So I hope we can just actively discourage anyone else using it. Given
that exactly it has not been standardized, across different operating
systems, and different file systems for Linux --- hopefully most
people will have already made that choice.
- Ted
^ permalink raw reply
* Re: [PATCH 2/9] fstests: add _mkfs_scratch_clone() helper
From: Anand Jain @ 2026-03-21 12:02 UTC (permalink / raw)
To: Christoph Hellwig, Zorro Lang
Cc: Anand Jain, fstests, linux-btrfs, linux-ext4, linux-xfs
In-Reply-To: <abgsorUy3KM8KZV5@infradead.org>
On 17/3/26 00:15, Christoph Hellwig wrote:
> On Sun, Mar 15, 2026 at 01:04:22AM +0800, Zorro Lang wrote:
>> I'm concerned that testers from other filesystems might not be familiar
>> with SCRATCH_DEV_POOL, as it's typically not a requirement for their
>> testing. I'm not sure if they would be willing to configure and switch
>> to SCRATCH_DEV_POOL, just for these few test cases. If we can avoid making
>> the pool to be a hard requirement, it might be more user-friendly for other
>> fs list. Welcome feedback and review points from other filesystem mailing
>> lists :)
>
> Yes, doing this using loop devices or ramdisk is going to get us much
> better coverage.
Agreed. Let's stick with SCRATCH_DEV compatibility for now.
v2 is in the ML.
Thanks.
^ permalink raw reply
* [PATCH v2 9/9] fstests: verify exportfs file handles on cloned filesystems
From: Anand Jain @ 2026-03-21 11:58 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, amir73il, zlang, hch
In-Reply-To: <cover.1774090817.git.asj@kernel.org>
Ensure that exportfs can correctly decode file handles on a cloned
filesystem across a mount cycle, by file handles generated on a
cloned device remain valid after mount cycle.
Signed-off-by: Anand Jain <asj@kernel.org>
---
tests/generic/795 | 73 +++++++++++++++++++++++++++++++++++++++++++
tests/generic/795.out | 2 ++
2 files changed, 75 insertions(+)
create mode 100644 tests/generic/795
create mode 100644 tests/generic/795.out
diff --git a/tests/generic/795 b/tests/generic/795
new file mode 100644
index 000000000000..e7ba146350ee
--- /dev/null
+++ b/tests/generic/795
@@ -0,0 +1,73 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Anand Jain <asj@kernel.org>. All Rights Reserved.
+#
+# FS QA Test No. 795
+
+. ./common/preamble
+
+_begin_fstest auto quick exportfs clone
+
+_require_test
+_require_exportfs
+_require_loop
+_require_nouuid_mountopt
+_require_test_program "open_by_handle"
+
+_cleanup()
+{
+ cd /
+ rm -r -f $tmp.*
+ _unmount $mnt1 2>/dev/null
+ _unmount $mnt2 2>/dev/null
+ _loop_image_destroy "${devs[@]}" 2> /dev/null
+}
+
+# Create test dir and test files, encode file handles and store to tmp file
+create_test_files()
+{
+ rm -rf $testdir
+ mkdir -p $testdir
+ $here/src/open_by_handle -cwp -o $tmp.handles_file $testdir $NUMFILES
+}
+
+# Decode file handles loaded from tmp file
+test_file_handles()
+{
+ local opt=$1
+ local when=$2
+
+ echo test_file_handles after $when
+ $here/src/open_by_handle $opt -i $tmp.handles_file $mnt2 $NUMFILES
+}
+
+devs=()
+_loop_image_create_clone devs
+mkdir -p $TEST_DIR/$seq
+mnt1=$TEST_DIR/$seq/mnt1
+mnt2=$TEST_DIR/$seq/mnt2
+mkdir -p $mnt1
+mkdir -p $mnt2
+
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+
+NUMFILES=1
+testdir=$mnt2/testdir
+
+# Decode file handles of files/dir after cycle mount
+create_test_files
+
+_unmount $mnt1
+_unmount $mnt2
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+
+test_file_handles -rp "cycle mount"
+
+status=0
+exit
diff --git a/tests/generic/795.out b/tests/generic/795.out
new file mode 100644
index 000000000000..774fe7487d65
--- /dev/null
+++ b/tests/generic/795.out
@@ -0,0 +1,2 @@
+QA output created by 795
+test_file_handles after cycle mount
--
2.43.0
^ permalink raw reply related
* [PATCH v2 8/9] fstests: verify IMA isolation on cloned filesystems
From: Anand Jain @ 2026-03-21 11:58 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, amir73il, zlang, hch
In-Reply-To: <cover.1774090817.git.asj@kernel.org>
Add testcase to verify IMA measurement isolation when multiple devices
share the same FSUUID.
Signed-off-by: Anand Jain <asj@kernel.org>
---
tests/generic/794 | 103 ++++++++++++++++++++++++++++++++++++++++++
tests/generic/794.out | 10 ++++
2 files changed, 113 insertions(+)
create mode 100644 tests/generic/794
create mode 100644 tests/generic/794.out
diff --git a/tests/generic/794 b/tests/generic/794
new file mode 100644
index 000000000000..391563b84cfc
--- /dev/null
+++ b/tests/generic/794
@@ -0,0 +1,103 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Anand Jain <asj@kernel.org>. All Rights Reserved.
+#
+# FS QA Test 794
+# Verify IMA isolation on cloned filesystems:
+# . Mount two devices sharing the same FSUUID (cloned).
+# . Apply an IMA policy to measure files based on that FSUUID.
+# . Create unique files on each mount point to trigger measurements.
+# . Confirm the IMA log correctly attributes events to the respective mounts.
+
+. ./common/preamble
+. ./common/filter
+
+_begin_fstest auto quick clone
+
+_require_test
+_require_loop
+_require_nouuid_mountopt
+
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
+ "btrfs: use on-disk uuid for s_uuid in temp_fsid mounts"
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
+ "btrfs: derive f_fsid from on-disk fsuuid and dev_t"
+
+_cleanup()
+{
+ cd /
+ rm -r -f $tmp.*
+ _unmount $mnt1 2>/dev/null
+ _unmount $mnt2 2>/dev/null
+ _loop_image_destroy "${devs[@]}" 2> /dev/null
+}
+
+filter_pool()
+{
+ sed -e "s|${devs[0]}|DEV1|g" -e "s|$mnt1|MNT1|g" \
+ -e "s|${devs[1]}|DEV2|g" -e "s|$mnt2|MNT2|g" | _filter_spaces
+}
+
+do_ima()
+{
+ local ima_policy="/sys/kernel/security/ima/policy"
+ local ima_log="/sys/kernel/security/ima/ascii_runtime_measurements"
+ local fsuuid
+ local mnt=$1
+ local enable=$2
+
+ # Since the in-memory IMA audit log is only cleared upon reboot,
+ # use unique random filenames to avoid log collisions.
+ local foofile=$(mktemp --dry-run foobar_XXXXX)
+
+ echo $mnt $enable | filter_pool
+
+ [ -w "$ima_policy" ] || _notrun "IMA policy not writable"
+
+ fsuuid=$(blkid -s UUID -o value ${devs[0]})
+
+ # Load IMA policy to measure file access specifically for this
+ # filesystem UUID.
+ if [[ $enable -eq 1 ]]; then
+ echo "measure func=FILE_CHECK fsuuid=$fsuuid" > "$ima_policy" || \
+ _notrun "Policy rejected"
+ fi
+
+ # Create a file to trigger measurement and verify its entry in
+ # the IMA log.
+ echo "test_data" > $mnt/$foofile
+
+ # For $ima_log column entry please ref to
+ grep $foofile "$ima_log" | awk '{ print $5 }' | filter_pool | \
+ sed "s/$foofile/FOOBAR_FILE/"
+
+ echo "dbg: $mnt $fsuuid $foofile" >> $seqres.full
+ cat $ima_log | tail -1 >> $seqres.full
+ echo >> $seqres.full
+}
+
+devs=()
+_loop_image_create_clone devs
+mnt1=$TEST_DIR/$seq/mnt1
+mnt2=$TEST_DIR/$seq/mnt2
+mkdir -p $mnt1
+mkdir -p $mnt2
+
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+
+do_ima $mnt1 1
+do_ima $mnt2 0
+
+# Btrfs uses in-memory dynamic temp_fsid
+echo mount cycle
+_unmount $mnt2
+_mount $mount_opts ${devs[1]} $mnt2 || _fail "Failed to mount dev2"
+
+do_ima $mnt1 0
+do_ima $mnt2 0
+
+status=0
+exit
diff --git a/tests/generic/794.out b/tests/generic/794.out
new file mode 100644
index 000000000000..492f8b0b0bec
--- /dev/null
+++ b/tests/generic/794.out
@@ -0,0 +1,10 @@
+QA output created by 794
+MNT1 1
+MNT1/FOOBAR_FILE
+MNT2 0
+MNT2/FOOBAR_FILE
+mount cycle
+MNT1 0
+MNT1/FOOBAR_FILE
+MNT2 0
+MNT2/FOOBAR_FILE
--
2.43.0
^ permalink raw reply related
* [PATCH v2 7/9] fstests: verify libblkid resolution of duplicate UUIDs
From: Anand Jain @ 2026-03-21 11:58 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, amir73il, zlang, hch
In-Reply-To: <cover.1774090817.git.asj@kernel.org>
Verify how findmnt, df (libblkid) resolve device paths when multiple
block devices share the same FSUUID.
Signed-off-by: Anand Jain <asj@kernel.org>
---
tests/generic/793 | 76 +++++++++++++++++++++++++++++++++++++++++++
tests/generic/793.out | 19 +++++++++++
2 files changed, 95 insertions(+)
create mode 100644 tests/generic/793
create mode 100644 tests/generic/793.out
diff --git a/tests/generic/793 b/tests/generic/793
new file mode 100644
index 000000000000..fa5d8a6fe642
--- /dev/null
+++ b/tests/generic/793
@@ -0,0 +1,76 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Anand Jain <asj@kernel.org>. All Rights Reserved.
+#
+# FS QA Test 793
+# Verify how libblkid resolve devices when multiple devices sharing the
+# same FSUUID.
+
+. ./common/preamble
+. ./common/filter
+
+_begin_fstest auto quick mount clone
+
+_require_test
+_require_loop
+_require_nouuid_mountopt
+
+_cleanup()
+{
+ cd /
+ rm -r -f $tmp.*
+ umount $mnt1 $mnt2 2>/dev/null
+ _loop_image_destroy "${devs[@]}" 2> /dev/null
+}
+
+filter_pool()
+{
+ sed -e "s|${devs[0]}|DEV1|g" -e "s|${mnt1}|MNT1|g" \
+ -e "s|${devs[1]}|DEV2|g" -e "s|${mnt2}|MNT2|g" | _filter_spaces
+}
+
+print_info()
+{
+ local mntpt=$1
+ local tgt=$(findmnt -no SOURCE $mntpt)
+ local fsuuid=$(blkid -s UUID -o value $tgt)
+
+ echo "mntpt=$mntpt tgt=$tgt fsuuid=$fsuuid" >> $seqres.full
+ echo
+ findmnt -o SOURCE,TARGET,UUID "$tgt" | tail -n +2 | \
+ sed -e "s/${fsuuid}/FSUUID/g" | filter_pool
+ awk -v dev="$tgt" '$1 == dev { print $1, $2 }' /proc/self/mounts | \
+ filter_pool
+ df --all --output=source,target "$tgt" | tail -n +2 | filter_pool
+}
+
+devs=()
+_loop_image_create_clone devs
+mkdir -p $TEST_DIR/$seq
+mnt1=$TEST_DIR/$seq/mnt1
+mnt2=$TEST_DIR/$seq/mnt2
+mkdir -p $mnt1
+mkdir -p $mnt2
+
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+
+print_info $mnt1
+print_info $mnt2
+
+echo
+echo "**** mount cycle ****"
+_unmount $mnt1
+_unmount $mnt2
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+
+print_info $mnt1
+print_info $mnt2
+
+status=0
+exit
diff --git a/tests/generic/793.out b/tests/generic/793.out
new file mode 100644
index 000000000000..53cb4d850d93
--- /dev/null
+++ b/tests/generic/793.out
@@ -0,0 +1,19 @@
+QA output created by 793
+
+DEV1 MNT1 FSUUID
+DEV1 MNT1
+DEV1 MNT1
+
+DEV2 MNT2 FSUUID
+DEV2 MNT2
+DEV2 MNT2
+
+**** mount cycle ****
+
+DEV1 MNT1 FSUUID
+DEV1 MNT1
+DEV1 MNT1
+
+DEV2 MNT2 FSUUID
+DEV2 MNT2
+DEV2 MNT2
--
2.43.0
^ permalink raw reply related
* [PATCH v2 6/9] fstests: verify f_fsid for cloned filesystems
From: Anand Jain @ 2026-03-21 11:58 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, amir73il, zlang, hch
In-Reply-To: <cover.1774090817.git.asj@kernel.org>
Verify that the cloned filesystem provides an f_fsid that is persistent
across mount cycles, yet unique from the original filesystem's f_fsid.
Signed-off-by: Anand Jain <asj@kernel.org>
---
tests/generic/792 | 62 +++++++++++++++++++++++++++++++++++++++++++
tests/generic/792.out | 7 +++++
2 files changed, 69 insertions(+)
create mode 100644 tests/generic/792
create mode 100644 tests/generic/792.out
diff --git a/tests/generic/792 b/tests/generic/792
new file mode 100644
index 000000000000..7716928cb7d2
--- /dev/null
+++ b/tests/generic/792
@@ -0,0 +1,62 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Anand Jain <asj@kernel.org>. All Rights Reserved.
+#
+# FS QA Test 792
+# Verify f_fsid and s_uuid of cloned filesystems across mount cycle.
+
+. ./common/preamble
+
+_begin_fstest auto quick mount clone
+
+_require_test
+_require_loop
+_require_nouuid_mountopt
+
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
+ "btrfs: use on-disk uuid for s_uuid in temp_fsid mounts"
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
+ "btrfs: derive f_fsid from on-disk fsuuid and dev_t"
+
+_cleanup()
+{
+ cd /
+ rm -r -f $tmp.*
+ umount $mnt1 $mnt2 2>/dev/null
+ _loop_image_destroy "${devs[@]}" 2> /dev/null
+}
+
+devs=()
+_loop_image_create_clone devs
+mkdir -p $TEST_DIR/$seq
+mnt1=$TEST_DIR/$seq/mnt1
+mnt2=$TEST_DIR/$seq/mnt2
+mkdir -p $mnt1
+mkdir -p $mnt2
+
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+
+fsid_scratch=$(stat -f -c "%i" $mnt1)
+fsid_clone=$(stat -f -c "%i" $mnt2)
+
+echo "**** fsid initially ****"
+echo $fsid_scratch | sed -e "s/$fsid_scratch/FSID_SCRATCH/g"
+echo $fsid_clone | sed -e "s/$fsid_clone/FSID_CLONE/g"
+
+# Make sure fsid still match across a mount cycle, also reverse the order.
+echo "**** fsid after mount cycle ****"
+_unmount $mnt1
+_unmount $mnt2
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+
+stat -f -c "%i" $mnt1 | sed -e "s/$fsid_scratch/FSID_SCRATCH/g"
+stat -f -c "%i" $mnt2 | sed -e "s/$fsid_clone/FSID_CLONE/g"
+
+status=0
+exit
diff --git a/tests/generic/792.out b/tests/generic/792.out
new file mode 100644
index 000000000000..27ecbce2225e
--- /dev/null
+++ b/tests/generic/792.out
@@ -0,0 +1,7 @@
+QA output created by 792
+**** fsid initially ****
+FSID_SCRATCH
+FSID_CLONE
+**** fsid after mount cycle ****
+FSID_SCRATCH
+FSID_CLONE
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/9] fstests: verify fanotify isolation on cloned filesystems
From: Anand Jain @ 2026-03-21 11:58 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, amir73il, zlang, hch
In-Reply-To: <cover.1774090817.git.asj@kernel.org>
Verify that fanotify events are correctly routed to the appropriate
watcher when cloned filesystems are mounted.
Helps verify kernel's event notification distinguishes between devices
sharing the same FSID/UUID.
Signed-off-by: Anand Jain <asj@kernel.org>
---
common/config | 1 +
tests/generic/791 | 116 ++++++++++++++++++++++++++++++++++++++++++
tests/generic/791.out | 7 +++
3 files changed, 124 insertions(+)
create mode 100644 tests/generic/791
create mode 100644 tests/generic/791.out
diff --git a/common/config b/common/config
index c08f828575a2..73a2a91df9f9 100644
--- a/common/config
+++ b/common/config
@@ -229,6 +229,7 @@ export PARTED_PROG="$(type -P parted)"
export XFS_PROPERTY_PROG="$(type -P xfs_property)"
export FSCRYPTCTL_PROG="$(type -P fscryptctl)"
export INOTIFYWAIT_PROG="$(type -P inotifywait)"
+export FSNOTIFYWAIT_PROG="$(type -P fsnotifywait)"
# udev wait functions.
#
diff --git a/tests/generic/791 b/tests/generic/791
new file mode 100644
index 000000000000..26656cf8e450
--- /dev/null
+++ b/tests/generic/791
@@ -0,0 +1,116 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Anand Jain <asj@kernel.org>. All Rights Reserved.
+#
+# FS QA Test 791
+# Verify fanotify FID functionality on cloned filesystems by setting up
+# watchers and making sure notifications are in the correct logs files.
+
+. ./common/preamble
+
+_begin_fstest auto quick mount clone
+
+_require_test
+_require_loop
+_require_nouuid_mountopt
+_require_command "$FSNOTIFYWAIT_PROG" fsnotifywait
+
+[ "$FSTYP" = "ext4" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
+ "ext4: derive f_fsid from block device to avoid collisions"
+
+_cleanup()
+{
+ cd /
+ [[ -n $pid1 ]] && { kill -TERM "$pid1" 2> /dev/null; wait $pid1; }
+ [[ -n $pid2 ]] && { kill -TERM "$pid2" 2> /dev/null; wait $pid2; }
+ umount $mnt1 $mnt2 2>/dev/null
+ _loop_image_destroy "${devs[@]}" 2> /dev/null
+ rm -r -f $tmp.*
+}
+
+monitor_fanotify()
+{
+ local mmnt=$1
+ exec stdbuf -oL $FSNOTIFYWAIT_PROG -m -F -S -e create "$mmnt" 2>&1
+}
+
+fsid_to_fid_parts()
+{
+ local fsid=$1
+ # Pad to 16 hex chars (64-bit), then split into two 32-bit halves
+ local padded=$(printf '%016x' "0x${fsid}")
+ local hi=$(printf '%x' "0x${padded:0:8}") # strips leading zeros
+ local lo=$(printf '%x' "0x${padded:8:8}") # strips leading zeros
+ echo "${hi}.${lo}"
+}
+
+devs=()
+_loop_image_create_clone devs
+mkdir -p $TEST_DIR/$seq
+mnt1=$TEST_DIR/$seq/mnt1
+mnt2=$TEST_DIR/$seq/mnt2
+mkdir -p $mnt1
+mkdir -p $mnt2
+
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[0]} $mnt1 || \
+ _fail "Failed to mount dev1"
+_mount $(_common_dev_mount_options) $(_clone_mount_option) ${devs[1]} $mnt2 || \
+ _fail "Failed to mount dev2"
+
+fsid1=$(stat -f -c "%i" $mnt1)
+fsid2=$(stat -f -c "%i" $mnt2)
+
+[[ "$fsid1" == "$fsid2" ]] && \
+ _notrun "Require clone filesystem with unique f_fsid"
+
+log1=$tmp.fanotify1
+log2=$tmp.fanotify2
+
+pid1=""
+pid2=""
+echo "Setup FID fanotify watchers on both mnt1 and mnt2"
+( monitor_fanotify "$mnt1" > "$log1" ) &
+pid1=$!
+( monitor_fanotify "$mnt2" > "$log2" ) &
+pid2=$!
+sleep 2
+
+echo "Trigger file creation on mnt1"
+touch $mnt1/file_on_mnt1
+sync
+sleep 1
+
+echo "Trigger file creation on mnt2"
+touch $mnt2/file_on_mnt2
+sync
+sleep 1
+
+echo "Verify fsid in the fanotify"
+kill $pid1 $pid2
+wait $pid1 $pid2 2>/dev/null
+pid1=""
+pid2=""
+
+e_fsid1=$(fsid_to_fid_parts "$fsid1")
+e_fsid2=$(fsid_to_fid_parts "$fsid2")
+
+echo $fsid1 $e_fsid1 $fsid2 $e_fsid2 >> $seqres.full
+cat $log1 >> $seqres.full
+cat $log2 >> $seqres.full
+
+if grep -qF "$e_fsid1" "$log1" && ! grep -qF "$e_fsid2" "$log1"; then
+ echo "SUCCESS: mnt1 events found"
+else
+ [ ! -s "$log1" ] && echo " - mnt1 received no events."
+ grep -qF "$e_fsid2" "$log1" && echo " - mnt1 received event from mnt2."
+fi
+
+if grep -qF "$e_fsid2" "$log2" && ! grep -qF "$e_fsid1" "$log2"; then
+ echo "SUCCESS: mnt2 events found"
+else
+ [ ! -s "$log2" ] && echo " - mnt2 received no events."
+ grep -qF "$e_fsid1" "$log2" && echo " - mnt2 received event from mnt1."
+fi
+
+status=0
+exit
diff --git a/tests/generic/791.out b/tests/generic/791.out
new file mode 100644
index 000000000000..50e494647388
--- /dev/null
+++ b/tests/generic/791.out
@@ -0,0 +1,7 @@
+QA output created by 791
+Setup FID fanotify watchers on both mnt1 and mnt2
+Trigger file creation on mnt1
+Trigger file creation on mnt2
+Verify fsid in the fanotify
+SUCCESS: mnt1 events found
+SUCCESS: mnt2 events found
--
2.43.0
^ 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