* Re: [PATCH ext4 v3] ext4: fix out-of-bounds read in ext4_read_inline_dir()
From: Xiang Mei @ 2026-06-15 19:06 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, Theodore Ts'o, Andreas Dilger, Baokun Li,
Ojaswin Mujoo, Ritesh Harjani, Zhang Yi, Weiming Shi
In-Reply-To: <cohsy6t4qa7pxub7knepgd6nckybqx2xt7ps33zh7fwkx6p3ac@dswnh7vxc572>
On Mon, Jun 15, 2026 at 2:27 AM Jan Kara <jack@suse.cz> wrote:
>
> On Sat 13-06-26 15:18:36, Xiang Mei wrote:
> > ext4_read_inline_dir() can read a dirent header past the end of its inline
> > buffer, triggering a slab-out-of-bounds read during getdents64():
> >
> > BUG: KASAN: slab-out-of-bounds in __ext4_check_dir_entry
> > Read of size 2 at addr ffff88800f3dd23c by task exploit/148
> > ...
> > __ext4_check_dir_entry
> > ext4_read_inline_dir
> > iterate_dir
> >
> > The dirent payload lives in a buffer of exactly inline_size bytes:
> >
> > dir_buf = kmalloc(inline_size, GFP_NOFS);
> >
> > but iteration runs in a position space extra_offset bytes larger
> > (extra_size = extra_offset + inline_size) so the synthetic "." and ".."
> > land at their block-dir offsets. A dirent is formed at "dir_buf + pos -
> > extra_offset", yet the loop bound (ctx->pos < extra_size) and the
> > ext4_check_dir_entry() length argument both use the larger extra_size. A
> > ctx->pos that is valid in extra_size space but whose de lies past
> > inline_size is therefore accepted, and the rescan loop's rec_len probe
> > and ext4_check_dir_entry() dereference de->rec_len before the entry is
> > rejected.
> >
> > Bound the dirent header by inline_size in both loops: break out of the
> > rescan loop once a minimum-size header no longer fits, reject such a
> > position in the main loop before forming de, and pass inline_size rather
> > than extra_size to ext4_check_dir_entry().
> >
> > Fixes: c4d8b0235aa9 ("ext4: fix readdir error in case inline_data+^dir_index.")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
>
> Looks mostly good, just one simplification suggestion below:
>
> > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> > index 8045e4ff270c..1266c8827cca 100644
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -1454,6 +1454,9 @@ int ext4_read_inline_dir(struct file *file,
> > /* for other entry, the real offset in
> > * the buf has to be tuned accordingly.
> > */
> > + if (i - extra_offset + ext4_dir_rec_len(1, NULL) >
> > + inline_size)
> > + break;
>
> Since 'i' lives in "dir logical offset space", it might be simpler to check
> this as:
> if (i + ext4_dir_rec_len(1, NULL) > extra_size)
>
>
> > de = (struct ext4_dir_entry_2 *)
> > (dir_buf + i - extra_offset);
> > /* It's too expensive to do a full
> > @@ -1488,10 +1491,18 @@ int ext4_read_inline_dir(struct file *file,
> > continue;
> > }
> >
> > + /*
> > + * de lives at dir_buf + ctx->pos - extra_offset, within the
> > + * kmalloc(inline_size) buffer. Make sure its header fits before
> > + * ext4_check_dir_entry() dereferences de->rec_len.
> > + */
> > + if (ctx->pos - extra_offset + ext4_dir_rec_len(1, NULL) >
> > + inline_size)
>
> Similarly here this could be checked as:
>
> if (ctx->pos + ext4_dir_rec_len(1, NULL) > extra_size)
>
> which is both more consistent with the loop termination condition and saves
> the conversion from logical offset to physical offset.
>
> Otherwise the patch looks good.
Thanks so much for the tips and your review. V4 has been sent.
Xiang
>
> Honza
>
> > + goto out;
> > de = (struct ext4_dir_entry_2 *)
> > (dir_buf + ctx->pos - extra_offset);
> > if (ext4_check_dir_entry(inode, file, de, iloc.bh, dir_buf,
> > - extra_size, ctx->pos))
> > + inline_size, ctx->pos))
> > goto out;
> > if (le32_to_cpu(de->inode)) {
> > if (!dir_emit(ctx, de->name, de->name_len,
> > --
> > 2.43.0
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v3 0/3] f2fs: support encrypted inline data
From: Eric Biggers @ 2026-06-15 19:37 UTC (permalink / raw)
To: LiaoYuanhong-vivo
Cc: Jaegeuk Kim, Chao Yu, Jonathan Corbet, Shuah Khan,
Theodore Y. Ts'o, open list:F2FS FILE SYSTEM, open list,
open list:DOCUMENTATION,
open list:FSCRYPT: FILE SYSTEM LEVEL ENCRYPTION SUPPORT,
linux-ext4
In-Reply-To: <20260615125517.362294-1-liaoyuanhong@vivo.com>
[+Cc linux-ext4@vger.kernel.org]
On Mon, Jun 15, 2026 at 08:55:12PM +0800, LiaoYuanhong-vivo wrote:
> F2FS currently disables inline data for encrypted regular files because the
> inline payload is stored in the inode block and does not go through the
> regular bio-based fscrypt path. This wastes space for small encrypted
> files on Android devices using F2FS inlinecrypt.
>
> This series adds an encrypted_inline_data on-disk feature for F2FS.
> With this feature enabled, encrypted regular files may keep small contents
> in the inode block. The inline payload is encrypted before being stored in
> the inode and decrypted back into page-cache plaintext on read.
>
> The fscrypt changes are scoped to filesystem-managed data-unit crypto.
> F2FS first asks fscrypt whether the inode's key/policy supports this path.
> It prepares the software transform only when encrypted inline payloads are
> read or written. Inlinecrypt support is limited to v2 IV_INO_LBLK_64 and
> IV_INO_LBLK_32 policies, including the hardware-wrapped key configurations
> supported by fscrypt. Per-file inlinecrypt keys and DIRECT_KEY policies
> are not supported for encrypted inline data.
I still think we should hold off on this, for the reasons I gave at
https://lore.kernel.org/r/20260515184124.GA4903@quark/
As soon as you start using hardware-wrapped keys this will become
irrelevant, as it can't be used in that case. I see you added "support"
for that case anyway by deriving contents encryption keys from the
sw_secret. But that bypasses the security model, which isn't okay.
I'm also working to simplify ext4 and f2fs's file contents encryption
implementation by standardizing on blk-crypto. That aligns well with
what btrfs encryption is going to do as well. So this isn't a great
time to be making f2fs's file contents encryption implementation even
more complex by going in a different direction.
If there was demand for this feature from the ext4 side for
general-purpose Linux distros as well, that would make it a bit more
appealing, as it would show broader demand. But with the proposal being
f2fs-specific and effectively just for Android devices that *don't* use
wrapped keys, that feels too narrow for the added complexity.
This proposal also lacks test cases in xfstests and/or Android's
vts_kernel_encryption_test that verify that the inline data is actually
being encrypted correctly. Those tests are essential, and we *must not*
add new file contents encryption implementations without such tests.
- Eric
^ permalink raw reply
* Re: [PATCH ext4 v4] ext4: fix out-of-bounds read in ext4_read_inline_dir()
From: Jan Kara @ 2026-06-15 20:35 UTC (permalink / raw)
To: Xiang Mei
Cc: linux-ext4, Jan Kara, Theodore Ts'o, Andreas Dilger,
Baokun Li, Ojaswin Mujoo, Ritesh Harjani, Zhang Yi, Weiming Shi
In-Reply-To: <20260615190519.946736-1-xmei5@asu.edu>
On Mon 15-06-26 12:05:19, Xiang Mei wrote:
> ext4_read_inline_dir() can read a dirent header past the end of its inline
> buffer, triggering a slab-out-of-bounds read during getdents64():
>
> BUG: KASAN: slab-out-of-bounds in __ext4_check_dir_entry
> Read of size 2 at addr ffff88800f3dd23c by task exploit/148
> ...
> __ext4_check_dir_entry
> ext4_read_inline_dir
> iterate_dir
>
> The dirent payload lives in a buffer of exactly inline_size bytes:
>
> dir_buf = kmalloc(inline_size, GFP_NOFS);
>
> but iteration runs in a position space extra_offset bytes larger
> (extra_size = extra_offset + inline_size) so the synthetic "." and ".."
> land at their block-dir offsets. A dirent is formed at "dir_buf + pos -
> extra_offset", yet the ext4_check_dir_entry() length argument uses the
> larger extra_size. A position whose dirent header would extend past
> extra_size is therefore accepted, and the rescan loop's rec_len probe and
> ext4_check_dir_entry() dereference de->rec_len before the entry is rejected.
>
> Reject a position whose minimum-size dirent header would not fit within
> extra_size before forming de, in both the rescan and main loops, and pass
> inline_size rather than extra_size to ext4_check_dir_entry() so the length
> check matches the physical buffer.
>
> Fixes: c4d8b0235aa9 ("ext4: fix readdir error in case inline_data+^dir_index.")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> v4: use Jan's suggested logical-space bound (i/ctx->pos + rec_len >
> extra_size), consistent with the loop condition.
> v3: dropped the unreachable ctx->pos < extra_offset underflow check.
> v2: validate against inline_size instead of extra_size.
>
> fs/ext4/inline.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 8045e4ff270c..f1f7104d3dac 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1454,6 +1454,8 @@ int ext4_read_inline_dir(struct file *file,
> /* for other entry, the real offset in
> * the buf has to be tuned accordingly.
> */
> + if (i + ext4_dir_rec_len(1, NULL) > extra_size)
> + break;
> de = (struct ext4_dir_entry_2 *)
> (dir_buf + i - extra_offset);
> /* It's too expensive to do a full
> @@ -1488,10 +1490,17 @@ int ext4_read_inline_dir(struct file *file,
> continue;
> }
>
> + /*
> + * de lives at dir_buf + ctx->pos - extra_offset, within the
> + * kmalloc(inline_size) buffer. Make sure its header fits before
> + * ext4_check_dir_entry() dereferences de->rec_len.
> + */
> + if (ctx->pos + ext4_dir_rec_len(1, NULL) > extra_size)
> + goto out;
> de = (struct ext4_dir_entry_2 *)
> (dir_buf + ctx->pos - extra_offset);
> if (ext4_check_dir_entry(inode, file, de, iloc.bh, dir_buf,
> - extra_size, ctx->pos))
> + inline_size, ctx->pos))
> goto out;
> if (le32_to_cpu(de->inode)) {
> if (!dir_emit(ctx, de->name, de->name_len,
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] ext4: Remove ext4_end_buffer_io_sync()
From: Jan Kara @ 2026-06-15 20:41 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Theodore Ts'o, Harshad Shirwadkar, Andreas Dilger, Baokun Li,
Jan Kara, Ojaswin Mujoo, Ritesh Harjani, Zhang Yi, linux-ext4
In-Reply-To: <20260615182527.2208479-1-willy@infradead.org>
On Mon 15-06-26 19:25:25, Matthew Wilcox (Oracle) wrote:
> There's no need for a custom end_io routine here. We lose some
> tracing of I/O completions, but we gain better error handling.
> Well, consistent error handling anyway.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Yes, I think this is better. I was also considering that when revieweing
your patch series but decided to leave that as a future cleanup. The future
is now! :) Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/fast_commit.c | 21 +--------------------
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 5773b85e43cb..6b00a7285344 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -184,25 +184,6 @@
> #include <trace/events/ext4.h>
> static struct kmem_cache *ext4_fc_dentry_cachep;
>
> -static void ext4_end_buffer_io_sync(struct bio *bio)
> -{
> - struct buffer_head *bh;
> - bool uptodate = bio_endio_bh(bio, &bh);
> -
> - BUFFER_TRACE(bh, "");
> - if (uptodate) {
> - ext4_debug("%s: Block %lld up-to-date",
> - __func__, bh->b_blocknr);
> - set_buffer_uptodate(bh);
> - } else {
> - ext4_debug("%s: Block %lld not up-to-date",
> - __func__, bh->b_blocknr);
> - clear_buffer_uptodate(bh);
> - }
> -
> - unlock_buffer(bh);
> -}
> -
> static inline void ext4_fc_reset_inode(struct inode *inode)
> {
> struct ext4_inode_info *ei = EXT4_I(inode);
> @@ -662,7 +643,7 @@ static void ext4_fc_submit_bh(struct super_block *sb, bool is_tail)
> lock_buffer(bh);
> set_buffer_dirty(bh);
> set_buffer_uptodate(bh);
> - bh_submit(bh, REQ_OP_WRITE | write_flags, ext4_end_buffer_io_sync);
> + bh_submit(bh, REQ_OP_WRITE | write_flags, bh_end_write);
> EXT4_SB(sb)->s_fc_bh = NULL;
> }
>
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [GIT PULL] udf, isofs, ext2, quota fixes and cleanups for 7.2-rc1
From: pr-tracker-bot @ 2026-06-16 7:08 UTC (permalink / raw)
To: Jan Kara; +Cc: Linus Torvalds, linux-fsdevel, linux-ext4
In-Reply-To: <jpb2wjkfh7j73dpinuvwe5qddkr54px64vmvx6bclzd5ter6hk@mjwwwgohsyxy>
The pull request you sent on Mon, 15 Jun 2026 16:51:01 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fs_for_v7.2-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/974b3dec2bfb4b2726a6194105cb048a9dab0626
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v3] ext4: fix circular lock dependency in ext4_ext_migrate
From: Zhou, Yun @ 2026-06-16 7:51 UTC (permalink / raw)
To: jack
Cc: linux-ext4, linux-kernel, tytso, adilger.kernel, libaokun,
ojaswin, ritesh.list, yi.zhang, ebiggers
In-Reply-To: <20260612005330.1930804-1-yun.zhou@windriver.com>
> Move iput(tmp_inode) after ext4_writepages_up_write() to avoid a
> circular lock dependency between s_writepages_rwsem and sb_internal
> (freeze protection).
>
> The deadlock scenario:
>
> CPU0 (EXT4_IOC_MIGRATE) CPU1 (orphan cleanup during mount)
> ---- ----
> ext4_ext_migrate()
> ext4_writepages_down_write()
> s_writepages_rwsem (write)
> ext4_evict_inode()
> sb_start_intwrite() [sb_internal]
> ...
> ext4_writepages()
> s_writepages_rwsem (read) [BLOCKED]
> iput(tmp_inode)
> ext4_evict_inode()
> sb_start_intwrite() [BLOCKED]
>
> The tmp_inode is a temporary inode with nlink=0 created solely for
> building the extent tree. Its eviction does not require
> s_writepages_rwsem protection, so deferring iput() until after
> releasing the rwsem is safe.
>
> Reported-by: syzbot+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
> Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> v3: fixes Reported-by tag and Closes tag.
>
> v2: remove redundant null pointer check for iput(tmp_inode).
>
> fs/ext4/migrate.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Hi Honza,
Thank you very much for taking the time to review these patches and
providing your valuable suggestions. I am eager to solve these
long-standing deadlock issues on Syzkaller, but I do not have much
community experience. I'd like to know, regarding this patch, should I
launch a new RR thread or continue waiting? BR, Yun
^ permalink raw reply
* Re: [PATCH v3] ext4: fix circular lock dependency in ext4_ext_migrate
From: Jan Kara @ 2026-06-16 9:07 UTC (permalink / raw)
To: Zhou, Yun
Cc: jack, linux-ext4, linux-kernel, tytso, adilger.kernel, libaokun,
ojaswin, ritesh.list, yi.zhang, ebiggers
In-Reply-To: <00673f65-cfd4-4042-93cf-cb04ad1d92fb@windriver.com>
Hello Yun!
On Tue 16-06-26 15:51:13, Zhou, Yun wrote:
> > Move iput(tmp_inode) after ext4_writepages_up_write() to avoid a
> > circular lock dependency between s_writepages_rwsem and sb_internal
> > (freeze protection).
> >
> > The deadlock scenario:
> >
> > CPU0 (EXT4_IOC_MIGRATE) CPU1 (orphan cleanup during mount)
> > ---- ----
> > ext4_ext_migrate()
> > ext4_writepages_down_write()
> > s_writepages_rwsem (write)
> > ext4_evict_inode()
> > sb_start_intwrite() [sb_internal]
> > ...
> > ext4_writepages()
> > s_writepages_rwsem (read) [BLOCKED]
> > iput(tmp_inode)
> > ext4_evict_inode()
> > sb_start_intwrite() [BLOCKED]
> >
> > The tmp_inode is a temporary inode with nlink=0 created solely for
> > building the extent tree. Its eviction does not require
> > s_writepages_rwsem protection, so deferring iput() until after
> > releasing the rwsem is safe.
> >
> > Reported-by: syzbot+212e8f62790f8e0bc63b@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=212e8f62790f8e0bc63b
> > Fixes: cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL")
> > Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > ---
> > v3: fixes Reported-by tag and Closes tag.
> >
> > v2: remove redundant null pointer check for iput(tmp_inode).
> >
> > fs/ext4/migrate.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Thank you very much for taking the time to review these patches and
> providing your valuable suggestions. I am eager to solve these long-standing
> deadlock issues on Syzkaller, but I do not have much community experience.
> I'd like to know, regarding this patch, should I launch a new RR thread or
> continue waiting? BR, Yun
Please keep waiting. On Sunday the merge window for 7.2-rc1 has opened.
That means that about week before and at least for the next week,
maintainers often aren't taking any patches in their trees. I expect Ted to
pick your patch later when he collects fixes to send for 7.2-rc2 or so - he
sends email about that as a reply to the patch. If nothing happens for next
two weeks, I suggest you send a ping asking whether the patch didn't get
lost as a reply to your patch submission. Thanks for your fixes!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 02/23] ext4: factor out ext4_truncate_[up|down]()
From: Jan Kara @ 2026-06-16 9:31 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-3-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:22, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Refactor ext4_setattr() by introducing two helper functions,
> ext4_truncate_up() and ext4_truncate_down(), to handle size changes. The
> current ATTR_SIZE processing consolidates checks for both shrinking and
> non-shrinking cases, leading to cluttered code. Separating the
> truncation paths improves readability.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Nice! Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 199 +++++++++++++++++++++++++++---------------------
> 1 file changed, 112 insertions(+), 87 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 0751dc55e94f..35e958f89bd5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5855,6 +5855,112 @@ static void ext4_wait_for_tail_page_commit(struct inode *inode)
> }
> }
>
> +/*
> + * Set i_size and i_disksize to 'newsize'.
> + *
> + * Both i_rwsem and i_data_sem are required here to avoid races between
> + * generic append writeback and concurrent truncate that also modify
> + * i_size and i_disksize.
> + */
> +static inline void ext4_set_inode_size(struct inode *inode, loff_t newsize)
> +{
> + WARN_ON_ONCE(S_ISREG(inode->i_mode) && !inode_is_locked(inode));
> +
> + down_write(&EXT4_I(inode)->i_data_sem);
> + i_size_write(inode, newsize);
> + EXT4_I(inode)->i_disksize = newsize;
> + up_write(&EXT4_I(inode)->i_data_sem);
> +}
> +
> +static int ext4_truncate_up(struct inode *inode, loff_t oldsize, loff_t newsize)
> +{
> + ext4_lblk_t old_lblk, new_lblk;
> + handle_t *handle;
> + int ret;
> +
> + if (!IS_ALIGNED(oldsize | newsize, i_blocksize(inode))) {
> + ret = ext4_inode_attach_jinode(inode);
> + if (ret)
> + return ret;
> + }
> +
> + inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
> + if (!IS_ALIGNED(oldsize, i_blocksize(inode))) {
> + ret = ext4_block_zero_eof(inode, oldsize, LLONG_MAX);
> + if (ret)
> + return ret;
> + }
> +
> + handle = ext4_journal_start(inode, EXT4_HT_INODE, 3);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> + old_lblk = oldsize > 0 ? (oldsize - 1) >> inode->i_blkbits : 0;
> + new_lblk = newsize > 0 ? (newsize - 1) >> inode->i_blkbits : 0;
> + ext4_fc_track_range(handle, inode, old_lblk, new_lblk);
> +
> + ext4_set_inode_size(inode, newsize);
> +
> + ret = ext4_mark_inode_dirty(handle, inode);
> + ext4_journal_stop(handle);
> + if (ret)
> + return ret;
> + /*
> + * isize extend must be called outside an active handle due to
> + * the lock ordering of transaction start and folio lock in the
> + * iomap buffered I/O path (folio lock -> transaction start).
> + */
> + pagecache_isize_extended(inode, oldsize, newsize);
> + return 0;
> +}
> +
> +static int ext4_truncate_down(struct inode *inode, loff_t oldsize,
> + loff_t newsize, int *orphan)
> +{
> + ext4_lblk_t start_lblk;
> + handle_t *handle;
> + int ret;
> +
> + /* Do not change i_size. */
> + if (newsize == oldsize)
> + goto truncate;
> +
> + /* Shrink. */
> + handle = ext4_journal_start(inode, EXT4_HT_INODE, 3);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> + if (ext4_handle_valid(handle)) {
> + ret = ext4_orphan_add(handle, inode);
> + *orphan = 1;
> + if (ret) {
> + ext4_journal_stop(handle);
> + return ret;
> + }
> + }
> +
> + start_lblk = newsize > 0 ? (newsize - 1) >> inode->i_blkbits : 0;
> + ext4_fc_track_range(handle, inode, start_lblk, EXT_MAX_BLOCKS - 1);
> +
> + ext4_set_inode_size(inode, newsize);
> +
> + ret = ext4_mark_inode_dirty(handle, inode);
> + ext4_journal_stop(handle);
> + if (ret)
> + return ret;
> +
> + if (ext4_should_journal_data(inode))
> + ext4_wait_for_tail_page_commit(inode);
> +truncate:
> + /*
> + * Truncate pagecache after we've waited for commit in data=journal
> + * mode to make pages freeable. Call ext4_truncate() even if
> + * i_size didn't change to truncatea possible preallocated blocks.
> + */
> + truncate_pagecache(inode, newsize);
> + return ext4_truncate(inode);
> +}
> +
> /*
> * ext4_setattr()
> *
> @@ -5951,7 +6057,6 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> }
>
> if (attr->ia_valid & ATTR_SIZE) {
> - handle_t *handle;
> loff_t oldsize = inode->i_size;
> int shrink = (attr->ia_size < inode->i_size);
>
> @@ -6003,94 +6108,14 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> goto err_out;
> }
>
> - if (attr->ia_size != inode->i_size) {
> - /* attach jbd2 jinode for EOF folio tail zeroing */
> - if (attr->ia_size & (inode->i_sb->s_blocksize - 1) ||
> - oldsize & (inode->i_sb->s_blocksize - 1)) {
> - error = ext4_inode_attach_jinode(inode);
> - if (error)
> - goto out_mmap_sem;
> - }
> -
> - /*
> - * Update c/mtime and tail zero the EOF folio on
> - * truncate up. ext4_truncate() handles the shrink case
> - * below.
> - */
> - if (!shrink) {
> - inode_set_mtime_to_ts(inode,
> - inode_set_ctime_current(inode));
> - if (oldsize & (inode->i_sb->s_blocksize - 1)) {
> - error = ext4_block_zero_eof(inode,
> - oldsize, LLONG_MAX);
> - if (error)
> - goto out_mmap_sem;
> - }
> - }
> -
> - handle = ext4_journal_start(inode, EXT4_HT_INODE, 3);
> - if (IS_ERR(handle)) {
> - error = PTR_ERR(handle);
> - goto out_mmap_sem;
> - }
> - if (ext4_handle_valid(handle) && shrink) {
> - error = ext4_orphan_add(handle, inode);
> - orphan = 1;
> - if (error)
> - goto out_handle;
> - }
> -
> - if (shrink)
> - ext4_fc_track_range(handle, inode,
> - (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
> - inode->i_sb->s_blocksize_bits,
> - EXT_MAX_BLOCKS - 1);
> - else
> - ext4_fc_track_range(
> - handle, inode,
> - (oldsize > 0 ? oldsize - 1 : oldsize) >>
> - inode->i_sb->s_blocksize_bits,
> - (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
> - inode->i_sb->s_blocksize_bits);
> -
> - /*
> - * We have to update i_size under i_data_sem together
> - * with i_disksize to avoid races with writeback code
> - * updating disksize in mpage_map_and_submit_extent().
> - */
> - down_write(&EXT4_I(inode)->i_data_sem);
> - i_size_write(inode, attr->ia_size);
> - EXT4_I(inode)->i_disksize = attr->ia_size;
> - up_write(&EXT4_I(inode)->i_data_sem);
> -
> - error = ext4_mark_inode_dirty(handle, inode);
> -out_handle:
> - ext4_journal_stop(handle);
> - if (error)
> - goto out_mmap_sem;
> - if (!shrink) {
> - pagecache_isize_extended(inode, oldsize,
> - inode->i_size);
> - } else if (ext4_should_journal_data(inode)) {
> - ext4_wait_for_tail_page_commit(inode);
> - }
> + if (attr->ia_size > oldsize)
> + error = ext4_truncate_up(inode, oldsize, attr->ia_size);
> + else {
> + /* Shrink or do not change i_size. */
> + error = ext4_truncate_down(inode, oldsize,
> + attr->ia_size, &orphan);
> }
>
> - /*
> - * Truncate pagecache after we've waited for commit
> - * in data=journal mode to make pages freeable.
> - */
> - truncate_pagecache(inode, inode->i_size);
> - /*
> - * Call ext4_truncate() even if i_size didn't change to
> - * truncate possible preallocated blocks.
> - */
> - if (attr->ia_size <= oldsize) {
> - rc = ext4_truncate(inode);
> - if (rc)
> - error = rc;
> - }
> -out_mmap_sem:
> filemap_invalidate_unlock(inode->i_mapping);
> }
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 03/23] ext4: simplify error handling in ext4_setattr()
From: Jan Kara @ 2026-06-16 9:36 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-4-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:23, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Refactor the error handling in ext4_setattr() for better clarity:
>
> - Return directly on ext4_break_layouts() failure.
> - Propagate ext4_truncate() errors using the existing error variable
> and jump to the common 'err_out' label.
> - Propagate posix_acl_chmod() errors also through the error variable,
> as it theoretically does not return a non-fatal error.
>
> With these changes, every error path either returns immediately or jumps
> to err_out. Consequently, the "if (!error)" condition guarding
> setattr_copy() and mark_inode_dirty() becomes unreachable for error
> cases. Remove this redundant check and the unused rc variable can be
> removed as well.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 35e958f89bd5..b1ef706987c3 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5989,7 +5989,7 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> struct iattr *attr)
> {
> struct inode *inode = d_inode(dentry);
> - int error, rc = 0;
> + int error;
> int orphan = 0;
> const unsigned int ia_valid = attr->ia_valid;
> bool inc_ivers = true;
> @@ -6102,10 +6102,10 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>
> filemap_invalidate_lock(inode->i_mapping);
>
> - rc = ext4_break_layouts(inode);
> - if (rc) {
> + error = ext4_break_layouts(inode);
> + if (error) {
> filemap_invalidate_unlock(inode->i_mapping);
> - goto err_out;
> + return error;
> }
>
> if (attr->ia_size > oldsize)
> @@ -6117,15 +6117,19 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> }
>
> filemap_invalidate_unlock(inode->i_mapping);
> + if (error)
> + goto err_out;
> }
>
> - if (!error) {
> - if (inc_ivers)
> - inode_inc_iversion(inode);
> - setattr_copy(idmap, inode, attr);
> - mark_inode_dirty(inode);
> - }
> + if (inc_ivers)
> + inode_inc_iversion(inode);
> + setattr_copy(idmap, inode, attr);
> + mark_inode_dirty(inode);
>
> + if (ia_valid & ATTR_MODE)
> + error = posix_acl_chmod(idmap, dentry, inode->i_mode);
> +
> +err_out:
> /*
> * If the call to ext4_truncate failed to get a transaction handle at
> * all, we need to clean up the in-core orphan list manually.
> @@ -6133,14 +6137,8 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> if (orphan && inode->i_nlink)
> ext4_orphan_del(NULL, inode);
>
> - if (!error && (ia_valid & ATTR_MODE))
> - rc = posix_acl_chmod(idmap, dentry, inode->i_mode);
> -
> -err_out:
> - if (error)
> + if (error)
> ext4_std_error(inode->i_sb, error);
> - if (!error)
> - error = rc;
> return error;
> }
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v3 0/3] f2fs: support encrypted inline data
From: LiaoYuanhong-vivo @ 2026-06-16 9:46 UTC (permalink / raw)
To: ebiggers
Cc: chao, corbet, jaegeuk, liaoyuanhong, linux-doc, linux-ext4,
linux-f2fs-devel, linux-fscrypt, linux-kernel, skhan, tytso
In-Reply-To: <20260615193728.GA1764@quark>
Hi Eric,
Thanks for the explanation.
I understand the concern about deriving software contents keys from
sw_secret for hardware-wrapped-key files. I agree this is not the right
security model, and I will stop pursuing this direction for now.
Could you share more about the direction you have in mind for simplifying
f2fs/ext4 contents encryption around blk-crypto?
For f2fs inline_data, there is still a real space-saving benefit on phones,
since many encrypted files are smaller than 4K. Is there any acceptable
future direction to support this kind of inode-resident data with
blk-crypto or hardware-wrapped keys?
Thanks,
Liao Yuanhong
^ permalink raw reply
* Re: [PATCH v4 07/23] ext4: do not use data=ordered mode for inodes using buffered iomap path
From: Jan Kara @ 2026-06-16 10:01 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-8-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:27, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> The data=ordered mode introduces two fundamental conflicts with the
> iomap buffered write path, leading to potential deadlocks.
>
> 1) Lock ordering conflict
> In the iomap writeback path, each folio is processed sequentially:
> the folio lock is acquired first, followed by starting a transaction
> to create block mappings. In data=ordered mode, writeback triggered
> by the journal commit process may attempt to acquire a folio lock
> that is already held by iomap. Meanwhile, iomap, under that same
> folio lock, may start a new transaction and wait for the currently
> committing transaction to finish, resulting in a deadlock.
>
> 2) Partial folio submission not supported
> When block size is smaller than folio size, a folio may contain both
> mapped and unmapped blocks. In data=ordered mode, if the journal
> waits for such a folio to be written back while the regular writeback
> process has already started committing it (with the writeback flag
> set), mapping the remaining unmapped blocks can deadlock. This is
> because the writeback flag is cleared only after the entire folio is
> processed and committed.
>
> To support data=ordered mode, the iomap core would need two invasive
> changes:
> - Acquire the transaction handle before locking any folio for
> writeback.
> - Support partial folio submission.
>
> Both changes are complicated and risk performance regressions.
> Therefore, we must avoid using data=ordered mode when converting to the
> iomap path.
>
> Currently, data=ordered mode is used in three scenarios:
> - Append write
> - Post-EOF partial block truncate-up followed by append write
> - Online defragmentation
>
> We can address the first two without data=ordered mode:
> - For append write: always allocate unwritten blocks (i.e. always
> enable dioread_nolock), preserving the behavior of current
> extent-type inodes.
> - For post-EOF truncate-up + append write: postpone updating i_disksize
> until after the zeroed partial block has been written back.
>
> Online defragmentation does not yet support iomap; this can be resolved
> separately in the future.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/ext4_jbd2.h | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 63d17c5201b5..26999f173870 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -383,7 +383,12 @@ static inline int ext4_should_journal_data(struct inode *inode)
>
> static inline int ext4_should_order_data(struct inode *inode)
> {
> - return ext4_inode_journal_mode(inode) & EXT4_INODE_ORDERED_DATA_MODE;
> + /*
> + * inodes using the iomap buffered I/O path do not use the
> + * data=ordered mode.
> + */
> + return !ext4_inode_buffered_iomap(inode) &&
> + (ext4_inode_journal_mode(inode) & EXT4_INODE_ORDERED_DATA_MODE);
> }
>
> static inline int ext4_should_writeback_data(struct inode *inode)
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 06/23] ext4: pass out extent seq counter when mapping da blocks
From: Jan Kara @ 2026-06-16 10:04 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-7-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:26, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> The iomap buffered write path does not hold any locks between querying
> inode extent mapping information and performing buffered writes. It
> relies on the sequence counter saved in the inode to detect stale
> mappings.
Now that I'm looking at it again, I've got a bit confused here. Buffered
write path is holding i_rwsem between mapping blocks and using them so
there shouldn't be races. Perhaps you mean buffered *writeback* path? But
then ext4_da_map_blocks() should not ever get called in the writeback path
because it is allocating delayed blocks... So this change looks unnecessary
to me now. Am I missing something?
Honza
>
> Commit 07c440e8da8f ("ext4: pass out extent seq counter when mapping
> blocks") added the m_seq field to ext4_map_blocks to pass out extent
> sequence numbers, but it missed two callsites within
> ext4_da_map_blocks(). These callsites are on the delayed allocation
> path, which is also used in the iomap buffered write path. Pass out the
> sequence counter to ensure stale mappings can be detected.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> fs/ext4/inode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 6c4d9137b279..39577a6b65b9 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1929,7 +1929,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
> ext4_check_map_extents_env(inode);
>
> /* Lookup extent status tree firstly */
> - if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, NULL)) {
> + if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
> map->m_len = min_t(unsigned int, map->m_len,
> es.es_len - (map->m_lblk - es.es_lblk));
>
> @@ -1982,7 +1982,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
> * is held in write mode, before inserting a new da entry in
> * the extent status tree.
> */
> - if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, NULL)) {
> + if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
> map->m_len = min_t(unsigned int, map->m_len,
> es.es_len - (map->m_lblk - es.es_lblk));
>
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 08/23] ext4: implement buffered write path using iomap
From: Jan Kara @ 2026-06-16 10:45 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-9-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:28, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> Introduce two new iomap_ops instances for ext4 buffered writes:
>
> - ext4_iomap_buffered_da_write_ops: for delayed allocation mode, using
> ext4_da_map_blocks() to map delalloc extents.
> - ext4_iomap_buffered_write_ops: for non-delayed allocation mode, using
> ext4_iomap_get_blocks() to directly allocate blocks.
>
> Also add ext4_iomap_valid() for the iomap infrastructure to check extent
> validity.
>
> Key changes and considerations:
>
> - Unwritten extents for new blocks (dioread_nolock always on)
> Since data=ordered mode is not used to prevent stale data exposure in
> the non-delayed allocation path, new blocks are always allocated as
> unwritten extents.
>
> - Short write and write failure handling
> a. Delalloc path: On short write or failure, the stale delalloc range
> must be dropped and its space reservation released. Otherwise, a
> clean folio may cover leftover delalloc extents, causing
> inaccurate space reservation accounting.
> b. Non-delalloc path: No cleanup of allocated blocks is needed on
> short write.
>
> - Lock ordering reversal
> The folio lock and transaction start ordering is reversed compared to
> the buffer_head buffered write path. To handle this, the journal
> handle must be stopped in iomap_begin() callbacks. The lock ordering
> documentation in super.c has been updated accordingly.
>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Looks good to me - besides the IOMAP_F_NEW bugs Ojaswin found. One
observation I have here is that since the old indirect block based on-disk
format doesn't support unwritten extents we can never transition it to the
iomap scheme used here. So we'll have to figure out some way to avoid
maintaining two (actually three if we count data=journal) buffered write /
writeback paths in the long term. But let's address that once things settle
for the common paths.
Honza
> ---
> fs/ext4/ext4.h | 4 ++
> fs/ext4/file.c | 20 +++++-
> fs/ext4/inode.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++-
> fs/ext4/super.c | 10 ++-
> 4 files changed, 192 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1e27d73d7427..4832e7f7db82 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3057,6 +3057,7 @@ int ext4_walk_page_buffers(handle_t *handle,
> int do_journal_get_write_access(handle_t *handle, struct inode *inode,
> struct buffer_head *bh);
> void ext4_set_inode_mapping_order(struct inode *inode);
> +int ext4_nonda_switch(struct super_block *sb);
> #define FALL_BACK_TO_NONDELALLOC 1
> #define CONVERT_INLINE_DATA 2
>
> @@ -3926,6 +3927,9 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
>
> extern const struct iomap_ops ext4_iomap_ops;
> extern const struct iomap_ops ext4_iomap_report_ops;
> +extern const struct iomap_ops ext4_iomap_buffered_write_ops;
> +extern const struct iomap_ops ext4_iomap_buffered_da_write_ops;
> +extern const struct iomap_write_ops ext4_iomap_write_ops;
>
> static inline int ext4_buffer_uptodate(struct buffer_head *bh)
> {
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index eb1a323962b1..7f9bfbbc4a4e 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -299,6 +299,21 @@ static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
> return count;
> }
>
> +static ssize_t ext4_iomap_buffered_write(struct kiocb *iocb,
> + struct iov_iter *from)
> +{
> + struct inode *inode = file_inode(iocb->ki_filp);
> + const struct iomap_ops *iomap_ops;
> +
> + 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_file_buffered_write(iocb, from, iomap_ops,
> + &ext4_iomap_write_ops, NULL);
> +}
> +
> static ssize_t ext4_buffered_write_iter(struct kiocb *iocb,
> struct iov_iter *from)
> {
> @@ -313,7 +328,10 @@ static ssize_t ext4_buffered_write_iter(struct kiocb *iocb,
> if (ret <= 0)
> goto out;
>
> - ret = generic_perform_write(iocb, from);
> + if (ext4_inode_buffered_iomap(inode))
> + ret = ext4_iomap_buffered_write(iocb, from);
> + else
> + ret = generic_perform_write(iocb, from);
>
> out:
> inode_unlock(inode);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 39577a6b65b9..1ae7d3f4a1c8 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3097,7 +3097,7 @@ static int ext4_dax_writepages(struct address_space *mapping,
> return ret;
> }
>
> -static int ext4_nonda_switch(struct super_block *sb)
> +int ext4_nonda_switch(struct super_block *sb)
> {
> s64 free_clusters, dirty_clusters;
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> @@ -3467,6 +3467,15 @@ static bool ext4_inode_datasync_dirty(struct inode *inode)
> return inode_state_read_once(inode) & I_DIRTY_DATASYNC;
> }
>
> +static bool ext4_iomap_valid(struct inode *inode, const struct iomap *iomap)
> +{
> + return iomap->validity_cookie == READ_ONCE(EXT4_I(inode)->i_es_seq);
> +}
> +
> +const struct iomap_write_ops ext4_iomap_write_ops = {
> + .iomap_valid = ext4_iomap_valid,
> +};
> +
> static void ext4_set_iomap(struct inode *inode, struct iomap *iomap,
> struct ext4_map_blocks *map, loff_t offset,
> loff_t length, unsigned int flags)
> @@ -3501,6 +3510,8 @@ static void ext4_set_iomap(struct inode *inode, struct iomap *iomap,
> !ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> iomap->flags |= IOMAP_F_MERGED;
>
> + iomap->validity_cookie = map->m_seq;
> +
> /*
> * Flags passed to ext4_map_blocks() for direct I/O writes can result
> * in m_flags having both EXT4_MAP_MAPPED and EXT4_MAP_UNWRITTEN bits
> @@ -3908,8 +3919,12 @@ const struct iomap_ops ext4_iomap_report_ops = {
> .iomap_begin = ext4_iomap_begin_report,
> };
>
> +/* Map blocks */
> +typedef int (ext4_get_blocks_t)(struct inode *, struct ext4_map_blocks *);
> +
> static int ext4_iomap_map_blocks(struct inode *inode, loff_t offset,
> - loff_t length, struct ext4_map_blocks *map)
> + loff_t length, ext4_get_blocks_t get_blocks,
> + struct ext4_map_blocks *map)
> {
> u8 blkbits = inode->i_blkbits;
>
> @@ -3921,6 +3936,9 @@ static int ext4_iomap_map_blocks(struct inode *inode, loff_t offset,
> map->m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
> EXT4_MAX_LOGICAL_BLOCK) - map->m_lblk + 1;
>
> + if (get_blocks)
> + return get_blocks(inode, map);
> +
> return ext4_map_blocks(NULL, inode, map, 0);
> }
>
> @@ -3938,7 +3956,7 @@ static int ext4_iomap_buffered_read_begin(struct inode *inode, loff_t offset,
> if (WARN_ON_ONCE(ext4_has_inline_data(inode)))
> return -ERANGE;
>
> - ret = ext4_iomap_map_blocks(inode, offset, length, &map);
> + ret = ext4_iomap_map_blocks(inode, offset, length, NULL, &map);
> if (ret < 0)
> return ret;
>
> @@ -3946,6 +3964,147 @@ static int ext4_iomap_buffered_read_begin(struct inode *inode, loff_t offset,
> return 0;
> }
>
> +static int ext4_iomap_get_blocks(struct inode *inode,
> + struct ext4_map_blocks *map)
> +{
> + loff_t i_size = i_size_read(inode);
> + handle_t *handle;
> + int ret;
> +
> + /*
> + * Check if the blocks have already been allocated, this could
> + * avoid initiating a new journal transaction and return the
> + * mapping information directly.
> + */
> + if ((map->m_lblk + map->m_len) <=
> + round_up(i_size, i_blocksize(inode)) >> inode->i_blkbits) {
> + ret = ext4_map_blocks(NULL, inode, map, 0);
> + if (ret < 0)
> + return ret;
> + if (map->m_flags & (EXT4_MAP_MAPPED | EXT4_MAP_UNWRITTEN |
> + EXT4_MAP_DELAYED))
> + return 0;
> + }
> +
> + handle = ext4_journal_start(inode, EXT4_HT_MAP_BLOCKS,
> + ext4_chunk_trans_blocks(inode, map->m_len));
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> + ret = ext4_map_blocks(handle, inode, map,
> + EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT);
> + /*
> + * Stop handle here following the lock ordering of the folio lock
> + * and the transaction start.
> + */
> + ext4_journal_stop(handle);
> +
> + return ret;
> +}
> +
> +static int ext4_iomap_buffered_do_write_begin(struct inode *inode,
> + loff_t offset, loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap, bool delalloc)
> +{
> + int ret, retries = 0;
> + struct ext4_map_blocks map;
> + ext4_get_blocks_t *get_blocks;
> +
> + ret = ext4_emergency_state(inode->i_sb);
> + if (unlikely(ret))
> + return ret;
> +
> + /* Inline data and non-extent are not supported. */
> + if (WARN_ON_ONCE(ext4_has_inline_data(inode)))
> + return -ERANGE;
> + if (WARN_ON_ONCE(!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> + return -EINVAL;
> + if (WARN_ON_ONCE(!(flags & IOMAP_WRITE)))
> + return -EINVAL;
> +
> + if (delalloc)
> + get_blocks = ext4_da_map_blocks;
> + else
> + get_blocks = ext4_iomap_get_blocks;
> +retry:
> + ret = ext4_iomap_map_blocks(inode, offset, length, get_blocks, &map);
> + if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
> + goto retry;
> + if (ret < 0)
> + return ret;
> +
> + ext4_set_iomap(inode, iomap, &map, offset, length, flags);
> + return 0;
> +}
> +
> +static int ext4_iomap_buffered_write_begin(struct inode *inode,
> + loff_t offset, loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap)
> +{
> + return ext4_iomap_buffered_do_write_begin(inode, offset, length, flags,
> + iomap, srcmap, false);
> +}
> +
> +static int ext4_iomap_buffered_da_write_begin(struct inode *inode,
> + loff_t offset, loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap)
> +{
> + return ext4_iomap_buffered_do_write_begin(inode, offset, length, flags,
> + iomap, srcmap, true);
> +}
> +
> +/*
> + * On write failure, drop the stale delayed allocation range and release
> + * its reserved space for both start and end blocks. Otherwise, we may
> + * leave a range of delayed extents covered by a clean folio, which can
> + * result in inaccurate space reservation accounting.
> + */
> +static void ext4_iomap_punch_delalloc(struct inode *inode, loff_t offset,
> + loff_t length, struct iomap *iomap)
> +{
> + down_write(&EXT4_I(inode)->i_data_sem);
> + ext4_es_remove_extent(inode, offset >> inode->i_blkbits,
> + DIV_ROUND_UP_ULL(length, EXT4_BLOCK_SIZE(inode->i_sb)));
> + up_write(&EXT4_I(inode)->i_data_sem);
> +}
> +
> +static int ext4_iomap_buffered_da_write_end(struct inode *inode, loff_t offset,
> + loff_t length, ssize_t written,
> + unsigned int flags,
> + struct iomap *iomap)
> +{
> + loff_t start_byte, end_byte;
> +
> + /* If we didn't reserve the blocks, we're not allowed to punch them. */
> + if (iomap->type != IOMAP_DELALLOC || !(iomap->flags & IOMAP_F_NEW))
> + 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));
> + if (start_byte >= end_byte)
> + return 0;
> +
> + filemap_invalidate_lock(inode->i_mapping);
> + iomap_write_delalloc_release(inode, start_byte, end_byte, flags,
> + iomap, ext4_iomap_punch_delalloc);
> + filemap_invalidate_unlock(inode->i_mapping);
> + return 0;
> +}
> +
> +/*
> + * Since we always allocate unwritten extents, there is no need for
> + * iomap_end to clean up allocated blocks on a short write.
> + */
> +const struct iomap_ops ext4_iomap_buffered_write_ops = {
> + .iomap_begin = ext4_iomap_buffered_write_begin,
> +};
> +
> +const struct iomap_ops ext4_iomap_buffered_da_write_ops = {
> + .iomap_begin = ext4_iomap_buffered_da_write_begin,
> + .iomap_end = ext4_iomap_buffered_da_write_end,
> +};
> +
> const struct iomap_ops ext4_iomap_buffered_read_ops = {
> .iomap_begin = ext4_iomap_buffered_read_begin,
> };
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6a77db4d3124..9bc294b769db 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -104,9 +104,13 @@ static const struct fs_parameter_spec ext4_param_specs[];
> * -> page lock -> i_data_sem (rw)
> *
> * buffered write path:
> - * sb_start_write -> i_mutex -> mmap_lock
> - * sb_start_write -> i_mutex -> transaction start -> page lock ->
> - * i_data_sem (rw)
> + * sb_start_write -> i_rwsem (w) -> mmap_lock
> + * - buffer_head path:
> + * sb_start_write -> i_rwsem (w) -> transaction start -> folio lock ->
> + * i_data_sem (rw)
> + * - iomap path:
> + * sb_start_write -> i_rwsem (w) -> transaction start -> i_data_sem (rw)
> + * sb_start_write -> i_rwsem (w) -> folio lock (not under an active handle)
> *
> * truncate:
> * sb_start_write -> i_mutex -> invalidate_lock (w) -> i_mmap_rwsem (w) ->
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 09/23] ext4: implement writeback path using iomap
From: Jan Kara @ 2026-06-16 11:47 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-10-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:29, 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.
>
> 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. Additionally, the deadlock that the
> reserve handle was expected to resolve does not occur anymore.
> 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;
Ugh, this adds 48 bytes to ext4 inode. That's pretty heavy. Cannot we reuse
i_rsv_conversion_list / work for this? For each inode only one of them
should be used AFAICS.
> 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)) {
> + 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)))
> + 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));
> + 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);
> + 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);
So it looks somewhat strange that here we call ext4_map_blocks() (which
consults extent status tree and then possibly on-disk extent tree) and then
we call ext4_iomap_map_one_extent() which manipulates with the extent
status tree and possibly extent tree as well. Is all this complexity to
avoid starting a jbd2 handle unless really needed? If yes, is that really
worth it? Given iomap code caches the extent we'd start the transaction
only once per mapped extent which shouldn't be that bad?
If you have some benchmark showing this is really worth it, then I'd
probably prefer coming up with an ext4_get_blocks flag which tells it to
start a transaction on its own if we need to allocate blocks... That would
be much simpler than opencoding all this.
> + 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;
> +}
> +
...
> +void ext4_iomap_end_bio(struct bio *bio)
> +{
> + struct iomap_ioend *ioend = iomap_ioend_from_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 flags;
> +
> + /* 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))
> + goto defer;
> +
> + /* Needs to abort the journal on data_err=abort. */
> + if (unlikely(ioend->io_bio.bi_status))
> + goto defer;
> +
> + iomap_finish_ioend(ioend, 0);
> + return;
> +defer:
> + spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> + if (list_empty(&ei->i_iomap_ioend_list))
> + queue_work(sbi->rsv_conversion_wq, &ei->i_iomap_ioend_work);
> + list_add_tail(&ioend->io_list, &ei->i_iomap_ioend_list);
> + spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
> +}
For now, I'd prefer to do what XFS does and offload everything. Then you
don't have to export iomap_finish_ioend() (which would need to be in a
separate patch and acked by iomap maintainers) and the code is more
standard. There's a patchset in the works which adds general ioend offloading
infrastructure into iomap [1] and when that lands we should get all these
bells and whistles (even better ones with percpu work queues, batching,
etc.) for free.
[1] https://lore.kernel.org/all/20260514-blk-dontcache-v6-0-782e2fa7477b@columbia.edu/
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 10/23] ext4: implement mmap path using iomap
From: Jan Kara @ 2026-06-16 11:56 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-11-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:30, 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: Jan Kara <jack@suse.cz>
Honza
> ---
> 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
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 14/23] ext4: implement partial block zero range path using iomap
From: Jan Kara @ 2026-06-16 12:28 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-15-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:34, 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>
> ---
> 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);
This looks like a layering violation to me. I don't think you can safely
assume the iomap you're passed is a part of iomap_iter...
> + 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;
> + }
... and you need access to iter only for this which seems to be really a
hack that's trying to outsmart the iomap code. I have to admit I don't
fully understand what you are trying to achieve here. Are you trying to
avoid flushing of the range that will be zeroed out?
> + 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.
> + */
Is this still true after your fix to did_zero handling?
> + 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.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 15/23] ext4: add block mapping tracepoints for iomap buffered I/O path
From: Jan Kara @ 2026-06-16 12:29 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-16-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:35, 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: Jan Kara <jack@suse.cz>
Honza
> ---
> 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
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 16/23] ext4: disable online defrag when inode using iomap buffered I/O path
From: Jan Kara @ 2026-06-16 12:30 UTC (permalink / raw)
To: Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, jack, ojaswin, ritesh.list, djwong, hch, yi.zhang,
yizhang089, yangerkun, yukuai
In-Reply-To: <20260511072344.191271-17-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:36, 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>
Sure. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> 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
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH RFC 2/8] fs: add a global device to super block hash table
From: Christoph Hellwig @ 2026-06-16 12:34 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Jan Kara, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel, Carlos Maiolino,
linux-xfs, Chris Mason, David Sterba, linux-btrfs,
Theodore Ts'o, linux-ext4, Gao Xiang, linux-erofs
In-Reply-To: <20260602-work-super-bdev_holder_global-v1-2-bb0fd82f3861@kernel.org>
On Tue, Jun 02, 2026 at 12:10:08PM +0200, Christian Brauner wrote:
> fs_holder_ops recovers the owning superblock from bdev->bd_holder, which
> forces the holder to be exactly one superblock and prevents several
> superblocks from sharing one block device. That's what erofs is doing.
>
> Introduce a global dev_t-keyed rhltable mapping each block device to the
> superblock(s) using it. The holder argument becomes purely the block
> layer's exclusivity token (a superblock, or a file_system_type for
> shared devices) and is no longer needed by the fs specific callbacks.
Err, no. block devices need to have a specific owner. If erofs wants
to share a device between superblock it needs to come up with an entity
that owns the block devices which is not a superblock.
IMHO sharing devices between superblocks is a bad idea, but that ship
has sailed, but please keep it contained inside of erofs.
^ permalink raw reply
* Re: [PATCH v4 06/23] ext4: pass out extent seq counter when mapping da blocks
From: Zhang Yi @ 2026-06-16 12:37 UTC (permalink / raw)
To: Jan Kara, Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, ojaswin, ritesh.list, djwong, hch, yi.zhang, yangerkun,
yukuai
In-Reply-To: <ghe4izo4od2b4fcgmiayopkaombyjjopao266sseuqim72t5sj@h2meez5lalsl>
On 6/16/2026 6:04 PM, Jan Kara wrote:
> On Mon 11-05-26 15:23:26, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> The iomap buffered write path does not hold any locks between querying
>> inode extent mapping information and performing buffered writes. It
>> relies on the sequence counter saved in the inode to detect stale
>> mappings.
>
> Now that I'm looking at it again, I've got a bit confused here. Buffered
> write path is holding i_rwsem between mapping blocks and using them so
> there shouldn't be races. Perhaps you mean buffered *writeback* path? But
> then ext4_da_map_blocks() should not ever get called in the writeback path
> because it is allocating delayed blocks... So this change looks unnecessary
> to me now. Am I missing something?
>
> Honza
>
Hi Jan,
Thanks for coming back to this series. Sorry for the inaccurate
description in the commit message. However, this change is still needed.
As mentioned in the comment before the ->iomap_valid() callback in
iomap_write_begin(), consider the following scenario — a buffered write
to a dirty unwritten extent, with this concurrent race:
Buffer write (holds i_rwsem) Writeback (no i_rwsem)
ext4_da_map_blocks()
// ext4_es_lookup_extent()
// finds UNWRITTEN extent
ext4_set_iomap()
// type = IOMAP_UNWRITTEN
// validity_cookie = m_seq
ext4_iomap_writepages()
// writeback for unwritten extent
// ext4_convert_unwritten_extents()
// extent tree: UNWRITTEN → WRITTEN
// advancing i_es_seq
__iomap_write_begin()
// ext4_iomap_valid(): cookie != i_es_seq
// iomap invalidated, re-maps
// gets type = IOMAP_MAPPED (WRITTEN)
// iomap_block_needs_zeroing(): FALSE
Without passing out m_seq, the stale IOMAP_UNWRITTEN type from the iomap
would cause __iomap_write_begin()->iomap_block_needs_zeroing() to zero
out blocks that have already been written, corrupting data on partial
writes.
Thanks,
Yi
>>
>> Commit 07c440e8da8f ("ext4: pass out extent seq counter when mapping
>> blocks") added the m_seq field to ext4_map_blocks to pass out extent
>> sequence numbers, but it missed two callsites within
>> ext4_da_map_blocks(). These callsites are on the delayed allocation
>> path, which is also used in the iomap buffered write path. Pass out the
>> sequence counter to ensure stale mappings can be detected.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> Reviewed-by: Jan Kara <jack@suse.cz>
>> ---
>> fs/ext4/inode.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 6c4d9137b279..39577a6b65b9 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -1929,7 +1929,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
>> ext4_check_map_extents_env(inode);
>>
>> /* Lookup extent status tree firstly */
>> - if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, NULL)) {
>> + if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
>> map->m_len = min_t(unsigned int, map->m_len,
>> es.es_len - (map->m_lblk - es.es_lblk));
>>
>> @@ -1982,7 +1982,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
>> * is held in write mode, before inserting a new da entry in
>> * the extent status tree.
>> */
>> - if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, NULL)) {
>> + if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
>> map->m_len = min_t(unsigned int, map->m_len,
>> es.es_len - (map->m_lblk - es.es_lblk));
>>
>> --
>> 2.52.0
>>
^ permalink raw reply
* Re: [PATCH 2/2] ext4: base unaligned DIO lock decision on partial block zeroing
From: Baokun Li @ 2026-06-16 13:10 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260611163441.2431805-3-libaokun@linux.alibaba.com>
Hi all,
Thank you for your review!
After extensive testing, I found that after merging this patch, generic/746
started failing intermittently on ext3 (mkfs.ext4 -O ^extents). The test
triggers a "Page cache invalidation failure on direct I/O" warning, and
subsequent fsync returns -EIO.
The underlying race existed before this patch, but this patch appears to
have widened the reproduction window considerably, so I thought it worth
trying to address. Here is my analysis:
On no-extent inodes, DIO writes that hit holes cannot use unwritten
extents. ext4_iomap_alloc() leaves m_flags=0, so ext4_map_blocks()
returns 0 for a hole, and:
if (!m_flags && !ret)
ret = -ENOTBLK;
The iomap layer returns -ENOTBLK to ext4, which falls back to buffered
I/O. The fallback path dirties pages in the page cache, then flushes
and invalidates them. However, concurrent async DIO completions to
other blocks on the same inode can run kiocb_invalidate_post_direct_write()
without holding the inode lock.
Consider a file with two 4k extents: [hole][written]. Thread A does DIO
to the written extent, while thread B does DIO spanning both extents:
kworker A (4k DIO, allocated block) kworker B (8k DIO, hole->fallback)
----------------------------------- -----------------------------------
inode_lock_shared() inode_lock_shared()
iomap_dio_rw(): iomap_dio_rw():
kiocb_invalidate_pages -> clean iomap_begin -> -ENOTBLK
submit_bio (async) dio->size = 0
inode_unlock_shared() inode_unlock_shared()
[bio pending in block layer] /* fallback: inode lock released */
ext4_buffered_write_iter()
inode_lock(exclusive)
generic_perform_write()
-> dirty pages [0, 8k]
inode_unlock(exclusive)
/* pages still dirty here */
[bio completes] filemap_write_and_wait_range()
iomap_dio_complete() -> flush dirty pages
kiocb_invalidate_post_direct_write() invalidate_mapping_pages()
invalidate_inode_pages2_range()
-> finds dirty page! /* window closed */
-> dio_warn_stale_pagecache()
-> errseq_set(-EIO)
The critical window is the gap between ext4_buffered_write_iter() dirtying
pages and filemap_write_and_wait_range() flushing them. In this window the
inode lock is not held, so another thread's async DIO completion is free to
invalidate the still-dirty pages in the page cache.
This race has always existed on ext3 because indirect-block inodes lack
unwritten-extent support. However, the window was extremely narrow in
practice, because the old ext4_overwrite_io() checked every block and
would conservatively take an exclusive lock. This patch replaced it
with ext4_dio_needs_zeroing(), which only checks head and tail blocks,
making unaligned DIO more likely to take a shared lock and
proportionally increasing the chance of hitting the race.
I tried a couple of alternatives before settling on the patch below:
1. Force exclusive lock + IOMAP_DIO_FORCE_WAIT for all no-extent DIO.
This closes the window for new DIO submissions, but does not protect
against bio completions from previously submitted async DIO, which
run independently of the inode lock.
2. Wrap the fallback dirty+flush+invalidate sequence in
filemap_invalidate_lock(). However, the ext4 DIO and iomap layers
do not use this lock, so it would not serialise against DIO
completions.
One straightforward approach that seems correct is to skip direct I/O
for no-extent inodes entirely, by returning 0 from ext4_dio_alignment():
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -6131,6 +6131,8 @@ u32 ext4_dio_alignment(struct inode *inode)
{
if (fsverity_active(inode))
return 0;
+ if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+ return 0;
if (ext4_should_journal_data(inode))
return 0;
if (ext4_has_inline_data(inode))
With this, ext4_should_use_dio() returns false for no-extent inodes, and
all I/O goes through ext4_buffered_write_iter() directly, bypassing the
DIO path entirely. On ext3, DIO to a hole already falls back to buffered
I/O, so there is essentially no performance benefit to using DIO in the
first place.
Note that with this change, the fallback branch in ext4_dio_write_iter():
if (ret >= 0 && iov_iter_count(from)) {
/* buffered fallback */
}
would also become dead code for extent-based inodes (since unwritten
extents guarantee iomap_dio_rw() never returns zero with unconsumed
data), and could be removed in a follow-up cleanup.
Thoughts? Is there a reason to preserve DIO on no-extent inodes that
I'm missing?
Looking forward to your feedback.
Thanks,
Baokun
^ permalink raw reply
* [PATCH RFC v2 00/18] fs: support freeze/thaw/mark_dead/sync with shared devices
From: Christian Brauner @ 2026-06-16 14:08 UTC (permalink / raw)
To: Jan Kara
Cc: Christoph Hellwig, Jens Axboe, Alexander Viro, linux-block,
linux-kernel, linux-fsdevel, Carlos Maiolino, linux-xfs,
Chris Mason, David Sterba, linux-btrfs, Theodore Ts'o,
linux-ext4, Gao Xiang, linux-erofs, Christian Brauner (Amutable),
syzbot, Gao Xiang
This is a generalization of the device number to superblock so it works
for actual block device and anonymous (or even mtd) devices.
fs_holder_ops recovers the affected superblock from bdev->bd_holder. That
forces the holder of a block device to be exactly one superblock and makes
it impossible for several superblocks to share a single device.
erofs does exactly that. It can mount read-only "blob" devices that are
shared between many superblocks: a metadata-only erofs that indexes a set
of per-layer blobs (one filesystem instead of one per OCI layer), or an
incremental image whose base device is shared by several updates. Because
the block layer only tracks a single holder, a freeze, thaw, removal or
sync on such a device is never propagated to all the superblocks using it,
and the current infrastructure has no way to find them.
This series replaces the bd_holder-based lookup with a global, dev_t-keyed
table mapping each block device to the superblock(s) using it. The holder
argument becomes purely the block layer's exclusivity token -- a superblock,
or the file_system_type for a device shared within one filesystem type --
and the fs_holder_ops callbacks look the device up in the table and act on
every superblock registered for it: 1:1 for most filesystems, 1:many for
erofs.
Filesystems claim and release their devices through new
fs_bdev_file_open_by_{dev,path}() and fs_bdev_file_release() helpers; the
per-fs patches convert xfs, btrfs, ext4, f2fs and erofs over to them and
fix cramfs and romfs, which released the registered main device with a
raw bdev_fput().
Since every superblock is registered under its s_dev the table also
replaces the last s_dev-keyed walk of the super_blocks list:
user_get_super() resolves device numbers through it, so ustat() and
quotactl() now work on any device a filesystem claims and no longer
take sb_lock.
The longer-term motivation is to let userspace decide which devices may be
onlined from one central place, without having to teach every filesystem
about it individually.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v2:
- super: rework the device-to-superblock table reference counting: each
(device, superblock) entry carries a single claim count and holds one
passive reference on its superblock for the entry's lifetime. New prep
patches convert s_count to refcount_t s_passive and make put_super()
self-locking.
- super: preallocate the entry in alloc_super() and register it from the
set callbacks through set_anon_super()/set_bdev_super(); an insert
failure unwinds exactly like a set callback failure. The superblock
stashes the entry in sb->s_super_dev and kill_super_notify() drops the
claim through it.
- super: initialize the table from mnt_init(); the rootfs and shm mounts
are created long before any initcall runs.
- super: fold the v1 "refuse to claim a frozen block device" patch into
the registration helper and restore the EBUSY check for the primary
device in setup_bdev_super(): additional devices (the xfs log, the ext4
journal, erofs blobs) are now refused while frozen as well, answering
Jan's question on v1 3/8.
- Split the core patch into table/helpers/switch-over and move the
xfs/btrfs/ext4 conversions before the fs_holder_ops switch so no
freeze/mark_dead events are lost mid-series; erofs follows the switch.
- New prep patches: the ext4 KUnit tests allocate anonymous devices and
ocfs2 stops resetting s_dev on dismount.
- New: convert user_get_super() to the device table, plus a ustat()
selftest.
- New: fix a pre-existing double release of the realtime device file and
dangling buftarg pointers in xfs_open_devices()'s error unwind.
- New: convert f2fs's additional devices to the helpers; fix cramfs and
romfs releasing the registered main device with a raw bdev_fput().
- erofs: drop the .shutdown() and .remove_bdev() implementations and the
per-device "dead" flag. Immutable filesystems don't need them: the block
layer sets GD_DEAD before fs_bdev_mark_dead() so in-flight bios fail
anyway, erofs has no write path or journal to stop, and the read-only
loop_change_fd() case must not be forced to -EIO. Patch from Gao Xiang,
applied verbatim - thanks!
- btrfs: fix a general protection fault in close_fs_devices() on a failed
mount (reported by syzbot). The release path took the superblock from
device->fs_info, which is still NULL if open_ctree() fails before
btrfs_init_devices_late(); it now uses bdev_file->private_data.
- erofs: the v1 conversion was sent with a generic boilerplate changelog;
superseded by Gao's patch above.
- Collect Reviewed-by from Jan Kara and Tested-by from syzbot.
- Rebase onto v7.1-rc1.
- Link to v1: https://patch.msgid.link/20260602-work-super-bdev_holder_global-v1-0-bb0fd82f3861@kernel.org
---
Christian Brauner (18):
xfs: fix the error unwind in xfs_open_devices()
super: convert s_count to refcount_t s_passive
super: take lock after last reference count
fs, block: move blk_mode_t and fop_flags_t into <linux/types.h>
ext4: use anonymous devices for KUnit test superblocks
ocfs2: don't reset s_dev on dismount
fs: maintain a global device-to-superblock table
fs: add dedicated block device open helpers for filesystems
xfs: port to fs_bdev_file_open_by_path()
btrfs: open via dedicated fs bdev helpers
ext4: open via dedicated fs bdev helpers
fs: look up superblocks via the device table in fs_holder_ops
fs: tolerate per-superblock freeze errors on shared devices
erofs: open via dedicated fs bdev helpers
f2fs: open via dedicated fs bdev helpers
super: make fs_holder_ops private
fs: look up the superblock via the device table in user_get_super()
selftests/filesystems: add ustat() coverage
fs/btrfs/volumes.c | 31 +-
fs/cramfs/inode.c | 2 +-
fs/erofs/super.c | 35 +-
fs/ext4/extents-test.c | 9 +-
fs/ext4/mballoc-test.c | 9 +-
fs/ext4/super.c | 12 +-
fs/f2fs/super.c | 6 +-
fs/internal.h | 1 +
fs/namespace.c | 2 +
fs/ocfs2/super.c | 1 -
fs/romfs/super.c | 2 +-
fs/super.c | 620 ++++++++++++++++-------
fs/xfs/xfs_buf.c | 2 +-
fs/xfs/xfs_super.c | 13 +-
include/linux/blkdev.h | 9 -
include/linux/fs.h | 2 -
include/linux/fs/super.h | 8 +
include/linux/fs/super_types.h | 4 +-
include/linux/types.h | 2 +
tools/testing/selftests/filesystems/.gitignore | 1 +
tools/testing/selftests/filesystems/Makefile | 2 +-
tools/testing/selftests/filesystems/ustat_test.c | 135 +++++
22 files changed, 647 insertions(+), 261 deletions(-)
---
base-commit: 0c0d974f62e6603d4514e1a8035658edb353c68f
change-id: 20260602-work-super-bdev_holder_global-8cba5e52bed5
^ permalink raw reply
* [PATCH RFC v2 01/18] xfs: fix the error unwind in xfs_open_devices()
From: Christian Brauner @ 2026-06-16 14:08 UTC (permalink / raw)
To: Jan Kara
Cc: Christoph Hellwig, Jens Axboe, Alexander Viro, linux-block,
linux-kernel, linux-fsdevel, Carlos Maiolino, linux-xfs,
Chris Mason, David Sterba, linux-btrfs, Theodore Ts'o,
linux-ext4, Gao Xiang, linux-erofs, Christian Brauner (Amutable)
In-Reply-To: <20260616-work-super-bdev_holder_global-v2-0-7df6b864028e@kernel.org>
Since the rt and log block devices are closed in xfs_free_buftarg() the
buftarg owns the device file. The error unwind does not respect that:
when the log buftarg allocation fails, out_free_rtdev_targ frees the rt
buftarg - releasing rtdev_file - and then falls through to
out_close_rtdev and releases it a second time.
The unwind also leaves mp->m_rtdev_targp and mp->m_ddev_targp pointing
to the freed buftargs. The failed mount continues into
deactivate_locked_super() -> xfs_kill_sb() -> xfs_mount_free(), which
frees them again.
Clear the buftarg pointers once the unwind freed them and clear
rtdev_file once the rt buftarg owns it, so nothing is released twice.
Reachable when a buftarg allocation fails after the data buftarg was
set up: an I/O error in sync_blockdev() or an allocation failure in
xfs_init_buftarg() while mounting with external rt and log devices.
Fixes: 41233576e9a4 ("xfs: close the RT and log block devices in xfs_free_buftarg")
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/xfs/xfs_super.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index eac7f9503805..8531d526fc44 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -534,8 +534,11 @@ xfs_open_devices(
out_free_rtdev_targ:
if (mp->m_rtdev_targp)
xfs_free_buftarg(mp->m_rtdev_targp);
+ mp->m_rtdev_targp = NULL;
+ rtdev_file = NULL; /* released by xfs_free_buftarg() */
out_free_ddev_targ:
xfs_free_buftarg(mp->m_ddev_targp);
+ mp->m_ddev_targp = NULL;
out_close_rtdev:
if (rtdev_file)
bdev_fput(rtdev_file);
--
2.47.3
^ permalink raw reply related
* [PATCH RFC v2 02/18] super: convert s_count to refcount_t s_passive
From: Christian Brauner @ 2026-06-16 14:08 UTC (permalink / raw)
To: Jan Kara
Cc: Christoph Hellwig, Jens Axboe, Alexander Viro, linux-block,
linux-kernel, linux-fsdevel, Carlos Maiolino, linux-xfs,
Chris Mason, David Sterba, linux-btrfs, Theodore Ts'o,
linux-ext4, Gao Xiang, linux-erofs, Christian Brauner (Amutable)
In-Reply-To: <20260616-work-super-bdev_holder_global-v2-0-7df6b864028e@kernel.org>
The superblock carries two counters: s_active, the active reference
count that keeps the filesystem usable, and s_count, the passive
reference count that merely keeps the structure itself alive. Turn the
passive count into a refcount_t and rename it to s_passive to make the
pairing with s_active obvious.
Everything is still serialized by sb_lock, so there is no functional
change; the conversion buys the usual refcount_t saturation and
underflow checking. The following patches start dropping passive
references without holding sb_lock and make the device-to-superblock
table hold one passive reference per registered entry, which a plain
integer cannot support.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/super.c | 18 +++++++++---------
include/linux/fs/super_types.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index a8fd61136aaf..25dd72b550e0 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -102,7 +102,7 @@ static bool super_flags(const struct super_block *sb, unsigned int flags)
* creation will succeed and SB_BORN is set by vfs_get_tree() or we're
* woken and we'll see SB_DYING.
*
- * The caller must have acquired a temporary reference on @sb->s_count.
+ * The caller must have acquired a temporary reference on @sb->s_passive.
*
* Return: The function returns true if SB_BORN was set and with
* s_umount held. The function returns false if SB_DYING was
@@ -367,7 +367,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
spin_lock_init(&s->s_inode_wblist_lock);
fserror_mount(s);
- s->s_count = 1;
+ refcount_set(&s->s_passive, 1);
atomic_set(&s->s_active, 1);
mutex_init(&s->s_vfs_rename_mutex);
lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
@@ -407,7 +407,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
*/
static void __put_super(struct super_block *s)
{
- if (!--s->s_count) {
+ if (refcount_dec_and_test(&s->s_passive)) {
list_del_init(&s->s_list);
WARN_ON(s->s_dentry_lru.node);
WARN_ON(s->s_inode_lru.node);
@@ -529,7 +529,7 @@ static bool grab_super(struct super_block *sb)
{
bool locked;
- sb->s_count++;
+ refcount_inc(&sb->s_passive);
spin_unlock(&sb_lock);
locked = super_lock_excl(sb);
if (locked) {
@@ -556,7 +556,7 @@ static bool grab_super(struct super_block *sb)
* lock held in read mode in case of success. On successful return,
* the caller must drop the s_umount lock when done.
*
- * Note that unlike get_super() et.al. this one does *not* bump ->s_count.
+ * Note that unlike get_super() et.al. this one does *not* bump ->s_passive.
* The reason why it's safe is that we are OK with doing trylock instead
* of down_read(). There's a couple of places that are OK with that, but
* it's very much not a general-purpose interface.
@@ -858,7 +858,7 @@ static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
sb = next_super(sb, flags)) {
if (super_flags(sb, SB_DYING))
continue;
- sb->s_count++;
+ refcount_inc(&sb->s_passive);
spin_unlock(&sb_lock);
if (flags & SUPER_ITER_UNLOCKED) {
@@ -903,7 +903,7 @@ void iterate_supers_type(struct file_system_type *type,
if (super_flags(sb, SB_DYING))
continue;
- sb->s_count++;
+ refcount_inc(&sb->s_passive);
spin_unlock(&sb_lock);
locked = super_lock_shared(sb);
@@ -935,7 +935,7 @@ struct super_block *user_get_super(dev_t dev, bool excl)
if (sb->s_dev != dev)
continue;
- sb->s_count++;
+ refcount_inc(&sb->s_passive);
spin_unlock(&sb_lock);
locked = super_lock(sb, excl);
@@ -1369,7 +1369,7 @@ static struct super_block *bdev_super_lock(struct block_device *bdev, bool excl)
/* Make sure sb doesn't go away from under us */
spin_lock(&sb_lock);
- sb->s_count++;
+ refcount_inc(&sb->s_passive);
spin_unlock(&sb_lock);
mutex_unlock(&bdev->bd_holder_lock);
diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
index ef7941e9dc79..68747182abf9 100644
--- a/include/linux/fs/super_types.h
+++ b/include/linux/fs/super_types.h
@@ -145,7 +145,7 @@ struct super_block {
unsigned long s_magic;
struct dentry *s_root;
struct rw_semaphore s_umount;
- int s_count;
+ refcount_t s_passive;
atomic_t s_active;
#ifdef CONFIG_SECURITY
void *s_security;
--
2.47.3
^ permalink raw reply related
* [PATCH RFC v2 03/18] super: take lock after last reference count
From: Christian Brauner @ 2026-06-16 14:08 UTC (permalink / raw)
To: Jan Kara
Cc: Christoph Hellwig, Jens Axboe, Alexander Viro, linux-block,
linux-kernel, linux-fsdevel, Carlos Maiolino, linux-xfs,
Chris Mason, David Sterba, linux-btrfs, Theodore Ts'o,
linux-ext4, Gao Xiang, linux-erofs, Christian Brauner (Amutable)
In-Reply-To: <20260616-work-super-bdev_holder_global-v2-0-7df6b864028e@kernel.org>
__put_super() required the caller to hold sb_lock, so put_super()
wrapped it. The per-device superblock table introduced later drops its
passive references from contexts that do not hold sb_lock, so make
put_super() self-locking: drop the count first and take sb_lock only for
the final list_del.
With the count now dropped outside sb_lock a superblock can briefly sit
on @super_blocks with s_passive == 0 before it is unlinked, so the list
walkers (__iterate_supers(), iterate_supers_type(), user_get_super())
switch to refcount_inc_not_zero() and skip it.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/super.c | 63 ++++++++++++++++++++++++++++----------------------------------
1 file changed, 28 insertions(+), 35 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 25dd72b550e0..a771a0ad4c9a 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -403,12 +403,17 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
/* Superblock refcounting */
/*
- * Drop a superblock's refcount. The caller must hold sb_lock.
+ * Drop a superblock's passive reference. Must be called WITHOUT sb_lock held;
+ * put_super() acquires sb_lock itself when the final reference is dropped.
*/
-static void __put_super(struct super_block *s)
+void put_super(struct super_block *s)
{
if (refcount_dec_and_test(&s->s_passive)) {
+
+ spin_lock(&sb_lock);
list_del_init(&s->s_list);
+ spin_unlock(&sb_lock);
+
WARN_ON(s->s_dentry_lru.node);
WARN_ON(s->s_inode_lru.node);
WARN_ON(s->s_mounts);
@@ -416,20 +421,6 @@ static void __put_super(struct super_block *s)
}
}
-/**
- * put_super - drop a temporary reference to superblock
- * @sb: superblock in question
- *
- * Drops a temporary reference, frees superblock if there's no
- * references left.
- */
-void put_super(struct super_block *sb)
-{
- spin_lock(&sb_lock);
- __put_super(sb);
- spin_unlock(&sb_lock);
-}
-
static void kill_super_notify(struct super_block *sb)
{
lockdep_assert_not_held(&sb->s_umount);
@@ -478,11 +469,7 @@ void deactivate_locked_super(struct super_block *s)
kill_super_notify(s);
- /*
- * Since list_lru_destroy() may sleep, we cannot call it from
- * put_super(), where we hold the sb_lock. Therefore we destroy
- * the lru lists right now.
- */
+ /* list_lru_destroy() may sleep; put_super() callers may not. */
list_lru_destroy(&s->s_dentry_lru);
list_lru_destroy(&s->s_inode_lru);
@@ -851,14 +838,17 @@ static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
struct super_block *sb, *p = NULL;
bool excl = flags & SUPER_ITER_EXCL;
- guard(spinlock)(&sb_lock);
+ spin_lock(&sb_lock);
for (sb = first_super(flags);
!list_entry_is_head(sb, &super_blocks, s_list);
sb = next_super(sb, flags)) {
if (super_flags(sb, SB_DYING))
continue;
- refcount_inc(&sb->s_passive);
+
+ if (!refcount_inc_not_zero(&sb->s_passive))
+ continue;
+
spin_unlock(&sb_lock);
if (flags & SUPER_ITER_UNLOCKED) {
@@ -868,13 +858,14 @@ static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
super_unlock(sb, excl);
}
- spin_lock(&sb_lock);
if (p)
- __put_super(p);
+ put_super(p);
p = sb;
+ spin_lock(&sb_lock);
}
+ spin_unlock(&sb_lock);
if (p)
- __put_super(p);
+ put_super(p);
}
void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
@@ -903,7 +894,9 @@ void iterate_supers_type(struct file_system_type *type,
if (super_flags(sb, SB_DYING))
continue;
- refcount_inc(&sb->s_passive);
+ if (!refcount_inc_not_zero(&sb->s_passive))
+ continue;
+
spin_unlock(&sb_lock);
locked = super_lock_shared(sb);
@@ -912,14 +905,14 @@ void iterate_supers_type(struct file_system_type *type,
super_unlock_shared(sb);
}
- spin_lock(&sb_lock);
if (p)
- __put_super(p);
+ put_super(p);
p = sb;
+ spin_lock(&sb_lock);
}
- if (p)
- __put_super(p);
spin_unlock(&sb_lock);
+ if (p)
+ put_super(p);
}
EXPORT_SYMBOL(iterate_supers_type);
@@ -935,15 +928,17 @@ struct super_block *user_get_super(dev_t dev, bool excl)
if (sb->s_dev != dev)
continue;
- refcount_inc(&sb->s_passive);
+ if (!refcount_inc_not_zero(&sb->s_passive))
+ continue;
+
spin_unlock(&sb_lock);
locked = super_lock(sb, excl);
if (locked)
return sb;
+ put_super(sb);
spin_lock(&sb_lock);
- __put_super(sb);
break;
}
spin_unlock(&sb_lock);
@@ -1368,9 +1363,7 @@ static struct super_block *bdev_super_lock(struct block_device *bdev, bool excl)
lockdep_assert_not_held(&bdev->bd_disk->open_mutex);
/* Make sure sb doesn't go away from under us */
- spin_lock(&sb_lock);
refcount_inc(&sb->s_passive);
- spin_unlock(&sb_lock);
mutex_unlock(&bdev->bd_holder_lock);
--
2.47.3
^ 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