* Re: [PATCH 14/34] ext4: Convert write_mmp_block_thawed() to bh_submit()
From: Jan Kara @ 2026-05-27 10:42 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jan Kara, Christian Brauner, Christoph Hellwig, linux-fsdevel,
linux-ext4
In-Reply-To: <20260525171931.4144395-15-willy@infradead.org>
On Mon 25-05-26 18:19:07, Matthew Wilcox (Oracle) wrote:
> Avoid an extra indirect function call by using bh_submit() instead of
> submit_bh().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: linux-ext4@vger.kernel.org
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/mmp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 6f57c181ff77..493528fbed75 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -46,9 +46,9 @@ static int write_mmp_block_thawed(struct super_block *sb,
>
> ext4_mmp_csum_set(sb, mmp);
> lock_buffer(bh);
> - bh->b_end_io = end_buffer_write_sync;
> get_bh(bh);
> - submit_bh(REQ_OP_WRITE | REQ_SYNC | REQ_META | REQ_PRIO, bh);
> + bh_submit(bh, REQ_OP_WRITE | REQ_SYNC | REQ_META | REQ_PRIO,
> + bh_end_write);
> wait_on_buffer(bh);
> if (unlikely(!buffer_uptodate(bh)))
> return -EIO;
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 15/34] ext4: Convert ext4_commit_super() to bh_submit()
From: Jan Kara @ 2026-05-27 10:42 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jan Kara, Christian Brauner, Christoph Hellwig, linux-fsdevel,
linux-ext4
In-Reply-To: <20260525171931.4144395-16-willy@infradead.org>
On Mon 25-05-26 18:19:08, Matthew Wilcox (Oracle) wrote:
> Avoid an extra indirect function call by using bh_submit() instead of
> submit_bh().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: linux-ext4@vger.kernel.org
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/super.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index fbe175951e01..905d66cbe3f2 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6320,9 +6320,8 @@ static int ext4_commit_super(struct super_block *sb)
> get_bh(sbh);
> /* Clear potential dirty bit if it was journalled update */
> clear_buffer_dirty(sbh);
> - sbh->b_end_io = end_buffer_write_sync;
> - submit_bh(REQ_OP_WRITE | REQ_SYNC |
> - (test_opt(sb, BARRIER) ? REQ_FUA : 0), sbh);
> + bh_submit(sbh, REQ_OP_WRITE | REQ_SYNC |
> + (test_opt(sb, BARRIER) ? REQ_FUA : 0), bh_end_write);
> wait_on_buffer(sbh);
> if (buffer_write_io_error(sbh)) {
> ext4_msg(sb, KERN_ERR, "I/O error while writing "
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2] ext2: Remove deprecated DAX support
From: Ashwin Gundarapu @ 2026-05-27 10:53 UTC (permalink / raw)
To: Jan Kara; +Cc: jack, linux-ext4, linux-kernel
In-Reply-To: <fxaiddid432ivmkcsqmzeovemmnkyh37nfgwn4xurxb2wx5u5y@lhkjib3tmd7e>
Thanks for the review, Jan. All the style issues you mentioned have
been addressed in v3 and v4:
v3: https://lore.kernel.org/linux-ext4/19e595ac3d0.1a0dcfbe128078.1031782761444069401@zohomail.in/
v4: https://lore.kernel.org/linux-ext4/19e5aa07c9b.3a2e576d130187.5289857983023045470@zohomail.in/
v3 fixed the spaces-to-tabs indentation issues in inode.c, super.c,
and file.c. It also restored Opt_dax for a graceful mount error message.
v4 changed Opt_xip and Opt_dax from -EINVAL to break with a warning,
per Sashiko AI review, to avoid potential boot failures on systems
with these options in /etc/fstab.
The stray empty lines and tab indentation are all cleaned up in the
latest version (v4).
Thanks,
Ashwin
From: Jan Kara <jack@suse.cz>
To: "Ashwin Gundarapu"<linuxuser509@zohomail.in>
Cc: "jack"<jack@suse.com>, "linux-ext4"<linux-ext4@vger.kernel.org>, "linux-kernel"<linux-kernel@vger.kernel.org>
Date: Mon, 25 May 2026 22:00:40 +0530
Subject: Re: [PATCH v2] ext2: Remove deprecated DAX support
> On Sun 24-05-26 11:08:53, Ashwin Gundarapu wrote:
> >
> > DAX support in ext2 was deprecated in commit d5a2693f93e4
> > ("ext2: Deprecate DAX") with a removal deadline of end of 2025.
> > Remove all DAX code from ext2 as scheduled.
> >
> > This removes the DAX mount option, IOMAP DAX support, DAX file
> > operations, DAX address_space_operations, and the DAX fault handler.
> >
> > Signed-off-by: Ashwin Gundarapu <linuxuser509@zohomail.in>
> > ---
> > v2: Removed unused sbi variable and fixed indentation as reported
> > by kernel test robot.
>
> Thanks for the patch. Some style nits below.
>
> > static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > {
> > -#ifdef CONFIG_FS_DAX
> > - if (IS_DAX(iocb->ki_filp->f_mapping->host))
> > - return ext2_dax_read_iter(iocb, to);
> > -#endif
> > +
>
> Stray empty line here.
>
> > if (iocb->ki_flags & IOCB_DIRECT)
> > return ext2_dio_read_iter(iocb, to);
> >
> > @@ -297,10 +188,7 @@ static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >
> > static ssize_t ext2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > {
> > -#ifdef CONFIG_FS_DAX
> > - if (IS_DAX(iocb->ki_filp->f_mapping->host))
> > - return ext2_dax_write_iter(iocb, from);
> > -#endif
> > +
>
> ... and here.
>
> > if (iocb->ki_flags & IOCB_DIRECT)
> > return ext2_dio_write_iter(iocb, from);
> >
> > @@ -321,7 +209,7 @@ const struct file_operations ext2_file_operations = {
> > #ifdef CONFIG_COMPAT
> > .compat_ioctl = ext2_compat_ioctl,
> > #endif
> > - .mmap_prepare = ext2_file_mmap_prepare,
> > + .mmap_prepare = generic_file_mmap_prepare,
>
> Please indent this with tab the same way as other methods.
>
> > @@ -841,10 +818,7 @@ static int ext2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> >
> > iomap->flags = 0;
> > iomap->offset = (u64)first_block << blkbits;
> > - if (flags & IOMAP_DAX)
> > - iomap->dax_dev = sbi->s_daxdev;
> > - else
> > - iomap->bdev = inode->i_sb->s_bdev;
> > + iomap->bdev = inode->i_sb->s_bdev;
>
> Indented with spaces instead of tabs.
>
> > @@ -1290,12 +1248,8 @@ static int ext2_setsize(struct inode *inode, loff_t newsize)
> >
> > inode_dio_wait(inode);
> >
> > - if (IS_DAX(inode))
> > - error = dax_truncate_page(inode, newsize, NULL,
> > - &ext2_iomap_ops);
> > - else
> > - error = block_truncate_page(inode->i_mapping,
> > - newsize, ext2_get_block);
> > + error = block_truncate_page(inode->i_mapping,
> > + newsize, ext2_get_block);
>
> Indented with spaces instead of tabs.
>
> > if (error)
> > return error;
> >
>
> ...
> > + case Opt_xip:
> > + ext2_msg_fc(fc, KERN_ERR, "DAX support has been removed. Please use ext4 instead.");
> > + return -EINVAL;
>
> Indented with spaces instead of tabs.
>
> > @@ -992,16 +974,8 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
> > }
> > blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size);
> >
> > - if (test_opt(sb, DAX)) {
> > - if (!sbi->s_daxdev) {
> > - ext2_msg(sb, KERN_ERR,
> > - "DAX unsupported by block device. Turning off DAX.");
> > - clear_opt(sbi->s_mount_opt, DAX);
> > - } else if (blocksize != PAGE_SIZE) {
> > - ext2_msg(sb, KERN_ERR, "unsupported blocksize for DAX\n");
> > - clear_opt(sbi->s_mount_opt, DAX);
> > - }
> > - }
> > +
> > +
>
> Stray empty lines.
>
> Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
>
>
^ permalink raw reply
* Re: [PATCH 16/34] jbd2: Convert journal commit to bh_submit()
From: Jan Kara @ 2026-05-27 10:54 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jan Kara, Christian Brauner, Christoph Hellwig, linux-fsdevel,
linux-ext4
In-Reply-To: <20260525171931.4144395-17-willy@infradead.org>
On Mon 25-05-26 18:19:09, Matthew Wilcox (Oracle) wrote:
> Avoid an extra indirect function call by using bh_submit()
> instead of submit_bh() in journal_submit_commit_record()
> and jbd2_journal_commit_transaction(). These both use
> journal_end_buffer_io_sync(), so it's more straightforward to do them
> both at once.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: linux-ext4@vger.kernel.org
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Another note for future work here: The BH_Shadow handling looks like a dead
code. We hold buffer lock when writing out bh to the journal and we do
acquire the buffer lock in do_get_write_access() anyway (which is the only
place that checks for BH_Shadow) so the buffer_shadow() check should never
trigger. Needs checking, some more thought, and possibly slightly expanding
the area where buffer lock is held in do_get_write_access() but it should
be relatively low hanging fruit. Then we can completely remove BH_Shadow
and use generic IO completion function.
Honza
> ---
> fs/jbd2/commit.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 8cf61e7185c4..38f318bb4279 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -29,8 +29,10 @@
> /*
> * IO end handler for temporary buffer_heads handling writes to the journal.
> */
> -static void journal_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
> +static void journal_end_buffer_io_sync(struct bio *bio)
> {
> + bool uptodate = bio->bi_status == BLK_STS_OK;
> + struct buffer_head *bh = bio_endio_bh(bio);
> struct buffer_head *orig_bh = bh->b_private;
>
> BUFFER_TRACE(bh, "");
> @@ -147,13 +149,12 @@ static int journal_submit_commit_record(journal_t *journal,
> lock_buffer(bh);
> clear_buffer_dirty(bh);
> set_buffer_uptodate(bh);
> - bh->b_end_io = journal_end_buffer_io_sync;
>
> if (journal->j_flags & JBD2_BARRIER &&
> !jbd2_has_feature_async_commit(journal))
> write_flags |= REQ_PREFLUSH | REQ_FUA;
>
> - submit_bh(write_flags, bh);
> + bh_submit(bh, write_flags, journal_end_buffer_io_sync);
> *cbh = bh;
> return 0;
> }
> @@ -751,9 +752,9 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> lock_buffer(bh);
> clear_buffer_dirty(bh);
> set_buffer_uptodate(bh);
> - bh->b_end_io = journal_end_buffer_io_sync;
> - submit_bh(REQ_OP_WRITE | JBD2_JOURNAL_REQ_FLAGS,
> - bh);
> + bh_submit(bh,
> + REQ_OP_WRITE | JBD2_JOURNAL_REQ_FLAGS,
> + journal_end_buffer_io_sync);
> }
> cond_resched();
>
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 17/34] jbd2: Convert jbd2_write_superblock() to bh_submit()
From: Jan Kara @ 2026-05-27 10:54 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jan Kara, Christian Brauner, Christoph Hellwig, linux-fsdevel,
linux-ext4
In-Reply-To: <20260525171931.4144395-18-willy@infradead.org>
On Mon 25-05-26 18:19:10, Matthew Wilcox (Oracle) wrote:
> Avoid an extra indirect function call by using bh_submit() instead of
> submit_bh().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: linux-ext4@vger.kernel.org
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/jbd2/journal.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 4f397fcdb13c..a6616380ce38 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1821,8 +1821,7 @@ static int jbd2_write_superblock(journal_t *journal, blk_opf_t write_flags)
> if (jbd2_journal_has_csum_v2or3(journal))
> sb->s_checksum = jbd2_superblock_csum(sb);
> get_bh(bh);
> - bh->b_end_io = end_buffer_write_sync;
> - submit_bh(REQ_OP_WRITE | write_flags, bh);
> + bh_submit(bh, REQ_OP_WRITE | write_flags, bh_end_write);
> wait_on_buffer(bh);
> if (buffer_write_io_error(bh)) {
> clear_buffer_write_io_error(bh);
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 5/8] super: drop sb_lock from setup_bdev_super() tuple publication
From: Christian Brauner @ 2026-05-27 11:53 UTC (permalink / raw)
To: linux-fsdevel
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Ritesh Harjani (IBM),
linux-ext4, linux-cifs, Alexander Viro
In-Reply-To: <20260526-work-sget-v1-5-263f7025cedd@kernel.org>
> }
> - spin_lock(&sb_lock);
Yeah, I failed to consider that we need to protect against a concurrent
sget_fc() call with a custom callback so we cannot reasonably drop this
lock.
> - spin_unlock(&sb_lock);
> + WRITE_ONCE(sb->s_iflags, sb->s_iflags | SB_I_STABLE_WRITES);
^ permalink raw reply
* Re: [PATCH 0/8] super: retire sget(), convert iterators to RCU
From: Christian Brauner @ 2026-05-27 11:54 UTC (permalink / raw)
To: linux-fsdevel
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Ritesh Harjani (IBM),
linux-ext4, linux-cifs, Alexander Viro
In-Reply-To: <20260526-work-sget-v1-0-263f7025cedd@kernel.org>
On Tue, May 26, 2026 at 05:09:02PM +0200, Christian Brauner wrote:
> * retire sget(): CIFS plus the two ext4 KUnit tests (extents-test,
>
> * Walk @super_blocks and @type->fs_supers under RCU, pinned by
Can't work as I originally envisioned.
^ permalink raw reply
* Re: [PATCH 00/17] fs: replace __get_free_pages() call with kmalloc()
From: Christian Brauner @ 2026-05-27 12:05 UTC (permalink / raw)
To: Jan Kara, Mark Fasheh, Joel Becker, Joseph Qi, Ryusuke Konishi,
Viacheslav Dubeyko, Trond Myklebust, Anna Schumaker, Chuck Lever,
Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Alexander Viro, Jan Kara, Dave Kleikamp, Theodore Ts'o,
Miklos Szeredi, Andreas Hindborg, Breno Leitao, Kees Cook,
Tigran A. Aivazian, Mike Rapoport (Microsoft)
Cc: Christian Brauner, linux-kernel, linux-fsdevel, ocfs2-devel,
linux-nilfs, linux-nfs, jfs-discussion, linux-ext4, linux-mm
In-Reply-To: <20260523-b4-fs-v1-0-275e36a83f0e@kernel.org>
On Sat, 23 May 2026 20:54:12 +0300, Mike Rapoport (Microsoft) wrote:
> This is a (small) part of larger work of replacing page allocator calls
> with kmalloc.
>
> Also in git:
> https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/fs
>
>
> [...]
Applied to the vfs-7.2.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.2.misc 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.2.misc
[01/17] quota: allocate dquot_hash with kmalloc()
https://git.kernel.org/vfs/vfs/c/c94d1fa0af45
[02/17] proc: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/3c849e5fe1db
[03/17] ocfs2/dlm: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/40b7e5db6a25
[04/17] nilfs2: replace get_zeroed_page() with kzalloc()
https://git.kernel.org/vfs/vfs/c/2abe95d9f56d
[05/17] NFS: replace __get_free_page() with kmalloc() in nfs_show_devname()
https://git.kernel.org/vfs/vfs/c/75805c8f6d43
[06/17] NFS: remove unused page and page2 in nfs4_replace_transport()
https://git.kernel.org/vfs/vfs/c/0d77bacd0eab
[07/17] NFSD: replace __get_free_page() with kmalloc() in nfsd_buffered_readdir()
https://git.kernel.org/vfs/vfs/c/64f162f93a81
[08/17] libfs: simple_transaction_get(): replace get_zeroed_page() with kzalloc()
https://git.kernel.org/vfs/vfs/c/5a3763a94e95
[09/17] jfs: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/d50250728dc1
[10/17] jbd2: replace __get_free_pages() with kmalloc()
https://git.kernel.org/vfs/vfs/c/75c9377833a1
[11/17] isofs: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/95f2509040ac
[12/17] fuse: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/c78262429022
[13/17] fs/select: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/ac6aa4672cef
[14/17] fs/namespace: use __getname() to allocate mntpath buffer
https://git.kernel.org/vfs/vfs/c/bd822134dcaf
[15/17] configfs: replace __get_free_pages() with kzalloc()
https://git.kernel.org/vfs/vfs/c/32466534cba7
[16/17] binfmt_misc: replace __get_free_page() with kmalloc()
https://git.kernel.org/vfs/vfs/c/df5f3ac3e999
[17/17] bfs: replace get_zeroed_page() with kzalloc()
https://git.kernel.org/vfs/vfs/c/0a994e1ab090
^ permalink raw reply
* Re: [PATCH v4 09/23] ext4: implement writeback path using iomap
From: Ojaswin Mujoo @ 2026-05-27 12:49 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-10-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:29PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Add the iomap writeback path for ext4 buffered I/O. This introduces:
>
> - ext4_iomap_writepages(): the main writeback entry point.
> - ext4_writeback_ops: a new iomap_writeback_ops instance to handle
> block mapping and I/O submission.
> - A new end I/O worker for converting unwritten extents, updating file
> size, and handling DATA_ERR_ABORT after I/O completion.
>
> Core implementation details:
>
> - ->writeback_range() callback
> Calls ext4_iomap_map_writeback_range() to query the longest range of
> existing mapped extents. For performance, when a block range is not
> yet allocated, it allocates based on the writeback length and delalloc
> extent length, rather than allocating for a single folio at a time.
> The folio is then added to an iomap_ioend instance.
>
> - ->writeback_submit() callback
> Registers ext4_iomap_end_bio() as the end bio callback. This callback
> schedules a worker to handle:
> - Unwritten extent conversion.
> - i_disksize update after data is written back.
> - Journal abort on writeback I/O failure.
Hi Zhang, the changes look good. I have a few comments below:
>
> Key changes and considerations:
>
> - Append write and unwritten extents
> Since data=ordered mode is not used to prevent stale data exposure
> during append writebacks, new blocks are always allocated as unwritten
> extents (i.e. always enable dioread_nolock), and i_disksize update is
> postponed until I/O completion.
Makes sense.
> Additionally, the deadlock that the
> reserve handle was expected to resolve does not occur anymore.
I guess this is since we don't use ordered data so we can't block on
starting a txn in end io.
> Therefore, the end I/O worker can start a normal journal handle
> instead of a reserve handle when converting unwritten extents.
>
> - Lock ordering
> The ->writeback_range() callback runs under the folio lock, requiring
> the journal handle to be started under that same lock. This reverses
> the order compared to the buffer_head writeback path. The lock ordering
> documentation in super.c has been updated accordingly.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
> fs/ext4/ext4.h | 4 +
> fs/ext4/inode.c | 208 +++++++++++++++++++++++++++++++++++++++++-
> fs/ext4/page-io.c | 126 +++++++++++++++++++++++++
> fs/ext4/super.c | 7 +-
> fs/iomap/ioend.c | 3 +-
> include/linux/iomap.h | 1 +
> 6 files changed, 346 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 4832e7f7db82..078feda47e36 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1173,6 +1173,8 @@ struct ext4_inode_info {
> */
> struct list_head i_rsv_conversion_list;
> struct work_struct i_rsv_conversion_work;
> + struct list_head i_iomap_ioend_list;
> + struct work_struct i_iomap_ioend_work;
>
> /*
> * Transactions that contain inode's metadata needed to complete
> @@ -3870,6 +3872,8 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *page,
> size_t len);
> extern struct ext4_io_end_vec *ext4_alloc_io_end_vec(ext4_io_end_t *io_end);
> extern struct ext4_io_end_vec *ext4_last_io_end_vec(ext4_io_end_t *io_end);
> +extern void ext4_iomap_end_io(struct work_struct *work);
> +extern void ext4_iomap_end_bio(struct bio *bio);
>
> /* mmp.c */
> extern int ext4_multi_mount_protect(struct super_block *, ext4_fsblk_t);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 1ae7d3f4a1c8..a80195bd6f20 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -44,6 +44,7 @@
> #include <linux/iversion.h>
>
> #include "ext4_jbd2.h"
> +#include "ext4_extents.h"
> #include "xattr.h"
> #include "acl.h"
> #include "truncate.h"
> @@ -4120,10 +4121,215 @@ static void ext4_iomap_readahead(struct readahead_control *rac)
> iomap_bio_readahead(rac, &ext4_iomap_buffered_read_ops);
> }
>
> +static int ext4_iomap_map_one_extent(struct inode *inode,
> + struct ext4_map_blocks *map)
> +{
> + struct extent_status es;
> + handle_t *handle = NULL;
> + int credits, map_flags;
> + int retval;
> +
> + credits = ext4_chunk_trans_blocks(inode, map->m_len);
> + handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, credits);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> + map->m_flags = 0;
> + /*
> + * It is necessary to look up extent and map blocks under i_data_sem
> + * in write mode, otherwise, the delalloc extent may become stale
> + * during concurrent truncate operations.
> + */
> + ext4_fc_track_inode(handle, inode);
> + down_write(&EXT4_I(inode)->i_data_sem);
> + if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
> + retval = es.es_len - (map->m_lblk - es.es_lblk);
> + map->m_len = min_t(unsigned int, retval, map->m_len);
> +
> + if (ext4_es_is_delayed(&es)) {
I understand that it is okay for us to rely on extent status ==
delayed here because we never reclaim delayed es entries and hence we
are sure to not skip any delayed block allocations here.
> + map->m_flags |= EXT4_MAP_DELAYED;
> + trace_ext4_da_write_pages_extent(inode, map);
> + /*
> + * Call ext4_map_create_blocks() to allocate any
> + * delayed allocation blocks. It is possible that
> + * we're going to need more metadata blocks, however
> + * we must not fail because we're in writeback and
> + * there is nothing we can do so it might result in
> + * data loss. So use reserved blocks to allocate
> + * metadata if possible.
> + */
> + map_flags = EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT |
> + EXT4_GET_BLOCKS_METADATA_NOFAIL |
> + EXT4_EX_NOCACHE;
> +
> + retval = ext4_map_create_blocks(handle, inode, map,
> + map_flags);
> + if (retval > 0)
> + ext4_fc_track_range(handle, inode, map->m_lblk,
> + map->m_lblk + map->m_len - 1);
> + goto out;
> + } else if (unlikely(ext4_es_is_hole(&es)))
Now that you've fixed the partial invalidate in iomap (patch 12/23)
can we still hit this hole case?
> + goto out;
> +
> + /* Found written or unwritten extent. */
> + map->m_pblk = ext4_es_pblock(&es) + map->m_lblk - es.es_lblk;
> + map->m_flags = ext4_es_is_written(&es) ?
> + EXT4_MAP_MAPPED : EXT4_MAP_UNWRITTEN;
> + goto out;
> + }
> +
> + retval = ext4_map_query_blocks(handle, inode, map, EXT4_EX_NOCACHE);
> +out:
> + up_write(&EXT4_I(inode)->i_data_sem);
> + ext4_journal_stop(handle);
> + return retval < 0 ? retval : 0;
> +}
> +
> +static int ext4_iomap_map_writeback_range(struct iomap_writepage_ctx *wpc,
> + loff_t offset, unsigned int dirty_len)
> +{
> + struct inode *inode = wpc->inode;
> + struct super_block *sb = inode->i_sb;
> + struct journal_s *journal = EXT4_SB(sb)->s_journal;
> + struct ext4_map_blocks map;
> + unsigned int blkbits = inode->i_blkbits;
> + unsigned int index = offset >> blkbits;
> + unsigned int blk_end, blk_len;
> + int ret;
> +
> + ret = ext4_emergency_state(sb);
> + if (unlikely(ret))
> + return ret;
> +
> + /* Check validity of the cached writeback mapping. */
> + if (offset >= wpc->iomap.offset &&
> + offset < wpc->iomap.offset + wpc->iomap.length &&
> + ext4_iomap_valid(inode, &wpc->iomap))
> + return 0;
> +
> + blk_len = dirty_len >> blkbits;
> + blk_end = min_t(unsigned int, (wpc->wbc->range_end >> blkbits),
> + (UINT_MAX - 1));
This is an interesting idea. I'm just a bit worried when we have
range_end == LLONG_MAX (bg flush) and we will always be trying to allocate
MAX_WRITEPAGES, incase of a slightly fragmented FS, we might keep
falling into slower mballoc criterias and might waste a lot of time
scanning the groups.
> + if (blk_end > index + blk_len)
> + blk_len = blk_end - index + 1;
> +
> +retry:
> + map.m_lblk = index;
> + map.m_len = min_t(unsigned int, MAX_WRITEPAGES_EXTENT_LEN, blk_len);
> + ret = ext4_map_blocks(NULL, inode, &map,
> + EXT4_GET_BLOCKS_IO_SUBMIT | EXT4_EX_NOCACHE);
Do we really need the IO_SUBMIT flag here now that we are:
1. Not using ordered data
2. We anyways don't use it in ext4_iomap_map_one_extent().
I think we can drop it.
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * The map is not a delalloc extent, it must either be a hole
> + * or an extent which have already been allocated.
> + */
> + if (!(map.m_flags & EXT4_MAP_DELAYED))
> + goto out;
> +
> + /* Map one delalloc extent. */
> + ret = ext4_iomap_map_one_extent(inode, &map);
> + if (ret < 0) {
> + if (ext4_emergency_state(sb))
> + return ret;
> +
> + /*
> + * Retry transient ENOSPC errors, if
> + * ext4_count_free_blocks() is non-zero, a commit
> + * should free up blocks.
> + */
> + if (ret == -ENOSPC && journal && ext4_count_free_clusters(sb)) {
> + jbd2_journal_force_commit_nested(journal);
> + goto retry;
> + }
> +
> + ext4_msg(sb, KERN_CRIT,
> + "Delayed block allocation failed for inode %llu at logical offset %llu with max blocks %u with error %d",
> + inode->i_ino, (unsigned long long)map.m_lblk,
> + (unsigned int)map.m_len, -ret);
> + ext4_msg(sb, KERN_CRIT,
> + "This should not happen!! Data will be lost\n");
> + if (ret == -ENOSPC)
> + ext4_print_free_blocks(inode);
> + return ret;
> + }
> +out:
> + ext4_set_iomap(inode, &wpc->iomap, &map, offset, dirty_len, 0);
> + return 0;
> +}
> +
<snip>
>
^ permalink raw reply
* Re: [PATCH v4 10/23] ext4: implement mmap path using iomap
From: Ojaswin Mujoo @ 2026-05-27 12:56 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-11-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:30PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Introduce ext4_iomap_page_mkwrite() to implement the mmap iomap path
> for ext4. The heavy lifting is delegated to iomap_page_mkwrite(), which
> only requires ext4_iomap_buffered_write_ops and
> ext4_iomap_buffered_da_write_ops to allocate and map blocks.
>
> Note that the lock ordering between folio lock and transaction start in
> this path is reversed compared to the buffer_head buffered write path.
> The lock ordering documentation in super.c has been updated accordingly.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Regards,
Ojaswin
> ---
> fs/ext4/inode.c | 32 +++++++++++++++++++++++++++++++-
> fs/ext4/super.c | 8 ++++++--
> 2 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index a80195bd6f20..c6fe42d012fc 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4020,7 +4020,7 @@ static int ext4_iomap_buffered_do_write_begin(struct inode *inode,
> return -ERANGE;
> if (WARN_ON_ONCE(!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> return -EINVAL;
> - if (WARN_ON_ONCE(!(flags & IOMAP_WRITE)))
> + if (WARN_ON_ONCE(!(flags & (IOMAP_WRITE | IOMAP_FAULT))))
> return -EINVAL;
>
> if (delalloc)
> @@ -4080,6 +4080,14 @@ static int ext4_iomap_buffered_da_write_end(struct inode *inode, loff_t offset,
> if (iomap->type != IOMAP_DELALLOC || !(iomap->flags & IOMAP_F_NEW))
> return 0;
>
> + /*
> + * iomap_page_mkwrite() will never fail in a way that requires delalloc
> + * extents that it allocated to be revoked. Hence never try to release
> + * them here.
> + */
> + if (flags & IOMAP_FAULT)
> + return 0;
> +
> /* Nothing to do if we've written the entire delalloc extent */
> start_byte = iomap_last_written_block(inode, offset, written);
> end_byte = round_up(offset + length, i_blocksize(inode));
> @@ -7191,6 +7199,23 @@ static int ext4_block_page_mkwrite(struct inode *inode, struct folio *folio,
> return ret;
> }
>
> +static vm_fault_t ext4_iomap_page_mkwrite(struct vm_fault *vmf)
> +{
> + struct inode *inode = file_inode(vmf->vma->vm_file);
> + const struct iomap_ops *iomap_ops;
> +
> + /*
> + * ext4_nonda_switch() could writeback this folio, so have to
> + * call it before lock folio.
> + */
> + if (test_opt(inode->i_sb, DELALLOC) && !ext4_nonda_switch(inode->i_sb))
> + iomap_ops = &ext4_iomap_buffered_da_write_ops;
> + else
> + iomap_ops = &ext4_iomap_buffered_write_ops;
> +
> + return iomap_page_mkwrite(vmf, iomap_ops, NULL);
> +}
> +
> vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> @@ -7213,6 +7238,11 @@ vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
>
> filemap_invalidate_lock_shared(mapping);
>
> + if (ext4_inode_buffered_iomap(inode)) {
> + ret = ext4_iomap_page_mkwrite(vmf);
> + goto out;
> + }
> +
> err = ext4_convert_inline_data(inode);
> if (err)
> goto out_ret;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 51d87db53543..62bfe05a64bc 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -100,8 +100,12 @@ static const struct fs_parameter_spec ext4_param_specs[];
> * Lock ordering
> *
> * page fault path:
> - * mmap_lock -> sb_start_pagefault -> invalidate_lock (r) -> transaction start
> - * -> page lock -> i_data_sem (rw)
> + * - buffer_head path:
> + * mmap_lock -> sb_start_pagefault -> invalidate_lock (r) ->
> + * transaction start -> folio lock -> i_data_sem (rw)
> + * - iomap path:
> + * mmap_lock -> sb_start_pagefault -> invalidate_lock (r) ->
> + * folio lock -> transaction start -> i_data_sem (rw)
> *
> * buffered write path:
> * sb_start_write -> i_rwsem (w) -> mmap_lock
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 14/23] ext4: implement partial block zero range path using iomap
From: Ojaswin Mujoo @ 2026-05-27 13:13 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-15-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:34PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Introduce a new iomap_ops instance, ext4_iomap_zero_ops, along with
> ext4_iomap_block_zero_range() to implement block zeroing via the iomap
> infrastructure for ext4.
>
> ext4_iomap_block_zero_range() calls iomap_zero_range() with
> ext4_iomap_zero_begin() as the callback. The callback locates and zeros
> out either a mapped partial block or a dirty, unwritten partial block.
>
> Important constraints:
>
> Zeroing out under an active journal handle can cause deadlock, because
> the order of acquiring the folio lock and starting a handle is
> inconsistent with the iomap writeback path.
>
> Therefore, ext4_iomap_block_zero_range():
> - Must NOT be called under an active handle.
> - Cannot rely on data=ordered mode to ensure zeroed data persistence
> before updating i_disksize (for the cases of post-EOF append write,
> post-EOF fallocate, and truncate up). In subsequent patches, we will
> address this by synchronizing commit I/O but doesn't waiting for
> completion, and updating i_disksize to i_size only after the zeroed
> data has been written back.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good in itself. Feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Regards,
Ojaswin
> ---
> fs/ext4/inode.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c6fe42d012fc..e0dae2501292 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4101,6 +4101,51 @@ static int ext4_iomap_buffered_da_write_end(struct inode *inode, loff_t offset,
> return 0;
> }
>
> +static int ext4_iomap_zero_begin(struct inode *inode,
> + loff_t offset, loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap)
> +{
> + struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap);
> + struct ext4_map_blocks map;
> + u8 blkbits = inode->i_blkbits;
> + unsigned int iomap_flags = 0;
> + int ret;
> +
> + ret = ext4_emergency_state(inode->i_sb);
> + if (unlikely(ret))
> + return ret;
> +
> + if (WARN_ON_ONCE(!(flags & IOMAP_ZERO)))
> + return -EINVAL;
> +
> + ret = ext4_iomap_map_blocks(inode, offset, length, NULL, &map);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * Look up dirty folios for unwritten mappings within EOF. Providing
> + * this bypasses the flush iomap uses to trigger extent conversion
> + * when unwritten mappings have dirty pagecache in need of zeroing.
> + */
> + if (map.m_flags & EXT4_MAP_UNWRITTEN) {
> + loff_t start = ((loff_t)map.m_lblk) << blkbits;
> + loff_t end = ((loff_t)map.m_lblk + map.m_len) << blkbits;
> +
> + iomap_fill_dirty_folios(iter, &start, end, &iomap_flags);
> + if ((start >> blkbits) < map.m_lblk + map.m_len)
> + map.m_len = (start >> blkbits) - map.m_lblk;
> + }
> +
> + ext4_set_iomap(inode, iomap, &map, offset, length, flags);
> + iomap->flags |= iomap_flags;
> +
> + return 0;
> +}
> +
> +static const struct iomap_ops ext4_iomap_zero_ops = {
> + .iomap_begin = ext4_iomap_zero_begin,
> +};
> +
> /*
> * Since we always allocate unwritten extents, there is no need for
> * iomap_end to clean up allocated blocks on a short write.
> @@ -4616,6 +4661,47 @@ static int ext4_block_journalled_zero_range(struct inode *inode, loff_t from,
> return err;
> }
>
> +static int ext4_block_iomap_zero_range(struct inode *inode, loff_t from,
> + loff_t length, bool *did_zero,
> + bool *zero_written)
> +{
> + int ret;
> +
> + /*
> + * Zeroing out under an active handle can cause deadlock since
> + * the order of acquiring the folio lock and starting a handle is
> + * inconsistent with the iomap writeback procedure.
> + */
> + if (WARN_ON_ONCE(ext4_handle_valid(journal_current_handle())))
> + return -EINVAL;
> +
> + /* The zeroing scope should not extend across a block. */
> + if (WARN_ON_ONCE((from >> inode->i_blkbits) !=
> + ((from + length - 1) >> inode->i_blkbits)))
> + return -EINVAL;
> +
> + if (!(EXT4_SB(inode->i_sb)->s_mount_state & EXT4_ORPHAN_FS) &&
> + !(inode_state_read_once(inode) & (I_NEW | I_FREEING)))
> + WARN_ON_ONCE(!inode_is_locked(inode) &&
> + !rwsem_is_locked(&inode->i_mapping->invalidate_lock));
> +
> + ret = iomap_zero_range(inode, from, length, did_zero,
> + &ext4_iomap_zero_ops, &ext4_iomap_write_ops,
> + NULL);
> + if (ret)
> + return ret;
> +
> + /*
> + * TODO: The iomap does not distinguish between different types of
> + * zeroing and always sets zero_written if a zeroing operation is
> + * performed, which may result in unnecessary order operations.
> + */
> + if (did_zero && zero_written)
> + *zero_written = *did_zero;
> +
> + return 0;
> +}
> +
> /*
> * Zeros out a mapping of length 'length' starting from file offset
> * 'from'. The range to be zero'd must be contained with in one block.
> @@ -4642,6 +4728,9 @@ static int ext4_block_zero_range(struct inode *inode,
> } else if (ext4_should_journal_data(inode)) {
> return ext4_block_journalled_zero_range(inode, from, length,
> did_zero);
> + } else if (ext4_inode_buffered_iomap(inode)) {
> + return ext4_block_iomap_zero_range(inode, from, length,
> + did_zero, zero_written);
> }
> return ext4_block_do_zero_range(inode, from, length, did_zero,
> zero_written);
> @@ -4682,6 +4771,9 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> * truncating up or performing an append write, because there might be
> * exposing stale on-disk data which may caused by concurrent post-EOF
> * mmap write during folio writeback.
> + *
> + * TODO: In the iomap path, handle this by updating i_disksize to
> + * i_size after the zeroed data has been written back.
> */
> if (ext4_should_order_data(inode) &&
> did_zero && zero_written && !IS_DAX(inode)) {
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 15/23] ext4: add block mapping tracepoints for iomap buffered I/O path
From: Ojaswin Mujoo @ 2026-05-27 13:14 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-16-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:35PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Add tracepoints for iomap buffered read, write, partial block zeroing,
> and writeback operations to help debug the iomap buffered I/O path.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Regards,
Ojaswin
> ---
> fs/ext4/inode.c | 6 +++++
> include/trace/events/ext4.h | 45 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e0dae2501292..239d387ffaf2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3961,6 +3961,8 @@ static int ext4_iomap_buffered_read_begin(struct inode *inode, loff_t offset,
> if (ret < 0)
> return ret;
>
> + trace_ext4_iomap_buffered_read_begin(inode, &map, offset, length,
> + flags);
> ext4_set_iomap(inode, iomap, &map, offset, length, flags);
> return 0;
> }
> @@ -4034,6 +4036,8 @@ static int ext4_iomap_buffered_do_write_begin(struct inode *inode,
> if (ret < 0)
> return ret;
>
> + trace_ext4_iomap_buffered_write_begin(inode, &map, offset, length,
> + flags);
> ext4_set_iomap(inode, iomap, &map, offset, length, flags);
> return 0;
> }
> @@ -4136,6 +4140,7 @@ static int ext4_iomap_zero_begin(struct inode *inode,
> map.m_len = (start >> blkbits) - map.m_lblk;
> }
>
> + trace_ext4_iomap_zero_begin(inode, &map, offset, length, flags);
> ext4_set_iomap(inode, iomap, &map, offset, length, flags);
> iomap->flags |= iomap_flags;
>
> @@ -4308,6 +4313,7 @@ static int ext4_iomap_map_writeback_range(struct iomap_writepage_ctx *wpc,
> return ret;
> }
> out:
> + trace_ext4_iomap_map_writeback_range(inode, &map, offset, dirty_len, 0);
> ext4_set_iomap(inode, &wpc->iomap, &map, offset, dirty_len, 0);
> return 0;
> }
> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index f493642cf121..ebafa06cd191 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
> @@ -3096,6 +3096,51 @@ TRACE_EVENT(ext4_move_extent_exit,
> __entry->ret)
> );
>
> +DECLARE_EVENT_CLASS(ext4_set_iomap_class,
> + TP_PROTO(struct inode *inode, struct ext4_map_blocks *map,
> + loff_t offset, loff_t length, unsigned int flags),
> + TP_ARGS(inode, map, offset, length, flags),
> + TP_STRUCT__entry(
> + __field(dev_t, dev)
> + __field(u64, ino)
> + __field(ext4_lblk_t, m_lblk)
> + __field(unsigned int, m_len)
> + __field(unsigned int, m_flags)
> + __field(u64, m_seq)
> + __field(loff_t, offset)
> + __field(loff_t, length)
> + __field(unsigned int, iomap_flags)
> + ),
> + TP_fast_assign(
> + __entry->dev = inode->i_sb->s_dev;
> + __entry->ino = inode->i_ino;
> + __entry->m_lblk = map->m_lblk;
> + __entry->m_len = map->m_len;
> + __entry->m_flags = map->m_flags;
> + __entry->m_seq = map->m_seq;
> + __entry->offset = offset;
> + __entry->length = length;
> + __entry->iomap_flags = flags;
> +
> + ),
> + TP_printk("dev %d:%d ino %llu m_lblk %u m_len %u m_flags %s m_seq %llu orig_off 0x%llx orig_len 0x%llx iomap_flags 0x%x",
> + MAJOR(__entry->dev), MINOR(__entry->dev),
> + __entry->ino, __entry->m_lblk, __entry->m_len,
> + show_mflags(__entry->m_flags), __entry->m_seq,
> + __entry->offset, __entry->length, __entry->iomap_flags)
> +)
> +
> +#define DEFINE_SET_IOMAP_EVENT(name) \
> +DEFINE_EVENT(ext4_set_iomap_class, name, \
> + TP_PROTO(struct inode *inode, struct ext4_map_blocks *map, \
> + loff_t offset, loff_t length, unsigned int flags), \
> + TP_ARGS(inode, map, offset, length, flags))
> +
> +DEFINE_SET_IOMAP_EVENT(ext4_iomap_buffered_read_begin);
> +DEFINE_SET_IOMAP_EVENT(ext4_iomap_buffered_write_begin);
> +DEFINE_SET_IOMAP_EVENT(ext4_iomap_map_writeback_range);
> +DEFINE_SET_IOMAP_EVENT(ext4_iomap_zero_begin);
> +
> #endif /* _TRACE_EXT4_H */
>
> /* This part must be outside protection */
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 16/23] ext4: disable online defrag when inode using iomap buffered I/O path
From: Ojaswin Mujoo @ 2026-05-27 13:14 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-17-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:36PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Online defragmentation does not currently support inodes using the
> iomap buffered I/O path. The existing implementation relies on
> buffer_head for sub-folio block management and data=ordered mode for
> data consistency, both of which are incompatible with the iomap path.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Regards,
Ojaswin
> ---
> fs/ext4/move_extent.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 3329b7ad5dbd..f707a1096544 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -476,6 +476,17 @@ static int mext_check_validity(struct inode *orig_inode,
> return -EOPNOTSUPP;
> }
>
> + /*
> + * TODO: support online defrag for inodes that using the buffered
> + * I/O iomap path.
> + */
> + if (ext4_inode_buffered_iomap(orig_inode) ||
> + ext4_inode_buffered_iomap(donor_inode)) {
> + ext4_msg(sb, KERN_ERR,
> + "Online defrag not supported for inode with iomap buffered IO path");
> + return -EOPNOTSUPP;
> + }
> +
> if (donor_inode->i_mode & (S_ISUID|S_ISGID)) {
> ext4_debug("ext4 move extent: suid or sgid is set to donor file [ino:orig %llu, donor %llu]\n",
> orig_inode->i_ino, donor_inode->i_ino);
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 17/23] ext4: submit zeroed post-EOF data immediately in the iomap buffered I/O path
From: Ojaswin Mujoo @ 2026-05-27 13:41 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-18-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:37PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In the generic buffered_head I/O path, we rely on the data=order mode to
> ensure that the zeroed EOF block data is written before updating
> i_disksize, thus preventing stale data from being exposed.
>
> However, the iomap buffered I/O path cannot use this mechanism. Instead,
> we issue the I/O immediately after performing the zero operation
> (without synchronous waiting for performance). This can reduce the risk
> of exposing stale data, but it does not guarantee that the zero data
> will be flushed to disk before the metadata of i_disksize is updated.
> The subsequent patches will wait for this I/O to complete before
> updating i_disksize.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
I think we discussed that we may not need to do this [1] but I guess
you've decided to make the tradeoff of issuing the IO to avoid having to
wait for bg flush to complete the tail page zeroing
However, I think one side effect might be many threads calling the
writeback mechanism to issue zero IOs which might not scale well. I
don't know if it'll be a huge problem though, I guess it's a sort of
thing we will have to deal with in case we see it in real world
workloads.
[1] https://lore.kernel.org/linux-ext4/yhy4cgc4fnk7tzfejuhy6m6ljo425ebpg6khss6vtvpidg6lyp@5xcyabxrl6zm/
> ---
> fs/ext4/inode.c | 66 ++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 239d387ffaf2..e013aeb03d7b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4742,6 +4742,32 @@ static int ext4_block_zero_range(struct inode *inode,
> zero_written);
> }
>
> +static int ext4_iomap_submit_zero_block(struct inode *inode,
> + loff_t from, loff_t end)
> +{
> + struct address_space *mapping = inode->i_mapping;
> + struct folio *folio;
> + bool do_submit = false;
> +
> + folio = filemap_lock_folio(mapping, from >> PAGE_SHIFT);
> + if (IS_ERR(folio))
> + /* Already writeback and clear? */
> + return PTR_ERR(folio) == -ENOENT ? 0 : PTR_ERR(folio);
> +
> + folio_wait_writeback(folio);
> + WARN_ON_ONCE(folio_test_writeback(folio));
> +
> + if (likely(folio_test_dirty(folio)))
> + do_submit = true;
> + folio_unlock(folio);
> + folio_put(folio);
> +
> + /* Submit zeroed block. */
> + if (do_submit)
> + return filemap_fdatawrite_range(mapping, from, end - 1);
> + return 0;
> +}
> +
> /*
> * 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.
> @@ -4765,8 +4791,10 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> if (IS_ENCRYPTED(inode) && !fscrypt_has_encryption_key(inode))
> return 0;
>
> - if (length > blocksize - offset)
> + if (length > blocksize - offset) {
> length = blocksize - offset;
> + end = from + length;
> + }
>
> err = ext4_block_zero_range(inode, from, length,
> &did_zero, &zero_written);
> @@ -4781,18 +4809,34 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> * TODO: In the iomap path, handle this by updating i_disksize to
> * i_size after the zeroed data has been written back.
> */
> - if (ext4_should_order_data(inode) &&
> - did_zero && zero_written && !IS_DAX(inode)) {
> - handle_t *handle;
> + if (did_zero && zero_written && !IS_DAX(inode)) {
> + if (ext4_should_order_data(inode)) {
> + handle_t *handle;
>
> - handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> - if (IS_ERR(handle))
> - return PTR_ERR(handle);
> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
>
> - err = ext4_jbd2_inode_add_write(handle, inode, from, length);
> - ext4_journal_stop(handle);
> - if (err)
> - return err;
> + err = ext4_jbd2_inode_add_write(handle, inode, from,
> + length);
> + ext4_journal_stop(handle);
> + if (err)
> + return err;
> + /*
> + * inodes using the iomap buffered I/O path do not use the
> + * data=ordered mode. We submit zeroed range directly here.
> + * Do not wait for I/O completion for performance.
> + *
> + * TODO: Any operation that extends i_disksize (including
> + * append write end io past the zeroed boundary, truncate up,
> + * and append fallocate) must wait for the relevant I/O to
> + * complete before updating i_disksize.
> + */
> + } else if (ext4_inode_buffered_iomap(inode)) {
> + err = ext4_iomap_submit_zero_block(inode, from, end);
> + if (err)
> + return err;
> + }
> }
>
> return 0;
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 18/23] ext4: wait for ordered I/O in the iomap buffered I/O path
From: Ojaswin Mujoo @ 2026-05-27 15:58 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ritesh.list, djwong, hch, yi.zhang, yizhang089,
yangerkun, yukuai
In-Reply-To: <20260511072344.191271-19-yi.zhang@huaweicloud.com>
On Mon, May 11, 2026 at 03:23:38PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> For append writes, wait for ordered I/O to complete before updating
> i_disksize. This ensures that zeroed data is flushed to disk before the
> metadata update, preventing stale data from being exposed during
> unaligned post-EOF append writes.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
> fs/ext4/ext4.h | 11 +++++++
> fs/ext4/inode.c | 80 ++++++++++++++++++++++++++++++++++++++++++-----
> fs/ext4/page-io.c | 60 +++++++++++++++++++++++++++++++++++
> fs/ext4/super.c | 23 ++++++++++----
> 4 files changed, 161 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 078feda47e36..9ce2128eea3e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1195,6 +1195,15 @@ struct ext4_inode_info {
> #ifdef CONFIG_FS_ENCRYPTION
> struct fscrypt_inode_info *i_crypt_info;
> #endif
> +
> + /*
> + * Track ordered zeroed data during post-EOF append writes, fallocate,
> + * and truncate-up operations. These parameters are used only in the
> + * iomap buffered I/O path.
> + */
> + ext4_lblk_t i_ordered_lblk;
> + ext4_lblk_t i_ordered_len;
> + wait_queue_head_t i_ordered_wq;
> };
>
> /*
> @@ -3858,6 +3867,8 @@ extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
> __u64 len, __u64 *moved_len);
>
> /* page-io.c */
> +#define EXT4_IOMAP_IOEND_ORDER_IO 1UL /* This I/O is an ordered one */
> +
> extern int __init ext4_init_pageio(void);
> extern void ext4_exit_pageio(void);
> extern ext4_io_end_t *ext4_init_io_end(struct inode *inode, gfp_t flags);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e013aeb03d7b..11fb369efeb1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4345,6 +4345,7 @@ static int ext4_iomap_writeback_submit(struct iomap_writepage_ctx *wpc,
> {
> struct iomap_ioend *ioend = wpc->wb_ctx;
> struct ext4_inode_info *ei = EXT4_I(ioend->io_inode);
> + ext4_lblk_t start, end, order_lblk, order_len;
>
> /*
> * After I/O completion, a worker needs to be scheduled when:
> @@ -4357,6 +4358,30 @@ static int ext4_iomap_writeback_submit(struct iomap_writepage_ctx *wpc,
> test_opt(ioend->io_inode->i_sb, DATA_ERR_ABORT))
> ioend->io_bio.bi_end_io = ext4_iomap_end_bio;
>
> + /*
> + * Mark the I/O as ordered. Ordered I/O requires separate endio
> + * handling and must not be merged with regular I/O operations.
> + */
> + order_len = READ_ONCE(ei->i_ordered_len);
> + if (order_len) {
> + /*
> + * Pair with smp_store_release() in ext4_block_zero_eof().
> + * Ensure we see the updated i_ordered_lblk that was written
> + * before the release store to i_ordered_len.
> + */
> + smp_rmb();
> + order_lblk = READ_ONCE(ei->i_ordered_lblk);
> + start = ioend->io_offset >> ioend->io_inode->i_blkbits;
> + end = EXT4_B_TO_LBLK(ioend->io_inode,
> + ioend->io_offset + ioend->io_size);
> +
> + if (start <= order_lblk && end >= order_lblk + order_len) {
Hi Zhang,
I guess this check is enough cause ordered_lblk and ordered_len will
always be contained in a single block.
> + ioend->io_bio.bi_end_io = ext4_iomap_end_bio;
> + ioend->io_private = (void *)EXT4_IOMAP_IOEND_ORDER_IO;
> + ioend->io_flags |= IOMAP_IOEND_BOUNDARY;
FWIU, we are wanting the ordered IO to not be merged and submitted asap
since we want to wake up the waiters. Is there any other reason?
Adding the boundary in ->writeback_submit() only affects
iomap_ioend_can_merge() which happens after we have woken up the waiters
and deferred the IO to the wq. We ideally want it affect
iomap_can_add_to_ioend() ie we need to add IOMAP_F_BOUNDARY in
->writeback_range().
Secondly, I don't think boundary is the right flag here. It ensures
that everything before the ordered iomap gets submitted and the ordered
iomap starts a new ioend. This can still keep getting merged with the
newer ioends untils we decide to submit the IO, which can delay waking
up the waiters. If we really want the "no merge" behavior, we'll have to
do something like [1] (Check the 2 NOMERGE flag patches).
> + }
> + }
> +
> return iomap_ioend_writeback_submit(wpc, error);
> }
>
> @@ -4746,8 +4771,10 @@ static int ext4_iomap_submit_zero_block(struct inode *inode,
> loff_t from, loff_t end)
> {
> struct address_space *mapping = inode->i_mapping;
> + struct ext4_inode_info *ei = EXT4_I(inode);
> struct folio *folio;
> bool do_submit = false;
> + int ret;
>
> folio = filemap_lock_folio(mapping, from >> PAGE_SHIFT);
> if (IS_ERR(folio))
> @@ -4757,14 +4784,50 @@ static int ext4_iomap_submit_zero_block(struct inode *inode,
> folio_wait_writeback(folio);
> WARN_ON_ONCE(folio_test_writeback(folio));
>
> - if (likely(folio_test_dirty(folio)))
> + /*
> + * Mark the ordered range. It will be cleared upon I/O completion
> + * in ext4_iomap_end_bio(). Any operation that extends i_disksize
> + * (including append write end io past the zeroed boundary,
> + * truncate up and append fallocate) must wait for this I/O to
> + * complete before updating i_disksize.
> + *
> + * When multiple overlapping unaligned EOF writes are in flight, we
> + * only need to track and wait for the first one. Subsequent writes
> + * will zero the gap in memory and ensure that the zeroed data is
> + * written out along with the valid data in the same block before
> + * i_disksize is updated.
> + */
> + if (likely(folio_test_dirty(folio) &&
> + READ_ONCE(ei->i_ordered_len) == 0)) {
> + WRITE_ONCE(ei->i_ordered_lblk,
> + from >> inode->i_blkbits);
> + /*
> + * Pairs with smp_rmb() in ext4_iomap_writeback_submit()
> + * and ext4_iomap_wb_ordered_wait(). Ensure the updated
> + * i_ordered_lblk is visible when i_ordered_len becomes
> + * non-zero.
> + */
> + smp_store_release(&ei->i_ordered_len, 1);
> do_submit = true;
> + }
> folio_unlock(folio);
> folio_put(folio);
>
> /* Submit zeroed block. */
> - if (do_submit)
> - return filemap_fdatawrite_range(mapping, from, end - 1);
> + if (do_submit) {
> + ret = filemap_fdatawrite_range(mapping, from, end - 1);
> + if (ret) {
> + /*
> + * Pairs with wait_event() in
> + * ext4_iomap_wb_ordered_wait(). Ensure
> + * i_ordered_len = 0 is visible before waking up
> + * waiters.
> + */
> + smp_store_release(&ei->i_ordered_len, 0);
> + wake_up_all(&ei->i_ordered_wq);
> + return ret;
> + }
> + }
> return 0;
> }
>
> @@ -4827,10 +4890,13 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> * data=ordered mode. We submit zeroed range directly here.
> * Do not wait for I/O completion for performance.
> *
> - * TODO: Any operation that extends i_disksize (including
> - * append write end io past the zeroed boundary, truncate up,
> - * and append fallocate) must wait for the relevant I/O to
> - * complete before updating i_disksize.
> + * The end_io handler ext4_iomap_wb_ordered_wait() will wait
> + * for I/O completion before updating i_disksize if the write
> + * extends beyond the zeroed boundary.
> + *
> + * TODO: Any other operation that extends i_disksize
> + * (including truncate up and append fallocate) must wait for
> + * the relevant I/O to complete before updating i_disksize.
> */
> } else if (ext4_inode_buffered_iomap(inode)) {
> err = ext4_iomap_submit_zero_block(inode, from, end);
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 3050c887329f..ad05ebb49bf6 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -613,6 +613,46 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio,
> return 0;
> }
>
> +/*
> + * If the old disk size is not block size aligned and the current
> + * writeback range is entirely beyond the old EOF block, we should
> + * wait for the zeroed data written in ext4_block_zero_eof() to be
> + * written out, otherwise, it may expose stale data in that block.
> + */
> +static void ext4_iomap_wb_ordered_wait(struct inode *inode,
> + loff_t pos, loff_t end)
> +{
> + struct ext4_inode_info *ei = EXT4_I(inode);
> + unsigned int blocksize = i_blocksize(inode);
> + loff_t disksize = READ_ONCE(ei->i_disksize);
> + ext4_lblk_t order_lblk, order_len;
> +
> + /*
> + * Waiting for ordered I/O is unnecessary when:
> + * - The on-disk size is block-aligned (no stale data exists).
> + * - The write start is within the block of the old EOF
> + * (overwriting, or appending to a block that already contains
> + * valid data).
> + */
> + if (!(disksize & (blocksize - 1)) ||
> + pos < round_up(disksize, blocksize))
> + return;
> +
> + order_len = READ_ONCE(ei->i_ordered_len);
> + if (!order_len)
> + return;
> +
> + /*
> + * Pair with smp_store_release() in ext4_iomap_end_bio() and
> + * ext4_block_zero_eof(). Ensure we see the updated i_ordered_lblk
> + * that was written before the release store to i_ordered_len.
> + */
> + smp_rmb();
> + order_lblk = READ_ONCE(ei->i_ordered_lblk);
> + if ((pos >> inode->i_blkbits) >= order_lblk + order_len)
> + wait_event(ei->i_ordered_wq, READ_ONCE(ei->i_ordered_len) == 0);
> +}
> +
> static int ext4_iomap_wb_update_disksize(handle_t *handle, struct inode *inode,
> loff_t end)
> {
> @@ -656,6 +696,9 @@ static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
> goto out;
> }
>
> + /* Wait ordered zero data to be written out. */
> + ext4_iomap_wb_ordered_wait(inode, pos, pos + size);
> +
> /* We may need to convert one extent and dirty the inode. */
> credits = ext4_chunk_trans_blocks(inode,
> EXT4_MAX_BLOCKS(size, pos, inode->i_blkbits));
> @@ -717,8 +760,25 @@ void ext4_iomap_end_bio(struct bio *bio)
> struct inode *inode = ioend->io_inode;
> struct ext4_inode_info *ei = EXT4_I(inode);
> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> + unsigned long io_mode = (unsigned long)ioend->io_private;
> unsigned long flags;
>
> + /*
> + * This is an ordered I/O, clear the ordered range set in
> + * ext4_block_zero_eof() and wake up all waiters that will update
> + * the inode i_disksize.
> + */
> + if (io_mode == EXT4_IOMAP_IOEND_ORDER_IO) {
> + /*
> + * Pairs with wait_event() in ext4_iomap_wb_ordered_wait().
> + * Ensure i_ordered_len = 0 is visible before waking up
> + * waiters.
> + */
> + smp_store_release(&ei->i_ordered_len, 0);
> + wake_up_all(&ei->i_ordered_wq);
> + goto defer;
> + }
> +
> /* Needs to convert unwritten extents or update the i_disksize. */
> if ((ioend->io_flags & IOMAP_IOEND_UNWRITTEN) ||
> ioend->io_offset + ioend->io_size > READ_ONCE(ei->i_disksize))
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 62bfe05a64bc..9c0a00e716f3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1444,6 +1444,9 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
> ext4_fc_init_inode(&ei->vfs_inode);
> spin_lock_init(&ei->i_fc_lock);
> mmb_init(&ei->i_metadata_bhs, &ei->vfs_inode.i_data);
> + ei->i_ordered_lblk = 0;
> + ei->i_ordered_len = 0;
> + init_waitqueue_head(&ei->i_ordered_wq);
> return &ei->vfs_inode;
> }
>
> @@ -1480,12 +1483,20 @@ static void ext4_destroy_inode(struct inode *inode)
> dump_stack();
> }
>
> - if (!(EXT4_SB(inode->i_sb)->s_mount_state & EXT4_ERROR_FS) &&
> - WARN_ON_ONCE(EXT4_I(inode)->i_reserved_data_blocks))
> - ext4_msg(inode->i_sb, KERN_ERR,
> - "Inode %llu (%p): i_reserved_data_blocks (%u) not cleared!",
> - inode->i_ino, EXT4_I(inode),
> - EXT4_I(inode)->i_reserved_data_blocks);
> + if (!(EXT4_SB(inode->i_sb)->s_mount_state & EXT4_ERROR_FS)) {
> + if (WARN_ON_ONCE(EXT4_I(inode)->i_reserved_data_blocks))
> + ext4_msg(inode->i_sb, KERN_ERR,
> + "Inode %llu (%p): i_reserved_data_blocks (%u) not cleared!",
> + inode->i_ino, EXT4_I(inode),
> + EXT4_I(inode)->i_reserved_data_blocks);
> +
> + if (WARN_ON_ONCE(EXT4_I(inode)->i_ordered_len))
> + ext4_msg(inode->i_sb, KERN_ERR,
> + "Inode %llu (%p): i_ordered_lblk (%u) and i_ordered_len (%u) not cleared!",
> + inode->i_ino, EXT4_I(inode),
> + EXT4_I(inode)->i_ordered_lblk,
> + EXT4_I(inode)->i_ordered_len);
> + }
> }
>
> static void ext4_shutdown(struct super_block *sb)
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH 04/17] nilfs2: replace get_zeroed_page() with kzalloc()
From: Ryusuke Konishi @ 2026-05-27 16:02 UTC (permalink / raw)
To: Mike Rapoport (Microsoft)
Cc: Viacheslav Dubeyko, Jan Kara, Mark Fasheh, Joel Becker, Joseph Qi,
Viacheslav Dubeyko, Trond Myklebust, Anna Schumaker, Chuck Lever,
Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Alexander Viro, Christian Brauner, Jan Kara, Dave Kleikamp,
Theodore Ts'o, Miklos Szeredi, Andreas Hindborg, Breno Leitao,
Kees Cook, Tigran A. Aivazian, linux-kernel, linux-fsdevel,
ocfs2-devel, linux-nilfs, linux-nfs, jfs-discussion, linux-ext4,
linux-mm
In-Reply-To: <1bb537f6dc36b00788b613fb8f71579478418457.camel@redhat.com>
On Tue, May 26, 2026 at 2:07 AM Viacheslav Dubeyko wrote:
>
> On Sat, 2026-05-23 at 20:54 +0300, Mike Rapoport (Microsoft) wrote:
> > nilfs_ioctl_wrap_copy() allocates a temporary buffer with
> > get_zeroed_page().
> >
> > kzalloc() is a better API for such use and it also provides better
> > scalability and more debugging possibilities.
> >
> > Replace use of get_zeroed_page() with kzalloc().
> >
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
> > fs/nilfs2/ioctl.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> > index e0a606643e87..b73f2c5d10f0 100644
> > --- a/fs/nilfs2/ioctl.c
> > +++ b/fs/nilfs2/ioctl.c
> > @@ -69,7 +69,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
> > if (argv->v_index > ~(__u64)0 - argv->v_nmembs)
> > return -EINVAL;
> >
> > - buf = (void *)get_zeroed_page(GFP_NOFS);
> > + buf = kzalloc(PAGE_SIZE, GFP_NOFS);
> > if (unlikely(!buf))
> > return -ENOMEM;
> > maxmembs = PAGE_SIZE / argv->v_size;
> > @@ -107,7 +107,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
> > }
> > argv->v_nmembs = total;
> >
> > - free_pages((unsigned long)buf, 0);
> > + kfree(buf);
> > return ret;
> > }
> >
>
> Makes sense to me.
>
> Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
>
> Thanks,
> Slava.
Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
This conversion looks reasonable and won't affect the behavior of the
ioctls that use the modified function.
Thanks,
Ryusuke Konishi
^ permalink raw reply
* Re: [PATCH v2 1/2] ext4: avoid RWM atomic in EXT4_MB_GRP_TEST_AND_SET_READ
From: Andreas Dilger @ 2026-05-27 19:46 UTC (permalink / raw)
To: Bohdan Trach
Cc: Theodore Ts'o, Baokun Li, Jan Kara, Ojaswin Mujoo,
Ritesh Harjani (IBM), Zhang Yi, mchehab+huawei, bohdan.trach,
lilith.oberhauser, linux-ext4, linux-kernel
In-Reply-To: <20260527090329.2680170-2-bohdan.trach@huaweicloud.com>
On May 27, 2026, at 03:03, Bohdan Trach <bohdan.trach@huaweicloud.com> wrote:
>
> EXT4_MB_GRP_TEST_AND_SET_READ uses test_and_set_bit function which
> issues an atomic write. This can cause high overhead due to cache
> contention when multiple threads iterate over groups in a tight loop,
> as is the case for ext4_mb_prefetch(). We have seen this to be a
> problem for Kunpeng 920b CPUs which uses a single ARM LSE instruction
> for this purpose.
>
> Avoid this unconditional atomic write by testing the bit first without
> changing its value. This is OK for this use case as this bit is never
> unset.
>
> This change significantly reduces costs of fallocate() operations which
> trigger linear group scans on large multicore machines where
> test_and_set_bit issues an atomic write operation unconditionally.
>
> Signed-off-by: Bohdan Trach <bohdan.trach@huaweicloud.com>
Thanks for the patch. Definitely the benchmarks in the 0/2 email show
significant gains for the Kunpeng system, and reducing contention makes sense
as core counts increase and the likely case is that the bit is already set.
That said, I wonder if this should (also/instead) be put into test_and_set_bit()
itself, or add test_and_unlikely_set_bit() or test_and_rarely_set_bit()
(or similar) optimized for the case where the bit is likely to already be set.
I see in your benchmarking that there is not "apples-to-apples" comparisons for
ARM(Kunpeng) vs. AMD on the same storage. The storage hardware and space usage
is different for each test run, and the ARM numbers show only marginal gains and
more negative than positive results at all thread counts:
> Benchmark on an existing file system for AMD 9654 (15T FS, 6% space
> used), kernel 7.1-rc3. This shows the performance impact on a mostly
> free file system.
> | thr. | base | patched | improv. |
> | | perf | perf | |
> |------|-------|---------|------------|
> | 1 | 30901 | 31191 | +0.9384810 |
> | 2 | 50874 | 50504 | -0.7272870 |
> | 4 | 66068 | 64108 | -2.9666404 |
> | 8 | 63963 | 61927 | -3.1830902 |
> | 16 | 47809 | 47044 | -1.6001171 |
> | 32 | 42441 | 42326 | -0.2709644 |
> | 64 | 39773 | 39929 | +0.3922259 |
> | 128 | 37065 | 36413 | -1.7590719 |
The performance reduction might be caused by the now double memory access on
AMD that is only adding overhead on that CPU implementation? It would be useful
to see the testing on Kunpeng vs. AMD/Intel on the same storage device/usage.
That would tell us if it is more appropriate to optimize this in the aarch64
test_and_set_bit() rather than in ext4.
Cheers, Andreas
> ---
> fs/ext4/ext4.h | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 56b82d4a15d7..f8eacf1375f8 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3551,7 +3551,13 @@ struct ext4_group_info {
> #define EXT4_MB_GRP_CLEAR_TRIMMED(grp) \
> (clear_bit(EXT4_GROUP_INFO_WAS_TRIMMED_BIT, &((grp)->bb_state)))
> #define EXT4_MB_GRP_TEST_AND_SET_READ(grp) \
> - (test_and_set_bit(EXT4_GROUP_INFO_BBITMAP_READ_BIT, &((grp)->bb_state)))
> + (ext4_mb_grp_test_and_set_read((grp)))
> +
> +static inline int ext4_mb_grp_test_and_set_read(struct ext4_group_info *grp)
> +{
> + return (test_bit(EXT4_GROUP_INFO_BBITMAP_READ_BIT, &grp->bb_state) ||
> + test_and_set_bit(EXT4_GROUP_INFO_BBITMAP_READ_BIT, &grp->bb_state));
> +}
>
> #define EXT4_MAX_CONTENTION 8
> #define EXT4_CONTENTION_THRESHOLD 2
> --
> 2.43.0
>
Cheers, Andreas
^ permalink raw reply
* Re: [PATCH v2 2/2] ext4: get ext4_group_desc in ext4_mb_prefetch only when necessary
From: Andreas Dilger @ 2026-05-27 19:49 UTC (permalink / raw)
To: Bohdan Trach
Cc: Theodore Ts'o, Baokun Li, Jan Kara, Ojaswin Mujoo,
Ritesh Harjani (IBM), Zhang Yi, mchehab+huawei, bohdan.trach,
lilith.oberhauser, linux-ext4, linux-kernel
In-Reply-To: <20260527090329.2680170-3-bohdan.trach@huaweicloud.com>
On May 27, 2026, at 03:03, Bohdan Trach <bohdan.trach@huaweicloud.com> wrote:
>
> Getting ext4_group_desc structure can contribute to the cost of
> ext4_mb_prefetch() without any need, as most groups fail the
> !EXT4_MB_GRP_TEST_AND_SET_READ check.
>
> Optimize ext4_mb_prefetch by getting the group description only when
> necessary.
>
> The result is further increase in performance of fallocate() system call
> path that triggers ext4_mb_prefetch() via a linear group scan.
>
> Signed-off-by: Bohdan Trach <bohdan.trach@huaweicloud.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
This looks reasonable, and is independent of the EXT4_MB_GRP_TEST_AND_SET_READ()
micro-optimization in the 1/2 patch.
Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>
Cheers, Andreas
^ permalink raw reply
* Re: [PATCH] jbd2: Remove special jbd2 slabs
From: Tal Zussman @ 2026-05-27 20:33 UTC (permalink / raw)
To: Matthew Wilcox (Oracle), Theodore Ts'o
Cc: Jan Kara, linux-ext4, linux-fsdevel, Mike Rapoport (Microsoft),
Vlastimil Babka
In-Reply-To: <20260525201321.21717-1-willy@infradead.org>
On 5/25/26 4:13 PM, Matthew Wilcox (Oracle) wrote:
Hi,
One small comment below.
> When jbd2 was originally written, kmalloc() would not guarantee alignment
> for the requested memory. Since commit 59bb47985c1d in 2019, kmalloc
> has guaranteed natural alignment for power-of-two allocations. We can
> now remove the jbd2 special slabs and just use kmalloc() directly.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/jbd2/commit.c | 8 ++-
> fs/jbd2/journal.c | 121 ++----------------------------------------
> fs/jbd2/transaction.c | 8 +--
> include/linux/jbd2.h | 3 --
> 4 files changed, 11 insertions(+), 129 deletions(-)
>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 38f318bb4279..2e8dbc4547bb 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -514,10 +514,8 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> * leave undo-committed data.
> */
> if (jh->b_committed_data) {
> - struct buffer_head *bh = jh2bh(jh);
> -
> spin_lock(&jh->b_state_lock);
> - jbd2_free(jh->b_committed_data, bh->b_size);
> + kfree(jh->b_committed_data);
> jh->b_committed_data = NULL;
> spin_unlock(&jh->b_state_lock);
> }
> @@ -978,7 +976,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> * its triggers if they exist, so we can clear that too.
> */
> if (jh->b_committed_data) {
> - jbd2_free(jh->b_committed_data, bh->b_size);
> + kfree(jh->b_committed_data);
> jh->b_committed_data = NULL;
> if (jh->b_frozen_data) {
> jh->b_committed_data = jh->b_frozen_data;
> @@ -986,7 +984,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> jh->b_frozen_triggers = NULL;
> }
> } else if (jh->b_frozen_data) {
> - jbd2_free(jh->b_frozen_data, bh->b_size);
> + kfree(jh->b_frozen_data);
> jh->b_frozen_data = NULL;
> jh->b_frozen_triggers = NULL;
> }
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index a6616380ce38..ad10c8a92fa0 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -95,8 +95,6 @@ EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
> EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
> EXPORT_SYMBOL(jbd2_inode_cache);
>
> -static int jbd2_journal_create_slab(size_t slab_size);
> -
> #ifdef CONFIG_JBD2_DEBUG
> void __jbd2_debug(int level, const char *file, const char *func,
> unsigned int line, const char *fmt, ...)
> @@ -385,10 +383,10 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
> goto escape_done;
>
> spin_unlock(&jh_in->b_state_lock);
> - tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL);
> + tmp = kmalloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL);
> spin_lock(&jh_in->b_state_lock);
> if (jh_in->b_frozen_data) {
> - jbd2_free(tmp, bh_in->b_size);
> + kfree(tmp);
> goto copy_done;
> }
>
> @@ -2063,14 +2061,6 @@ EXPORT_SYMBOL(jbd2_journal_update_sb_errno);
> int jbd2_journal_load(journal_t *journal)
> {
> int err;
> - journal_superblock_t *sb = journal->j_superblock;
> -
> - /*
> - * Create a slab for this blocksize
> - */
> - err = jbd2_journal_create_slab(be32_to_cpu(sb->s_blocksize));
> - if (err)
> - return err;
>
> /* Let the recovery code check whether it needs to recover any
> * data from the journal. */
> @@ -2698,108 +2688,6 @@ size_t journal_tag_bytes(journal_t *journal)
> return sz - sizeof(__u32);
> }
>
> -/*
> - * JBD memory management
> - *
> - * These functions are used to allocate block-sized chunks of memory
> - * used for making copies of buffer_head data. Very often it will be
> - * page-sized chunks of data, but sometimes it will be in
> - * sub-page-size chunks. (For example, 16k pages on Power systems
> - * with a 4k block file system.) For blocks smaller than a page, we
> - * use a SLAB allocator. There are slab caches for each block size,
> - * which are allocated at mount time, if necessary, and we only free
> - * (all of) the slab caches when/if the jbd2 module is unloaded. For
> - * this reason we don't need to a mutex to protect access to
> - * jbd2_slab[] allocating or releasing memory; only in
> - * jbd2_journal_create_slab().
> - */
> -#define JBD2_MAX_SLABS 8
> -static struct kmem_cache *jbd2_slab[JBD2_MAX_SLABS];
> -
> -static const char *jbd2_slab_names[JBD2_MAX_SLABS] = {
> - "jbd2_1k", "jbd2_2k", "jbd2_4k", "jbd2_8k",
> - "jbd2_16k", "jbd2_32k", "jbd2_64k", "jbd2_128k"
> -};
> -
> -
> -static void jbd2_journal_destroy_slabs(void)
> -{
> - int i;
> -
> - for (i = 0; i < JBD2_MAX_SLABS; i++) {
> - kmem_cache_destroy(jbd2_slab[i]);
> - jbd2_slab[i] = NULL;
> - }
> -}
> -
> -static int jbd2_journal_create_slab(size_t size)
> -{
> - static DEFINE_MUTEX(jbd2_slab_create_mutex);
> - int i = order_base_2(size) - 10;
> - size_t slab_size;
> -
> - if (size == PAGE_SIZE)
> - return 0;
> -
> - if (i >= JBD2_MAX_SLABS)
> - return -EINVAL;
> -
> - if (unlikely(i < 0))
> - i = 0;
> - mutex_lock(&jbd2_slab_create_mutex);
> - if (jbd2_slab[i]) {
> - mutex_unlock(&jbd2_slab_create_mutex);
> - return 0; /* Already created */
> - }
> -
> - slab_size = 1 << (i+10);
> - jbd2_slab[i] = kmem_cache_create(jbd2_slab_names[i], slab_size,
> - slab_size, 0, NULL);
> - mutex_unlock(&jbd2_slab_create_mutex);
> - if (!jbd2_slab[i]) {
> - printk(KERN_EMERG "JBD2: no memory for jbd2_slab cache\n");
> - return -ENOMEM;
> - }
> - return 0;
> -}
> -
> -static struct kmem_cache *get_slab(size_t size)
> -{
> - int i = order_base_2(size) - 10;
> -
> - BUG_ON(i >= JBD2_MAX_SLABS);
> - if (unlikely(i < 0))
> - i = 0;
> - BUG_ON(jbd2_slab[i] == NULL);
> - return jbd2_slab[i];
> -}
> -
> -void *jbd2_alloc(size_t size, gfp_t flags)
> -{
> - void *ptr;
> -
> - BUG_ON(size & (size-1)); /* Must be a power of 2 */
> -
> - if (size < PAGE_SIZE)
> - ptr = kmem_cache_alloc(get_slab(size), flags);
> - else
> - ptr = (void *)__get_free_pages(flags, get_order(size));
> -
> - /* Check alignment; SLUB has gotten this wrong in the past,
> - * and this can lead to user data corruption! */
> - BUG_ON(((unsigned long) ptr) & (size-1));
> -
> - return ptr;
> -}
> -
> -void jbd2_free(void *ptr, size_t size)
> -{
> - if (size < PAGE_SIZE)
> - kmem_cache_free(get_slab(size), ptr);
> - else
> - free_pages((unsigned long)ptr, get_order(size));
> -};
> -
> /*
> * Journal_head storage management
> */
> @@ -2977,11 +2865,11 @@ static void journal_release_journal_head(struct journal_head *jh, size_t b_size)
I think the b_size parameter can be removed from journal_release_journal_head()
and its single caller now.
> {
> if (jh->b_frozen_data) {
> printk(KERN_WARNING "%s: freeing b_frozen_data\n", __func__);
> - jbd2_free(jh->b_frozen_data, b_size);
> + kfree(jh->b_frozen_data);
> }
> if (jh->b_committed_data) {
> printk(KERN_WARNING "%s: freeing b_committed_data\n", __func__);
> - jbd2_free(jh->b_committed_data, b_size);
> + kfree(jh->b_committed_data);
> }
> journal_free_journal_head(jh);
> }
> @@ -3142,7 +3030,6 @@ static void jbd2_journal_destroy_caches(void)
> jbd2_journal_destroy_handle_cache();
> jbd2_journal_destroy_inode_cache();
> jbd2_journal_destroy_transaction_cache();
> - jbd2_journal_destroy_slabs();
> }
>
> static int __init journal_init(void)
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 4885903bbd10..48ddb566d12d 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1131,7 +1131,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
> if (!frozen_buffer) {
> JBUFFER_TRACE(jh, "allocate memory for buffer");
> spin_unlock(&jh->b_state_lock);
> - frozen_buffer = jbd2_alloc(jh2bh(jh)->b_size,
> + frozen_buffer = kmalloc(jh2bh(jh)->b_size,
> GFP_NOFS | __GFP_NOFAIL);
> goto repeat;
> }
> @@ -1159,7 +1159,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
>
> out:
> if (unlikely(frozen_buffer)) /* It's usually NULL */
> - jbd2_free(frozen_buffer, bh->b_size);
> + kfree(frozen_buffer);
>
> JBUFFER_TRACE(jh, "exit");
> return error;
> @@ -1424,7 +1424,7 @@ int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh)
>
> repeat:
> if (!jh->b_committed_data)
> - committed_data = jbd2_alloc(jh2bh(jh)->b_size,
> + committed_data = kmalloc(jh2bh(jh)->b_size,
> GFP_NOFS|__GFP_NOFAIL);
>
> spin_lock(&jh->b_state_lock);
> @@ -1445,7 +1445,7 @@ int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh)
> out:
> jbd2_journal_put_journal_head(jh);
> if (unlikely(committed_data))
> - jbd2_free(committed_data, bh->b_size);
> + kfree(committed_data);
> return err;
> }
>
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 7e785aa6d35d..b68561187e90 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -63,9 +63,6 @@ void __jbd2_debug(int level, const char *file, const char *func,
> #define jbd2_debug(n, fmt, a...) no_printk(fmt, ##a)
> #endif
>
> -extern void *jbd2_alloc(size_t size, gfp_t flags);
> -extern void jbd2_free(void *ptr, size_t size);
> -
> #define JBD2_MIN_JOURNAL_BLOCKS 1024
> #define JBD2_DEFAULT_FAST_COMMIT_BLOCKS 256
>
^ permalink raw reply
* Re: [PATCH] ext4: add ext4_dir_entry_is_tail()
From: Andreas Dilger @ 2026-05-27 22:26 UTC (permalink / raw)
To: Artem Blagodarenko; +Cc: linux-ext4
In-Reply-To: <20260526233816.7654-1-ablagodarenko@thelustrecollective.com>
On May 26, 2026, at 17:38, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
>
> From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
>
> Replace open-coded checks for directory tail entries with a call
> to ext4_dir_entry_is_tail(). This helper will also be used by
> upcoming changes.
>
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>
> ---
> fs/ext4/ext4.h | 16 ++++++++++++++++
> fs/ext4/namei.c | 7 +------
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 94283a991e5c..01b1222b1454 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3917,6 +3917,22 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
> io_end->flag &= ~EXT4_IO_END_UNWRITTEN;
> }
>
> +/*
> + * ext4_dir_entry_is_tail() - Check if a directory entry is a tail entry.
> + * @de: directory entry to check
> + *
> + * Returns true if @de is a directory block tail entry (checksum record).
> + */
> +static inline bool ext4_dir_entry_is_tail(struct ext4_dir_entry_2 *de)
> +{
> + struct ext4_dir_entry_tail *t = (struct ext4_dir_entry_tail *)de;
> +
> + return !t->det_reserved_zero1 &&
> + le16_to_cpu(t->det_rec_len) == sizeof(*t) &&
> + !t->det_reserved_zero2 &&
> + t->det_reserved_ft == EXT4_FT_DIR_CSUM;
> +}
> +
> extern const struct iomap_ops ext4_iomap_ops;
> extern const struct iomap_ops ext4_iomap_report_ops;
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 4a47fbd8dd30..accf63fbbc79 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -314,7 +314,6 @@ static struct ext4_dir_entry_tail *get_dirent_tail(struct inode *inode,
> struct buffer_head *bh)
> {
> struct ext4_dir_entry_tail *t;
> - int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
>
> #ifdef PARANOID
> struct ext4_dir_entry *d, *top;
> @@ -334,11 +333,7 @@ static struct ext4_dir_entry_tail *get_dirent_tail(struct inode *inode,
> t = EXT4_DIRENT_TAIL(bh->b_data, EXT4_BLOCK_SIZE(inode->i_sb));
> #endif
>
> - if (t->det_reserved_zero1 ||
> - (ext4_rec_len_from_disk(t->det_rec_len, blocksize) !=
> - sizeof(struct ext4_dir_entry_tail)) ||
> - t->det_reserved_zero2 ||
> - t->det_reserved_ft != EXT4_FT_DIR_CSUM)
> + if (!ext4_dir_entry_is_tail((struct ext4_dir_entry_2 *)t))
> return NULL;
>
> return t;
> --
> 2.43.7
>
Cheers, Andreas
^ permalink raw reply
* Re: [PATCH] ext4: replace ext4_dir_entry with ext4_dir_entry_2
From: Andreas Dilger @ 2026-05-27 22:28 UTC (permalink / raw)
To: Artem Blagodarenko; +Cc: Theodore Ts'o, linux-ext4
In-Reply-To: <20260526233608.7600-1-ablagodarenko@thelustrecollective.com>
On May 26, 2026, at 17:36, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
>
> From: Artem Blagodarenko <artem.blagodarenko@gmail.com>
>
> Replace remaining uses of struct ext4_dir_entry in namei.c
> with struct ext4_dir_entry_2.
>
> The code paths affected by this change already depend on the
> filetype feature, so using struct ext4_dir_entry_2 is
> appropriate and avoids mixing the two directory entry types
> unnecessarily.
>
> This change does not affect support for 16-bit rec_len.
>
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>
> ---
> fs/ext4/namei.c | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 4a47fbd8dd30..a316fc2ac41b 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -102,7 +102,7 @@ static struct buffer_head *ext4_append(handle_t *handle,
> }
>
> static int ext4_dx_csum_verify(struct inode *inode,
> - struct ext4_dir_entry *dirent);
> + struct ext4_dir_entry_2 *dirent);
>
> /*
> * Hints to ext4_read_dirblock regarding whether we expect a directory
> @@ -128,7 +128,7 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
> unsigned int line)
> {
> struct buffer_head *bh;
> - struct ext4_dir_entry *dirent;
> + struct ext4_dir_entry_2 *dirent;
> int is_dx_block = 0;
>
> if (block >= inode->i_size >> inode->i_blkbits) {
> @@ -160,7 +160,7 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
> }
> if (!bh)
> return NULL;
> - dirent = (struct ext4_dir_entry *) bh->b_data;
> + dirent = (struct ext4_dir_entry_2 *) bh->b_data;
> /* Determine whether or not we have an index block */
> if (is_dx(inode)) {
> if (block == 0)
> @@ -317,13 +317,13 @@ static struct ext4_dir_entry_tail *get_dirent_tail(struct inode *inode,
> int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
>
> #ifdef PARANOID
> - struct ext4_dir_entry *d, *top;
> + struct ext4_dir_entry_2 *d, *top;
>
> - d = (struct ext4_dir_entry *)bh->b_data;
> - top = (struct ext4_dir_entry *)(bh->b_data +
> + d = (struct ext4_dir_entry_2 *)bh->b_data;
> + top = (struct ext4_dir_entry_2 *)(bh->b_data +
> (blocksize - sizeof(struct ext4_dir_entry_tail)));
> while (d < top && ext4_rec_len_from_disk(d->rec_len, blocksize))
> - d = (struct ext4_dir_entry *)(((void *)d) +
> + d = (struct ext4_dir_entry_2 *)(((void *)d) +
> ext4_rec_len_from_disk(d->rec_len, blocksize));
>
> if (d != top)
> @@ -410,10 +410,10 @@ int ext4_handle_dirty_dirblock(handle_t *handle,
> }
>
> static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
> - struct ext4_dir_entry *dirent,
> + struct ext4_dir_entry_2 *dirent,
> int *offset)
> {
> - struct ext4_dir_entry *dp;
> + struct ext4_dir_entry_2 *de;
> struct dx_root_info *root;
> int count_offset;
> int blocksize = EXT4_BLOCK_SIZE(inode->i_sb);
> @@ -422,10 +422,10 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
> if (rlen == blocksize)
> count_offset = 8;
> else if (rlen == 12) {
> - dp = (struct ext4_dir_entry *)(((void *)dirent) + 12);
> - if (ext4_rec_len_from_disk(dp->rec_len, blocksize) != blocksize - 12)
> + de = (struct ext4_dir_entry_2 *)(((void *)dirent) + 12);
> + if (ext4_rec_len_from_disk(de->rec_len, blocksize) != blocksize - 12)
> return NULL;
> - root = (struct dx_root_info *)(((void *)dp + 12));
> + root = (struct dx_root_info *)(((void *)de + 12));
> if (root->reserved_zero ||
> root->info_length != sizeof(struct dx_root_info))
> return NULL;
> @@ -438,7 +438,7 @@ static struct dx_countlimit *get_dx_countlimit(struct inode *inode,
> return (struct dx_countlimit *)(((void *)dirent) + count_offset);
> }
>
> -static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry *dirent,
> +static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry_2 *dirent,
> int count_offset, int count, struct dx_tail *t)
> {
> struct ext4_inode_info *ei = EXT4_I(inode);
> @@ -456,7 +456,7 @@ static __le32 ext4_dx_csum(struct inode *inode, struct ext4_dir_entry *dirent,
> }
>
> static int ext4_dx_csum_verify(struct inode *inode,
> - struct ext4_dir_entry *dirent)
> + struct ext4_dir_entry_2 *dirent)
> {
> struct dx_countlimit *c;
> struct dx_tail *t;
> @@ -485,7 +485,7 @@ static int ext4_dx_csum_verify(struct inode *inode,
> return 1;
> }
>
> -static void ext4_dx_csum_set(struct inode *inode, struct ext4_dir_entry *dirent)
> +static void ext4_dx_csum_set(struct inode *inode, struct ext4_dir_entry_2 *dirent)
> {
> struct dx_countlimit *c;
> struct dx_tail *t;
> @@ -515,7 +515,7 @@ static inline int ext4_handle_dirty_dx_node(handle_t *handle,
> struct inode *inode,
> struct buffer_head *bh)
> {
> - ext4_dx_csum_set(inode, (struct ext4_dir_entry *)bh->b_data);
> + ext4_dx_csum_set(inode, (struct ext4_dir_entry_2 *)bh->b_data);
> return ext4_handle_dirty_metadata(handle, inode, bh);
> }
>
> @@ -1488,7 +1488,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
> }
>
> static int is_dx_internal_node(struct inode *dir, ext4_lblk_t block,
> - struct ext4_dir_entry *de)
> + struct ext4_dir_entry_2 *de)
> {
> struct super_block *sb = dir->i_sb;
>
> @@ -1619,7 +1619,7 @@ static struct buffer_head *__ext4_find_entry(struct inode *dir,
> }
> if (!buffer_verified(bh) &&
> !is_dx_internal_node(dir, block,
> - (struct ext4_dir_entry *)bh->b_data) &&
> + (struct ext4_dir_entry_2 *)bh->b_data) &&
> !ext4_dirblock_csum_verify(dir, bh)) {
> EXT4_ERROR_INODE_ERR(dir, EFSBADCRC,
> "checksumming directory "
> --
> 2.43.7
>
Cheers, Andreas
^ permalink raw reply
* Re: [PATCH v5 05/10] fstests: verify f_fsid for cloned filesystems
From: Anand Jain @ 2026-05-27 22:41 UTC (permalink / raw)
To: Christoph Hellwig, Anand Jain
Cc: fstests, linux-btrfs, linux-ext4, linux-xfs, linux-f2fs-devel,
amir73il, zlang
In-Reply-To: <ahP2hjv9zl_WL1kg@infradead.org>
On 25/5/26 15:13, Christoph Hellwig wrote:
> On Thu, May 21, 2026 at 08:54:55PM +0800, Anand Jain wrote:
>> +[ "$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"
>
> Seems like these are stuck on the btrfs list. Any progress on that?
They are in the `for-next` branch of the Btrfs Linux repository:
https://github.com/btrfs/linux.git
^ permalink raw reply
* [PATCH v6 0/11] fstests: add test coverage for cloned filesystem ids
From: Anand Jain @ 2026-05-28 4:05 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, linux-f2fs-devel, zlang, hch
v6:
. Renamed `pre_clone_tune_uuid()` to `_change_metadata_uuid()`.
. Created the `_require_unique_f_fsid()` helper instead of handling it inside the test case (patch 5/11).
. Separated `FSNOTIFYWAIT_PROG` into its own patch.
. Dropped the `inotify` test case in favor of `fsnotify`.
. Added comments throughout, especially for helper functions.
v5:
https://lore.kernel.org/fstests/cover.1779367627.git.asj@kernel.org
v4:
https://lore.kernel.org/fstests/cover.1777357320.git.asj@kernel.org
v3:
https://lore.kernel.org/fstests/cover.1777281778.git.asj@kernel.org
v2:
https://lore.kernel.org/fstests/cover.1774090817.git.asj@kernel.org
v1:
https://lore.kernel.org/fstests/cover.1772095513.git.asj@kernel.org
This series adds fstests infrastructure and test cases to verify correct
filesystem identity when a filesystem is cloned (block-level copy).
Test covers inotify, fanotify, f_fsid, libblkid, IMA, exportfs file handles
and libblkid tools verify with metadata_uuid.
New helpers:
_loop_image_create_clone() and _loop_image_destroy() to help create fs and clone
_clone_mount_option() helper to apply per-filesystem clone mount options
_change_metadata_uuid() changes the UUID before the clone
New tests:
- fanotify events are isolated between cloned filesystems
- f_fsid is unique across cloned filesystem instances
- libblkid correctly resolves duplicate UUIDs to distinct devices
with and without metadata_uuid
- IMA distinct identity for each cloned filesystem
- exportfs file handles resolve correctly on cloned filesystems
Kernel Patches:
Requires Btrfs kernel patches for all tests to pass.
[1] https://lore.kernel.org/linux-btrfs/cover.1777281686.git.asj@kernel.org
Anand Jain (11):
fstests: add _loop_image_create_clone() helper
fstests: add _clone_mount_option() helper
fstests: add FSNOTIFYWAIT_PROG
fstests: add _require_unique_f_fsid() helper
fstests: verify fanotify isolation on cloned filesystems
fstests: verify f_fsid for cloned filesystems
fstests: verify libblkid resolution of duplicate UUIDs
fstests: verify IMA isolation on cloned filesystems
fstests: verify exportfs file handles on cloned filesystems
fstests: add _change_metadata_uuid helper
fstests: test UUID consistency for clones with metadata_uuid
common/config | 1 +
common/rc | 124 ++++++++++++++++++++++++++++++++++++++
tests/generic/801 | 135 ++++++++++++++++++++++++++++++++++++++++++
tests/generic/801.out | 7 +++
tests/generic/802 | 67 +++++++++++++++++++++
tests/generic/802.out | 7 +++
tests/generic/803 | 84 ++++++++++++++++++++++++++
tests/generic/803.out | 19 ++++++
tests/generic/804 | 108 +++++++++++++++++++++++++++++++++
tests/generic/804.out | 10 ++++
tests/generic/805 | 80 +++++++++++++++++++++++++
tests/generic/805.out | 2 +
tests/generic/806 | 84 ++++++++++++++++++++++++++
tests/generic/806.out | 19 ++++++
14 files changed, 747 insertions(+)
create mode 100644 tests/generic/801
create mode 100644 tests/generic/801.out
create mode 100644 tests/generic/802
create mode 100644 tests/generic/802.out
create mode 100644 tests/generic/803
create mode 100644 tests/generic/803.out
create mode 100644 tests/generic/804
create mode 100644 tests/generic/804.out
create mode 100644 tests/generic/805
create mode 100644 tests/generic/805.out
create mode 100644 tests/generic/806
create mode 100644 tests/generic/806.out
--
2.43.0
^ permalink raw reply
* [PATCH v6 01/11] fstests: add _loop_image_create_clone() helper
From: Anand Jain @ 2026-05-28 4:05 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, linux-f2fs-devel, zlang, hch
In-Reply-To: <cover.1779939330.git.asj@kernel.org>
Introduce _loop_image_create_clone() and _loop_image_destroy() to mkfs an
image file and clone it to another image file, and attach a loop device to
them. And its destroy part.
Signed-off-by: Anand Jain <asj@kernel.org>
---
common/rc | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/common/rc b/common/rc
index 79189e7e6e94..d7e3e0bdfb1e 100644
--- a/common/rc
+++ b/common/rc
@@ -1520,6 +1520,69 @@ _scratch_resvblks()
esac
}
+# Create a small loop image, run an optional tuning function ($2) on it,
+# clone it, and attach both to loop devices, returned in ($1).
+# Args:
+# $1: Nameref to return the array of allocated loop devices [base, clone].
+# $2: Optional callback function to tune the base filesystem before cloning.
+_loop_image_create_clone()
+{
+ local -n _ret=$1
+ local pre_clone_tune_func="$2"
+ local img_file=$TEST_DIR/${seq}.img
+ local img_file_clone=$TEST_DIR/${seq}_clone.img
+ local size=$(_small_fs_size_mb 128) # Smallest possible
+ local loop_devs
+
+ # Since we copy the block device image, we keep its size small.
+ _require_fs_space $TEST_DIR $((size * 1024))
+
+ _create_file_sized $((size * 1024 * 1024)) $img_file ||
+ _fail "Failed: Create $img_file $size"
+
+ loop_devs=$(_create_loop_device $img_file)
+ _ret=($loop_devs)
+
+ case $FSTYP in
+ xfs)
+ _mkfs_dev "-s size=4096" ${loop_devs[0]}
+ ;;
+ btrfs)
+ _mkfs_dev ${loop_devs[0]}
+ ;;
+ *)
+ _mkfs_dev ${loop_devs[0]}
+ ;;
+ esac
+
+ # Only execute if the function argument is not empty
+ if [ -n "$pre_clone_tune_func" ]; then
+ $pre_clone_tune_func ${loop_devs[0]}
+ fi
+
+ sync ${loop_devs[0]}
+ cp $img_file $img_file_clone
+
+ loop_devs="$loop_devs $(_create_loop_device $img_file_clone)"
+
+ _ret=($loop_devs)
+}
+
+# Teardown loop devices and delete their underlying backing image files.
+# Accepts a list of loop device paths (e.g., /dev/loop0 /dev/loop1).
+_loop_image_destroy()
+{
+ for d in "$@"; do
+ # Retrieve the path of the backing file
+ local f=$(losetup --noheadings --output BACK-FILE $d)
+
+ # Detach the loop device from the backing file
+ _destroy_loop_device "$d"
+
+ # Clean up the backing disk image file
+ [ -n "$f" ] && rm -f "$f"
+ done
+}
# Repair scratch filesystem. Returns 0 if the FS is good to go (either no
# errors found or errors were fixed) and nonzero otherwise; also spits out
--
2.43.0
^ permalink raw reply related
* [PATCH v6 02/11] fstests: add _clone_mount_option() helper
From: Anand Jain @ 2026-05-28 4:05 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, linux-ext4, linux-xfs, linux-f2fs-devel, zlang, hch
In-Reply-To: <cover.1779939330.git.asj@kernel.org>
Adds _clone_mount_option() helper function to handle filesystem-specific
requirements for mounting cloned devices. Abstract the need for -o nouuid
on XFS.
Signed-off-by: Anand Jain <asj@kernel.org>
---
common/rc | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/common/rc b/common/rc
index d7e3e0bdfb1e..937f478963b4 100644
--- a/common/rc
+++ b/common/rc
@@ -414,6 +414,23 @@ _scratch_mount_options()
$SCRATCH_DEV $SCRATCH_MNT
}
+# Return filesystem-specific mount options required for mounting clone/snapshot
+# devices.
+_clone_mount_option()
+{
+ local mount_opts=""
+
+ case "$FSTYP" in
+ xfs)
+ # Allow mounting a duplicate filesystem on the same host
+ mount_opts="-o nouuid"
+ ;;
+ *)
+ esac
+
+ echo $mount_opts
+}
+
_supports_filetype()
{
local dir=$1
--
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