* Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput
From: Zhou, Yun @ 2026-06-19 6:24 UTC (permalink / raw)
To: Jan Kara
Cc: tytso, adilger.kernel, libaokun, ojaswin, ritesh.list, yi.zhang,
linux-ext4, linux-kernel
In-Reply-To: <jxcbsd2ot63wy3dcoximemkuitwoqn2a7jgxcsfdwaf5q3ecdu@sahahqqopo6y>
On 6/18/2026 2:42 AM, Jan Kara wrote:
> On Tue 16-06-26 23:15:57, Yun Zhou wrote:
>> +
>> + /* Deferred iput for EA inodes to avoid lock ordering issues */
>> + struct llist_head s_ea_inode_to_free;
>> + struct work_struct s_ea_inode_work;
>> +
>
> I'd probably use delayed work and schedule it with a delay of one jiffie so
> that some inodes can accumulate before we process them which should reduce
> the amount of task switching to workqueues.
>
Good idea, I will use delayed_work in next version.
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 6a77db4d3124..b777bb0a81ea 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1308,6 +1308,9 @@ static void ext4_put_super(struct super_block *sb)
>> destroy_workqueue(sbi->rsv_conversion_wq);
>> ext4_release_orphan_info(sb);
>>
>> + /* Flush deferred EA inode iputs before destroying journal */
>> + flush_work(&sbi->s_ea_inode_work);
>> +
>
> This should happen earlier in ext4_put_super(). At this place quotas were
> already turned off and so quota accounting would go wrong.
That makes sense. I'll move it up to right before ext4_quotas_off().
>> +static void ext4_xattr_inode_array_free_deferred(struct super_block *sb,
>> + struct ext4_xattr_inode_array *array)
>
> The array of EA inodes used in xattr handling is just another mechanism
> used for delaying iput() of EA inodes. It doesn't make sense to stack these
> to one on top of another. Just completely replace the array mechanism with
> always deferring iput of EA inode into the workqueue.
>
I'm thinking that a complete replacement might be too large a change.
Should we consider postponing this work, or perhaps appending a new
patch to this series to handle it?
>
> Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
> I want to suggest another scheme (somewhat more involved but more efficient
> scheme):
>
> 1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
> drops inode reference if it is not the last one (and returns true in that
> case). Basically:
>
> bool iput_if_not_last(struct inode *inode)
> {
> return atomic_add_unless(&inode->i_count, -1, 1);
> }
>
> This needs to be a separate patch as it should get vetting from VFS
> maintainers.
>
> 2) Use iput_if_not_last() in ext4_put_ea_inode(). If it returns true, we
> are done. Otherwise we know we were at least for a moment holders of the
> last inode reference, so we link the inode to the list of inodes to drop
> through llist_node embedded in ext4_inode_info. We cannot race with anybody
> else trying to link the same inode into the list because we hold one inode
> ref and so nobody else can hit this "I was holding the last ref" path.
> I'd union this llist_node say with xattr_sem which is unused for EA inodes
> to avoid growing ext4_inode_info.
>
> This way we avoid offloading unless really necessary and we don't have to
> do allocations just to drop EA inode ref.
>
Your idea makes a lot of sense. It greatly simplifies the current deferred
iput logic and eliminates the risk of failing to allocate an entry during
an OOM. However, as you mentioned, getting the VFS maintainers to agree
might be quite challenging.
BR,
Yun
^ permalink raw reply
* WARNING: at ext4_check_map_extents_env, CPU: syz.NUM.NUM/ADDR
From: sanan.hasanou @ 2026-06-18 22:26 UTC (permalink / raw)
To: tytso, adilger.kernel, linux-ext4, linux-kernel; +Cc: syzkaller, contact
Good day, dear maintainers,
We found a bug using a modified version of syzkaller.
Kernel Branch: 7.0-rc1
Kernel Config: <https://drive.google.com/open?id=173DLEAEPKPhhR1TcqofdnkLpdoK7PMFl>
Unfortunately, we don't have any reproducer for this bug yet.
Thank you!
Best regards,
Sanan Hasanov
EXT4-fs (loop7): stripe (65535) is not aligned with cluster size (16), stripe is disabled
[EXT4 FS bs=1024, gc=1, bpg=131072, ipg=32, mo=a840e11d, mo2=0002]
------------[ cut here ]------------
WARNING: at ext4_check_map_extents_env+0x471/0x510 fs/ext4/inode.c:436, CPU#1: syz.7.16867/107084
Modules linked in:
CPU: 1 UID: 0 PID: 107084 Comm: syz.7.16867 Not tainted 7.0.0-rc1 #1 PREEMPT(full)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:ext4_check_map_extents_env+0x471/0x510 fs/ext4/inode.c:436
Code: ff e9 89 fc ff ff 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 05 fd ff ff 4c 89 e7 e8 da d6 ae ff e9 f8 fc ff ff e8 60 a7 42 ff 90 <0f> 0b 90 e9 8a fc ff ff 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 25
RSP: 0018:ffffc900015376c8 EFLAGS: 00010283
RAX: ffffffff827faa70 RBX: 0000000000000000 RCX: 0000000000080000
RDX: ffffc900150f1000 RSI: 00000000000052eb RDI: 00000000000052ec
RBP: 0000000000000000 R08: ffff888034a03be7 R09: 1ffff1100694077c
R10: dffffc0000000000 R11: ffffed100694077d R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 00007f2dfb24b6c0(0000) GS:ffff8880d99df000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f2dfa45fb85 CR3: 0000000038fd7000 CR4: 00000000000006f0
Call Trace:
<TASK>
ext4_map_blocks+0x1e9/0x1540 fs/ext4/inode.c:721
ext4_protect_reserved_inode fs/ext4/block_validity.c:168 [inline]
ext4_setup_system_zone+0x872/0xa90 fs/ext4/block_validity.c:251
__ext4_fill_super fs/ext4/super.c:5594 [inline]
ext4_fill_super+0x534c/0x6390 fs/ext4/super.c:5791
get_tree_bdev_flags+0x3fe/0x4c0 fs/super.c:1694
vfs_get_tree+0x8e/0x290 fs/super.c:1754
fc_mount fs/namespace.c:1193 [inline]
do_new_mount_fc fs/namespace.c:3760 [inline]
do_new_mount+0x31f/0xd40 fs/namespace.c:3836
do_mount fs/namespace.c:4159 [inline]
__do_sys_mount fs/namespace.c:4348 [inline]
__se_sys_mount+0x3a1/0x4b0 fs/namespace.c:4325
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x19a/0x7b0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x7f2dfa3a559e
Code: 0f 1f 40 00 48 c7 c2 b0 ff ff ff f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f2dfb24ae28 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007f2dfb24aec0 RCX: 00007f2dfa3a559e
RDX: 0000200000000080 RSI: 0000200000000040 RDI: 00007f2dfb24ae80
RBP: 0000200000000080 R08: 00007f2dfb24aec0 R09: 0000000000000011
R10: 0000000000000011 R11: 0000000000000246 R12: 0000200000000040
R13: 00007f2dfb24ae80 R14: 000000000000060c R15: 0000200000000180
</TASK>
<<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>
^ permalink raw reply
* WARNING: at ext4_check_map_extents_env, CPU: syz.NUM.NUM/NUM
From: sanan.hasanou @ 2026-06-18 22:24 UTC (permalink / raw)
To: tytso, adilger.kernel, linux-ext4, linux-kernel; +Cc: syzkaller, contact
Good day, dear maintainers,
We found a bug using a modified version of syzkaller.
Kernel Branch: 7.0-rc1
Kernel Config: <https://drive.google.com/open?id=173DLEAEPKPhhR1TcqofdnkLpdoK7PMFl>
Reproducer: <https://drive.google.com/open?id=1_mGsSS7wCRfk8qXdMEw08C6S49PejwXr>
Thank you!
Best regards,
Sanan Hasanov
EXT4-fs (loop0): stripe (65535) is not aligned with cluster size (16), stripe is disabled
[EXT4 FS bs=1024, gc=1, bpg=131072, ipg=32, mo=a840e11d, mo2=0002]
------------[ cut here ]------------
WARNING: at ext4_check_map_extents_env+0x471/0x510 fs/ext4/inode.c:436, CPU#0: syz.0.1217/21399
Modules linked in:
CPU: 0 UID: 0 PID: 21399 Comm: syz.0.1217 Not tainted 7.0.0-rc1 #1 PREEMPT(full)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:ext4_check_map_extents_env+0x471/0x510 fs/ext4/inode.c:436
Code: ff e9 89 fc ff ff 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 05 fd ff ff 4c 89 e7 e8 da d6 ae ff e9 f8 fc ff ff e8 60 a7 42 ff 90 <0f> 0b 90 e9 8a fc ff ff 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 25
RSP: 0018:ffffc90002ce76c8 EFLAGS: 00010287
RAX: ffffffff827faa70 RBX: 0000000000000000 RCX: 0000000000080000
RDX: ffffc900019f5000 RSI: 00000000000030b1 RDI: 00000000000030b2
RBP: 0000000000000000 R08: ffff888017201637 R09: 1ffff11002e402c6
R10: dffffc0000000000 R11: ffffed1002e402c7 R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 00007fbe746086c0(0000) GS:ffff8880d98df000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe495821940 CR3: 00000000683d9000 CR4: 00000000000006f0
Call Trace:
<TASK>
ext4_map_blocks+0x1e9/0x1540 fs/ext4/inode.c:721
ext4_protect_reserved_inode fs/ext4/block_validity.c:168 [inline]
ext4_setup_system_zone+0x872/0xa90 fs/ext4/block_validity.c:251
__ext4_fill_super fs/ext4/super.c:5594 [inline]
ext4_fill_super+0x534c/0x6390 fs/ext4/super.c:5791
get_tree_bdev_flags+0x3fe/0x4c0 fs/super.c:1694
vfs_get_tree+0x8e/0x290 fs/super.c:1754
fc_mount fs/namespace.c:1193 [inline]
do_new_mount_fc fs/namespace.c:3760 [inline]
do_new_mount+0x31f/0xd40 fs/namespace.c:3836
do_mount fs/namespace.c:4159 [inline]
__do_sys_mount fs/namespace.c:4348 [inline]
__se_sys_mount+0x3a1/0x4b0 fs/namespace.c:4325
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x19a/0x7b0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x7fbe737a559e
Code: 0f 1f 40 00 48 c7 c2 b0 ff ff ff f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fbe74607e28 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007fbe74607ec0 RCX: 00007fbe737a559e
RDX: 0000200000000080 RSI: 0000200000000040 RDI: 00007fbe74607e80
RBP: 0000200000000080 R08: 00007fbe74607ec0 R09: 0000000000000011
R10: 0000000000000011 R11: 0000000000000246 R12: 0000200000000040
R13: 00007fbe74607e80 R14: 000000000000060c R15: 0000200000000180
</TASK>
<<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>
^ permalink raw reply
* [PATCH] fscrypt: Fix key setup in edge case with multiple data unit sizes
From: Eric Biggers @ 2026-06-18 18:06 UTC (permalink / raw)
To: linux-fscrypt
Cc: Theodore Ts'o, Jaegeuk Kim, linux-kernel, linux-fsdevel,
linux-ext4, linux-f2fs-devel, Eric Biggers, stable
The addition of support for customizable data unit sizes introduced an
edge case where a file's contents can be en/decrypted with the wrong
data unit size. It occurs when there are multiple v2 policies that:
- Have *different* data unit sizes, via the log2_data_unit_size field
- Share the same master_key_identifier, contents_encryption_mode, and
either FSCRYPT_POLICY_FLAG_DIRECT_KEY,
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32, or
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64
- Are being used on the same filesystem, which also must be mounted with
the "inlinecrypt" mount option.
Fortunately this edge case doesn't actually occur in practice. I just
found it via code review. But it needs to be fixed regardless.
The bug is caused by the data unit size not being fully considered when
blk_crypto_keys are cached in mk_direct_keys, mk_iv_ino_lblk_32_keys,
and mk_iv_ino_lblk_64_keys. They're differentiated only by master key,
encryption mode, and flag. However, each one actually has a data unit
size too. Only the first data unit size that is cached is used.
To fix this, start using the data unit size to differentiate the cached
keys. For several reasons, including avoiding increasing the size of
struct fscrypt_master_key, just replace all three arrays with a single
linked list instead of changing them into two-dimensional arrays. This
works well when considering that in practice at most 2 entries are used
across all three arrays, so it was already mostly wasted space.
For simplicity, make the list also take over the publish/subscribe of
the prepared key itself. That is, create separate list nodes for
blk_crypto_keys vs crypto_skciphers, and add nodes to the list only when
their key is actually prepared. (Note that the legacy
fscrypt_direct_keys table in fs/crypto/keysetup_v1.c already works this
way.) This eliminates the need for the additional memory barriers when
reading and writing the fields of struct fscrypt_prepared_key.
Note that I technically should have included the data unit size in the
HKDF info string as well. But it's too late to change that.
Fixes: 5b1188847180 ("fscrypt: support crypto data unit size less than filesystem block size")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
I'm planning to take this via the fscrypt tree for 7.2
fs/crypto/fscrypt_private.h | 52 +++++++++-------
fs/crypto/inline_crypt.c | 8 +--
fs/crypto/keyring.c | 23 ++++---
fs/crypto/keysetup.c | 118 ++++++++++++++++++++++--------------
4 files changed, 120 insertions(+), 81 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 8d3c278a7591..4263cac24b32 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -234,19 +234,28 @@ struct fscrypt_symlink_data {
/**
* struct fscrypt_prepared_key - a key prepared for actual encryption/decryption
* @tfm: crypto API transform object
* @blk_key: key for blk-crypto
*
- * Normally only one of the fields will be non-NULL.
+ * Only one of the fields is non-NULL.
*/
struct fscrypt_prepared_key {
struct crypto_sync_skcipher *tfm;
#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
struct blk_crypto_key *blk_key;
#endif
};
+/* An entry in the linked list ->mk_mode_keys */
+struct fscrypt_mode_key {
+ struct fscrypt_prepared_key key;
+ struct list_head link;
+ u8 hkdf_context;
+ u8 mode_num;
+ u8 data_unit_bits;
+};
+
/*
* fscrypt_inode_info - the "encryption key" for an inode
*
* When an encrypted file's key is made available, an instance of this struct is
* allocated and a pointer to it is stored in the file's in-memory inode. Once
@@ -428,24 +437,16 @@ int fscrypt_derive_sw_secret(struct super_block *sb,
/*
* Check whether the crypto transform or blk-crypto key has been allocated in
* @prep_key, depending on which encryption implementation the file will use.
*/
static inline bool
-fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
+fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
const struct fscrypt_inode_info *ci)
{
- /*
- * The two smp_load_acquire()'s here pair with the smp_store_release()'s
- * in fscrypt_prepare_inline_crypt_key() and fscrypt_prepare_key().
- * I.e., in some cases (namely, if this prep_key is a per-mode
- * encryption key) another task can publish blk_key or tfm concurrently,
- * executing a RELEASE barrier. We need to use smp_load_acquire() here
- * to safely ACQUIRE the memory the other task published.
- */
if (fscrypt_using_inline_encryption(ci))
- return smp_load_acquire(&prep_key->blk_key) != NULL;
- return smp_load_acquire(&prep_key->tfm) != NULL;
+ return prep_key->blk_key != NULL;
+ return prep_key->tfm != NULL;
}
#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
static inline int fscrypt_select_encryption_impl(struct fscrypt_inode_info *ci,
@@ -484,14 +485,14 @@ fscrypt_derive_sw_secret(struct super_block *sb,
fscrypt_warn(NULL, "kernel doesn't support hardware-wrapped keys");
return -EOPNOTSUPP;
}
static inline bool
-fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
+fscrypt_is_key_prepared(const struct fscrypt_prepared_key *prep_key,
const struct fscrypt_inode_info *ci)
{
- return smp_load_acquire(&prep_key->tfm) != NULL;
+ return prep_key->tfm != NULL;
}
#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
/* keyring.c */
@@ -575,12 +576,12 @@ struct fscrypt_master_key {
struct rw_semaphore mk_sem;
/*
* Active and structural reference counts. An active ref guarantees
* that the struct continues to exist, continues to be in the keyring
- * ->s_master_keys, and that any embedded subkeys (e.g.
- * ->mk_direct_keys) that have been prepared continue to exist.
+ * ->s_master_keys, and that any non-file-scoped subkeys (e.g.
+ * ->mk_mode_keys) that have been prepared continue to exist.
* A structural ref only guarantees that the struct continues to exist.
*
* There is one active ref associated with ->mk_present being true, and
* one active ref for each inode in ->mk_decrypted_inodes.
*
@@ -630,16 +631,25 @@ struct fscrypt_master_key {
*/
struct list_head mk_decrypted_inodes;
spinlock_t mk_decrypted_inodes_lock;
/*
- * Per-mode encryption keys for the various types of encryption policies
- * that use them. Allocated and derived on-demand.
+ * A list of 'struct fscrypt_mode_key' for the (hkdf_context, mode_num,
+ * data_unit_bits, inlinecrypt) combinations that are in use for this
+ * master key, for hkdf_context in [HKDF_CONTEXT_DIRECT_KEY,
+ * HKDF_CONTEXT_IV_INO_LBLK_32_KEY, HKDF_CONTEXT_IV_INO_LBLK_64_KEY].
+ *
+ * This is a linked list and not a hash table because in practice
+ * there's just a single encryption policy per master key, using
+ * _at most_ 2 nodes in this list. Per-file keys don't use this at all.
+ *
+ * This list is append-only until the master key is fully removed, at
+ * which time the list is cleared. Before then,
+ * fscrypt_mode_key_setup_mutex synchronizes appends, and searches use
+ * the RCU read lock together with ->mk_sem held for read.
*/
- struct fscrypt_prepared_key mk_direct_keys[FSCRYPT_MODE_MAX + 1];
- struct fscrypt_prepared_key mk_iv_ino_lblk_64_keys[FSCRYPT_MODE_MAX + 1];
- struct fscrypt_prepared_key mk_iv_ino_lblk_32_keys[FSCRYPT_MODE_MAX + 1];
+ struct list_head mk_mode_keys;
/* Hash key for inode numbers. Initialized only when needed. */
siphash_key_t mk_ino_hash_key;
bool mk_ino_hash_key_initialized;
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 37d42d357925..47324062fee5 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -196,17 +196,11 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
if (err) {
fscrypt_err(inode, "error %d starting to use blk-crypto", err);
goto fail;
}
- /*
- * Pairs with the smp_load_acquire() in fscrypt_is_key_prepared().
- * I.e., here we publish ->blk_key with a RELEASE barrier so that
- * concurrent tasks can ACQUIRE it. Note that this concurrency is only
- * possible for per-mode keys, not for per-file keys.
- */
- smp_store_release(&prep_key->blk_key, blk_key);
+ prep_key->blk_key = blk_key;
return 0;
fail:
kfree_sensitive(blk_key);
return err;
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index be8e6e8011f2..5fe0d985a58d 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -85,18 +85,18 @@ void fscrypt_put_master_key(struct fscrypt_master_key *mk)
}
void fscrypt_put_master_key_activeref(struct super_block *sb,
struct fscrypt_master_key *mk)
{
- size_t i;
+ struct fscrypt_mode_key *node, *tmp;
if (!refcount_dec_and_test(&mk->mk_active_refs))
return;
/*
* No active references left, so complete the full removal of this
* fscrypt_master_key struct by removing it from the keyring and
- * destroying any subkeys embedded in it.
+ * destroying any non-file-scoped subkeys.
*/
if (WARN_ON_ONCE(!sb->s_master_keys))
return;
spin_lock(&sb->s_master_keys->lock);
@@ -108,17 +108,20 @@ void fscrypt_put_master_key_activeref(struct super_block *sb,
* ->mk_decrypted_inodes is empty.
*/
WARN_ON_ONCE(mk->mk_present);
WARN_ON_ONCE(!list_empty(&mk->mk_decrypted_inodes));
- for (i = 0; i <= FSCRYPT_MODE_MAX; i++) {
- fscrypt_destroy_prepared_key(
- sb, &mk->mk_direct_keys[i]);
- fscrypt_destroy_prepared_key(
- sb, &mk->mk_iv_ino_lblk_64_keys[i]);
- fscrypt_destroy_prepared_key(
- sb, &mk->mk_iv_ino_lblk_32_keys[i]);
+ /*
+ * Destroy any non-file-scoped subkeys. Since ->mk_active_refs == 0,
+ * they're no longer referenced by any inodes. Nor can key setup run
+ * and use them again. So they're no longer needed. (This implies no
+ * concurrent readers, so we don't need list_del_rcu() for example.)
+ */
+ list_for_each_entry_safe(node, tmp, &mk->mk_mode_keys, link) {
+ fscrypt_destroy_prepared_key(sb, &node->key);
+ list_del(&node->link);
+ kfree(node);
}
memzero_explicit(&mk->mk_ino_hash_key,
sizeof(mk->mk_ino_hash_key));
mk->mk_ino_hash_key_initialized = false;
@@ -443,10 +446,12 @@ static int add_new_master_key(struct super_block *sb,
mk->mk_spec = *mk_spec;
INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
spin_lock_init(&mk->mk_decrypted_inodes_lock);
+ INIT_LIST_HEAD(&mk->mk_mode_keys);
+
if (mk_spec->type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
err = allocate_master_key_users_keyring(mk);
if (err)
goto out_put;
err = add_master_key_user(mk);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index ce327bfdada4..f905f9f94bdd 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -161,17 +161,11 @@ int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
false, ci);
tfm = fscrypt_allocate_skcipher(ci->ci_mode, raw_key, ci->ci_inode);
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- /*
- * Pairs with the smp_load_acquire() in fscrypt_is_key_prepared().
- * I.e., here we publish ->tfm with a RELEASE barrier so that
- * concurrent tasks can ACQUIRE it. Note that this concurrency is only
- * possible for per-mode keys, not for per-file keys.
- */
- smp_store_release(&prep_key->tfm, tfm);
+ prep_key->tfm = tfm;
return 0;
}
/* Destroy a crypto transform object and/or blk-crypto key. */
void fscrypt_destroy_prepared_key(struct super_block *sb,
@@ -188,21 +182,50 @@ int fscrypt_set_per_file_enc_key(struct fscrypt_inode_info *ci,
{
ci->ci_owns_key = true;
return fscrypt_prepare_key(&ci->ci_enc_key, raw_key, ci);
}
+/*
+ * Find the fscrypt_prepared_key (if any) for a particular (mk, hkdf_context,
+ * mode_num, data_unit_bits, inlinecrypt) combination.
+ *
+ * The caller must hold ->mk_sem for reading and ->mk_present must be true,
+ * ensuring that ->mk_mode_keys is still append-only.
+ */
+static struct fscrypt_prepared_key *
+fscrypt_find_mode_key(struct fscrypt_master_key *mk, u8 hkdf_context,
+ u8 mode_num, const struct fscrypt_inode_info *ci)
+{
+ struct fscrypt_mode_key *node;
+
+ /*
+ * The RCU read lock here is used only to synchronize with concurrent
+ * list_add_tail_rcu(). Concurrent deletions are impossible here, so
+ * returning a pointer to a node without taking any refcount is safe.
+ */
+ guard(rcu)();
+ list_for_each_entry_rcu(node, &mk->mk_mode_keys, link) {
+ if (node->hkdf_context == hkdf_context &&
+ node->mode_num == mode_num &&
+ node->data_unit_bits == ci->ci_data_unit_bits &&
+ fscrypt_is_key_prepared(&node->key, ci))
+ return &node->key;
+ }
+ return NULL;
+}
+
static int setup_per_mode_enc_key(struct fscrypt_inode_info *ci,
struct fscrypt_master_key *mk,
- struct fscrypt_prepared_key *keys,
u8 hkdf_context, bool include_fs_uuid)
{
const struct inode *inode = ci->ci_inode;
const struct super_block *sb = inode->i_sb;
struct fscrypt_mode *mode = ci->ci_mode;
const u8 mode_num = mode - fscrypt_modes;
struct fscrypt_prepared_key *prep_key;
- u8 mode_key[FSCRYPT_MAX_RAW_KEY_SIZE];
+ struct fscrypt_mode_key *new_node;
+ u8 raw_mode_key[FSCRYPT_MAX_RAW_KEY_SIZE];
u8 hkdf_info[sizeof(mode_num) + sizeof(sb->s_uuid)];
unsigned int hkdf_infolen = 0;
bool use_hw_wrapped_key = false;
int err;
@@ -221,52 +244,60 @@ static int setup_per_mode_enc_key(struct fscrypt_inode_info *ci,
return -EINVAL;
}
use_hw_wrapped_key = true;
}
- prep_key = &keys[mode_num];
- if (fscrypt_is_key_prepared(prep_key, ci)) {
+ prep_key = fscrypt_find_mode_key(mk, hkdf_context, mode_num, ci);
+ if (prep_key) {
ci->ci_enc_key = *prep_key;
return 0;
}
- mutex_lock(&fscrypt_mode_key_setup_mutex);
+ guard(mutex)(&fscrypt_mode_key_setup_mutex);
- if (fscrypt_is_key_prepared(prep_key, ci))
- goto done_unlock;
+ prep_key = fscrypt_find_mode_key(mk, hkdf_context, mode_num, ci);
+ if (prep_key) {
+ ci->ci_enc_key = *prep_key;
+ return 0;
+ }
+
+ new_node = kzalloc_obj(*new_node);
+ if (!new_node)
+ return -ENOMEM;
+ new_node->hkdf_context = hkdf_context;
+ new_node->mode_num = mode_num;
+ new_node->data_unit_bits = ci->ci_data_unit_bits;
+ prep_key = &new_node->key;
if (use_hw_wrapped_key) {
err = fscrypt_prepare_inline_crypt_key(prep_key,
mk->mk_secret.bytes,
mk->mk_secret.size, true,
ci);
- if (err)
- goto out_unlock;
- goto done_unlock;
+ } else {
+ static_assert(sizeof(mode_num) == 1);
+ static_assert(sizeof(sb->s_uuid) == 16);
+ static_assert(sizeof(hkdf_info) == 17);
+ hkdf_info[hkdf_infolen++] = mode_num;
+ if (include_fs_uuid) {
+ memcpy(&hkdf_info[hkdf_infolen], &sb->s_uuid,
+ sizeof(sb->s_uuid));
+ hkdf_infolen += sizeof(sb->s_uuid);
+ }
+ fscrypt_hkdf_expand(&mk->mk_secret.hkdf, hkdf_context,
+ hkdf_info, hkdf_infolen, raw_mode_key,
+ mode->keysize);
+ err = fscrypt_prepare_key(prep_key, raw_mode_key, ci);
+ memzero_explicit(raw_mode_key, mode->keysize);
}
-
- BUILD_BUG_ON(sizeof(mode_num) != 1);
- BUILD_BUG_ON(sizeof(sb->s_uuid) != 16);
- BUILD_BUG_ON(sizeof(hkdf_info) != 17);
- hkdf_info[hkdf_infolen++] = mode_num;
- if (include_fs_uuid) {
- memcpy(&hkdf_info[hkdf_infolen], &sb->s_uuid,
- sizeof(sb->s_uuid));
- hkdf_infolen += sizeof(sb->s_uuid);
+ if (err) {
+ kfree(new_node);
+ return err;
}
- fscrypt_hkdf_expand(&mk->mk_secret.hkdf, hkdf_context, hkdf_info,
- hkdf_infolen, mode_key, mode->keysize);
- err = fscrypt_prepare_key(prep_key, mode_key, ci);
- memzero_explicit(mode_key, mode->keysize);
- if (err)
- goto out_unlock;
-done_unlock:
+ list_add_tail_rcu(&new_node->link, &mk->mk_mode_keys);
ci->ci_enc_key = *prep_key;
- err = 0;
-out_unlock:
- mutex_unlock(&fscrypt_mode_key_setup_mutex);
- return err;
+ return 0;
}
/*
* Derive a SipHash key from the given fscrypt master key and the given
* application-specific information string.
@@ -309,12 +340,12 @@ void fscrypt_hash_inode_number(struct fscrypt_inode_info *ci,
static int fscrypt_setup_iv_ino_lblk_32_key(struct fscrypt_inode_info *ci,
struct fscrypt_master_key *mk)
{
int err;
- err = setup_per_mode_enc_key(ci, mk, mk->mk_iv_ino_lblk_32_keys,
- HKDF_CONTEXT_IV_INO_LBLK_32_KEY, true);
+ err = setup_per_mode_enc_key(ci, mk, HKDF_CONTEXT_IV_INO_LBLK_32_KEY,
+ true);
if (err)
return err;
/* pairs with smp_store_release() below */
if (!smp_load_acquire(&mk->mk_ino_hash_key_initialized)) {
@@ -362,23 +393,22 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_inode_info *ci,
* v1 policies, for v2 policies in this case we don't encrypt
* with the master key directly but rather derive a per-mode
* encryption key. This ensures that the master key is
* consistently used only for HKDF, avoiding key reuse issues.
*/
- err = setup_per_mode_enc_key(ci, mk, mk->mk_direct_keys,
- HKDF_CONTEXT_DIRECT_KEY, false);
+ err = setup_per_mode_enc_key(ci, mk, HKDF_CONTEXT_DIRECT_KEY,
+ false);
} else if (ci->ci_policy.v2.flags &
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64) {
/*
* IV_INO_LBLK_64: encryption keys are derived from (master_key,
* mode_num, filesystem_uuid), and inode number is included in
* the IVs. This format is optimized for use with inline
* encryption hardware compliant with the UFS standard.
*/
- err = setup_per_mode_enc_key(ci, mk, mk->mk_iv_ino_lblk_64_keys,
- HKDF_CONTEXT_IV_INO_LBLK_64_KEY,
- true);
+ err = setup_per_mode_enc_key(
+ ci, mk, HKDF_CONTEXT_IV_INO_LBLK_64_KEY, true);
} else if (ci->ci_policy.v2.flags &
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) {
err = fscrypt_setup_iv_ino_lblk_32_key(ci, mk);
} else {
u8 derived_key[FSCRYPT_MAX_RAW_KEY_SIZE];
base-commit: 83f1454877cc292b88baf13c829c16ce6937d120
--
2.54.0
^ permalink raw reply related
* Re: [GIT PULL] ext4 changes for 7.2-rc1
From: pr-tracker-bot @ 2026-06-18 17:04 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Linus Torvalds, Linux Kernel Developers List,
Ext4 Developers List
In-Reply-To: <ajPrqTd4FaxlpYPs@mit.edu>
The pull request you sent on Thu, 18 Jun 2026 09:00:01 -0400:
> https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git tags/ext4_for_linus-7.2-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/83f1454877cc292b88baf13c829c16ce6937d120
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2 6/8] ext4: return -EAGAIN from ext4_map_blocks() in NOWAIT cache miss
From: Baokun Li @ 2026-06-18 15:51 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, tytso, adilger.kernel, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <ekslfvokadqeiqmfbhv7d3v4ayguuqt7z53i5ze4u55fqkvkjg@waocl7jdgpja>
On 2026/6/18 22:09, Jan Kara wrote:
> On Thu 18-06-26 20:57:33, Baokun Li wrote:
>> Make ext4_map_blocks() return -EAGAIN instead of 0 when
>> EXT4_GET_BLOCKS_CACHED_NOWAIT is set and the extent status cache
>> misses. This allows callers to easily distinguish between a successful
>> cache lookup (positive return value) and a cache miss requiring disk
>> access (-EAGAIN), simplifying error handling in NOWAIT paths.
>>
>> The change affects two locations:
>> 1. After cache hit: return retval ? retval : -EAGAIN
>> (return -EAGAIN if cache hit is hole/delayed)
> Are you sure about this case? -EAGAIN looks wrong here - we have the valid
> information cached and provide it to the caller without blocking. So at
> least from the POV of ext4_map_blocks() there's no reason to return -EAGAIN.
>
> Honza
You're right, there's no need to return -EAGAIN here. I only considered
the write path - even without returning -EAGAIN, ext4_iomap_alloc()
would return it anyway. But I missed the read path, where this would cause
an unnecessary retry.
I'll remove this change in the next version and only keep the essential
second modification (for cache miss case).
Thanks for the review!
Cheers,
Baokun
>> 2. After cache miss: return -EAGAIN
>> (instead of 0, indicating need for disk lookup)
>>
>> The only existing caller using EXT4_GET_BLOCKS_CACHED_NOWAIT is the
>> ext4_get_link() -> ext4_getblk() path. Although ext4_getblk() now
>> takes a different return branch (err < 0 instead of err == 0) and
>> propagates -EAGAIN instead of NULL, ext4_get_link() converts both
>> cases to -ECHILD via IS_ERR_OR_NULL(), so the final error seen by
>> the VFS remains unchanged.
>>
>> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
>> ---
>> fs/ext4/inode.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 832794294ccf..03adbca3ec78 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -760,7 +760,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>> }
>>
>> if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
>> - return retval;
>> + return retval ? retval : -EAGAIN;
>> +
>> #ifdef ES_AGGRESSIVE_TEST
>> ext4_map_blocks_es_recheck(handle, inode, map,
>> &orig_map, flags);
>> @@ -776,7 +777,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>> * cannot find extent in the cache.
>> */
>> if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
>> - return 0;
>> + return -EAGAIN;
>>
>> /*
>> * Try to see if we can get the block without requesting a new
>> --
>> 2.43.7
>>
^ permalink raw reply
* Re: [PATCH v3] ext4: drop s_writepages_rwsem around inline data handling in writepages
From: Zhou, Yun @ 2026-06-18 14:52 UTC (permalink / raw)
To: Jan Kara
Cc: tytso, adilger.kernel, libaokun, ojaswin, ritesh.list, yi.zhang,
ebiggers, linux-ext4, linux-kernel
In-Reply-To: <rvwzagttciesrgonspk37dm4sxkxxgd7marnwtz5c6cpag747e@wvnqduid6hv7>
On 6/18/2026 10:23 PM, Jan Kara wrote:
>
> On Mon 15-06-26 14:10:15, Yun Zhou wrote:
>
> You have fixed this differently (not expanding extra isize from
> ext4_evict_inode()) and furthermore this scenario is really impossible
> because you cannot be inside ext4_writepages() on inode that's undergoing
> eviction. SO let's discard this patch.
>
Yes, that patch series can resolves all the deadlock risks associated with
calling iput(ea_inode) while holding a jbd2 handle—something I hadn't
even considered at first. I really owe this to your suggestions.
BR,
Yun
^ permalink raw reply
* Re: [PATCH v3] ext4: drop s_writepages_rwsem around inline data handling in writepages
From: Jan Kara @ 2026-06-18 14:23 UTC (permalink / raw)
To: Yun Zhou
Cc: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
yi.zhang, ebiggers, linux-ext4, linux-kernel
In-Reply-To: <20260615061015.1523668-1-yun.zhou@windriver.com>
On Mon 15-06-26 14:10:15, Yun Zhou wrote:
> ext4_do_writepages() calls ext4_destroy_inline_data() which acquires
> xattr_sem while s_writepages_rwsem is held (read). This creates a
> circular lock dependency:
>
> CPU0 CPU1
> ---- ----
> ext4_writepages()
> ext4_writepages_down_read()
> [holds s_writepages_rwsem]
> ext4_evict_inode()
> __ext4_mark_inode_dirty()
> ext4_expand_extra_isize_ea()
> ext4_xattr_block_set()
> [holds xattr_sem]
> iput(old_bh inode)
> write_inode_now()
> ext4_writepages()
> ext4_writepages_down_read()
> [BLOCKED on s_writepages_rwsem]
> ext4_do_writepages()
> ext4_destroy_inline_data()
> down_write(xattr_sem)
> [BLOCKED on xattr_sem]
You have fixed this differently (not expanding extra isize from
ext4_evict_inode()) and furthermore this scenario is really impossible
because you cannot be inside ext4_writepages() on inode that's undergoing
eviction. SO let's discard this patch.
Honza
>
> Fix by temporarily dropping s_writepages_rwsem for the entire inline
> data handling block, including the journal handle start/stop. The
> rwsem must be dropped before ext4_journal_start() -- not between
> journal_start and journal_stop -- to avoid a secondary deadlock with
> ext4_change_inode_journal_flag() which takes rwsem (write) and then
> calls jbd2_journal_lock_updates() waiting for active handles to stop.
>
> This is safe because:
>
> - This code runs before any block mapping or IO submission, so no
> writepages state depends on the rwsem being held at this point.
>
> - Inline data destruction is a one-way format transition (once cleared,
> EXT4_INODE_INLINE_DATA is never set again). The rwsem is
> re-acquired after journal_stop, ensuring format stability for the
> remainder of writepages.
>
> - The can_map flag identifies the ext4_writepages() path (holds rwsem)
> vs ext4_normal_submit_inode_data_buffers() (does not), so the
> drop/reacquire is skipped when the rwsem is not held.
>
> Also check the return value of ext4_destroy_inline_data() to avoid
> proceeding with an inconsistent inode format on failure.
>
> Reported-by: syzbot+bb2455d02bda0b5701e3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=bb2455d02bda0b5701e3
> Fixes: c8585c6fcaf2 ("ext4: fix races between changing inode journal mode and ext4_writepages")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> ---
> v3: Drop s_writepages_rwsem before ext4_journal_start() and reacquire
> after ext4_journal_stop(), instead of dropping between journal_start
> and journal_stop as in v2. This avoids two issues identified in v2
> review:
> - memalloc_nofs_restore() in ext4_writepages_up_read() would clear
> PF_MEMALLOC_NOFS while the jbd2 handle is active.
> - Reacquiring s_writepages_rwsem while holding a handle creates an
> ABBA deadlock with ext4_change_inode_journal_flag() which takes
> the rwsem (write) then calls jbd2_journal_lock_updates().
>
> v2: Instead of moving inline data handling to ext4_writepages(),
> temporarily drop s_writepages_rwsem around ext4_destroy_inline_data()
> in ext4_do_writepages(). The move approach had a race where concurrent
> writes could create dirty pages with inline data after the early check,
> and unconditional destruction without dirty pages would lose data.
>
> v1: Moved inline data cleanup from ext4_do_writepages() to
> ext4_writepages() before acquiring s_writepages_rwsem.
>
> fs/ext4/inode.c | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c2c2d6ac7f3d..cd7588a3fa45 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1694,6 +1694,9 @@ struct mpage_da_data {
> struct writeback_control *wbc;
> unsigned int can_map:1; /* Can writepages call map blocks? */
>
> + /* Saved memalloc context from ext4_writepages_down_read() */
> + int alloc_ctx;
> +
> /* These are internal state of ext4_do_writepages() */
> loff_t start_pos; /* The start pos to write */
> loff_t next_pos; /* Current pos to examine */
> @@ -2816,16 +2819,35 @@ static int ext4_do_writepages(struct mpage_da_data *mpd)
> * we'd better clear the inline data here.
> */
> if (ext4_has_inline_data(inode)) {
> - /* Just inode will be modified... */
> + /*
> + * Temporarily drop s_writepages_rwsem because
> + * ext4_destroy_inline_data() acquires xattr_sem, which has
> + * a higher lock ordering rank. Holding both would create a
> + * circular dependency with ext4_xattr_block_set() -> iput()
> + * -> ext4_writepages() -> s_writepages_rwsem.
> + *
> + * Drop the rwsem before starting the journal handle to also
> + * avoid a deadlock with ext4_change_inode_journal_flag(),
> + * which takes rwsem (write) then jbd2_journal_lock_updates().
> + */
> + if (mpd->can_map)
> + ext4_writepages_up_read(inode->i_sb, mpd->alloc_ctx);
> handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> if (IS_ERR(handle)) {
> + if (mpd->can_map)
> + mpd->alloc_ctx =
> + ext4_writepages_down_read(inode->i_sb);
> ret = PTR_ERR(handle);
> goto out_writepages;
> }
> BUG_ON(ext4_test_inode_state(inode,
> EXT4_STATE_MAY_INLINE_DATA));
> - ext4_destroy_inline_data(handle, inode);
> + ret = ext4_destroy_inline_data(handle, inode);
> ext4_journal_stop(handle);
> + if (mpd->can_map)
> + mpd->alloc_ctx = ext4_writepages_down_read(inode->i_sb);
> + if (ret)
> + goto out_writepages;
> }
>
> /*
> @@ -3032,13 +3054,12 @@ static int ext4_writepages(struct address_space *mapping,
> .can_map = 1,
> };
> int ret;
> - int alloc_ctx;
>
> ret = ext4_emergency_state(sb);
> if (unlikely(ret))
> return ret;
>
> - alloc_ctx = ext4_writepages_down_read(sb);
> + mpd.alloc_ctx = ext4_writepages_down_read(sb);
> ret = ext4_do_writepages(&mpd);
> /*
> * For data=journal writeback we could have come across pages marked
> @@ -3047,7 +3068,7 @@ static int ext4_writepages(struct address_space *mapping,
> */
> if (!ret && mpd.journalled_more_data)
> ret = ext4_do_writepages(&mpd);
> - ext4_writepages_up_read(sb, alloc_ctx);
> + ext4_writepages_up_read(sb, mpd.alloc_ctx);
>
> return ret;
> }
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 8/8] ext4: handle IOCB_NOWAIT in ext4_dio_needs_zeroing() with cache-only lookup
From: Jan Kara @ 2026-06-18 14:10 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, peng_wang
In-Reply-To: <20260618125735.4156639-9-libaokun@linux.alibaba.com>
On Thu 18-06-26 20:57:35, Baokun Li wrote:
> Add a nowait parameter to ext4_dio_needs_zeroing() and pass
> EXT4_GET_BLOCKS_CACHED_NOWAIT flag to ext4_map_blocks() when
> IOCB_NOWAIT is set. This ensures the needs_zeroing check only uses
> cached extent info. If cache misses, ext4_map_blocks() returns
> -EAGAIN, causing ext4_dio_needs_zeroing() to conservatively return
> true (needs zeroing). The caller then tries to upgrade to exclusive
> lock, which returns -EAGAIN for NOWAIT, avoiding potential sleep on
> down_read(i_data_sem).
>
> The caller in ext4_dio_write_checks() is updated to pass the
> IOCB_NOWAIT flag from the kiocb.
>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/file.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 5ffc1afd8050..44d1658d2b5a 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -228,7 +228,8 @@ ext4_extending_io(struct inode *inode, loff_t offset, size_t len)
> * unwritten conversion for middle blocks are protected by i_data_sem
> * and inode_dio_begin().
> */
> -static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
> +static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len,
> + bool nowait)
> {
> struct ext4_map_blocks map;
> unsigned int blkbits = inode->i_blkbits;
> @@ -236,10 +237,14 @@ static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
> bool head_partial, tail_partial;
> ext4_lblk_t head_lblk, tail_lblk;
> int err;
> + int map_flags = 0;
>
> if (pos + len > i_size_read(inode))
> return true;
>
> + if (nowait)
> + map_flags = EXT4_GET_BLOCKS_CACHED_NOWAIT;
> +
> head_partial = (pos & blockmask) != 0;
> tail_partial = ((pos + len) & blockmask) != 0;
> head_lblk = pos >> blkbits;
> @@ -249,7 +254,7 @@ static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
> if (head_partial) {
> map.m_lblk = head_lblk;
> map.m_len = tail_lblk - head_lblk + 1;
> - err = ext4_map_blocks(NULL, inode, &map, 0);
> + err = ext4_map_blocks(NULL, inode, &map, map_flags);
> if (err <= 0 || !(map.m_flags & EXT4_MAP_MAPPED))
> return true;
> /* If this mapping already covers the tail block, we're done. */
> @@ -261,7 +266,7 @@ static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
> if (tail_partial) {
> map.m_lblk = tail_lblk;
> map.m_len = 1;
> - err = ext4_map_blocks(NULL, inode, &map, 0);
> + err = ext4_map_blocks(NULL, inode, &map, map_flags);
> if (err <= 0 || !(map.m_flags & EXT4_MAP_MAPPED))
> return true;
> }
> @@ -516,7 +521,8 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
> * under shared lock is safe.
> */
> if (ext4_unaligned_io(inode, from, offset))
> - needs_zeroing = ext4_dio_needs_zeroing(inode, offset, count);
> + needs_zeroing = ext4_dio_needs_zeroing(inode, offset, count,
> + iocb->ki_flags & IOCB_NOWAIT);
>
> /* Determine whether we need to upgrade to an exclusive lock. */
> if (*ilock_shared &&
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 7/8] ext4: handle IOMAP_NOWAIT in ext4_iomap_begin() with cache-only lookup
From: Jan Kara @ 2026-06-18 14:09 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, peng_wang
In-Reply-To: <20260618125735.4156639-8-libaokun@linux.alibaba.com>
On Thu 18-06-26 20:57:34, Baokun Li wrote:
> Pass EXT4_GET_BLOCKS_CACHED_NOWAIT flag to ext4_map_blocks() when
> IOMAP_NOWAIT is set, ensuring that extent lookups only use the cached
> extent status tree. If the cache misses, ext4_map_blocks() returns
> -EAGAIN instead of sleeping on down_read(i_data_sem) to read extent
> tree from disk.
>
> This applies to both write and read paths in ext4_iomap_begin(),
> allowing DIO/DAX operations with RWF_NOWAIT to avoid blocking on
> extent tree lookups.
>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 03adbca3ec78..09f85cd6c118 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3781,6 +3781,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> struct ext4_map_blocks map;
> u8 blkbits = inode->i_blkbits;
> unsigned int orig_mlen;
> + int map_flags = 0;
>
> if ((offset >> blkbits) > EXT4_MAX_LOGICAL_BLOCK)
> return -EINVAL;
> @@ -3795,6 +3796,12 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> map.m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
> EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
> orig_mlen = map.m_len;
> + /*
> + * In NOWAIT context, only use cached extent info. If es cache misses,
> + * return -EAGAIN to avoid sleeping on down_read(i_data_sem).
> + */
> + if (flags & IOMAP_NOWAIT)
> + map_flags = EXT4_GET_BLOCKS_CACHED_NOWAIT;
>
> if (flags & IOMAP_WRITE) {
> /*
> @@ -3804,7 +3811,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> * especially in multi-threaded overwrite requests.
> */
> if (offset + length <= i_size_read(inode)) {
> - ret = ext4_map_blocks(NULL, inode, &map, 0);
> + ret = ext4_map_blocks(NULL, inode, &map, map_flags);
> /*
> * For DAX we convert extents to initialized ones before
> * copying the data, otherwise we do it after I/O so
> @@ -3825,7 +3832,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> }
> ret = ext4_iomap_alloc(inode, &map, flags);
> } else {
> - ret = ext4_map_blocks(NULL, inode, &map, 0);
> + ret = ext4_map_blocks(NULL, inode, &map, map_flags);
> }
>
> if (ret < 0)
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 6/8] ext4: return -EAGAIN from ext4_map_blocks() in NOWAIT cache miss
From: Jan Kara @ 2026-06-18 14:09 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, peng_wang
In-Reply-To: <20260618125735.4156639-7-libaokun@linux.alibaba.com>
On Thu 18-06-26 20:57:33, Baokun Li wrote:
> Make ext4_map_blocks() return -EAGAIN instead of 0 when
> EXT4_GET_BLOCKS_CACHED_NOWAIT is set and the extent status cache
> misses. This allows callers to easily distinguish between a successful
> cache lookup (positive return value) and a cache miss requiring disk
> access (-EAGAIN), simplifying error handling in NOWAIT paths.
>
> The change affects two locations:
> 1. After cache hit: return retval ? retval : -EAGAIN
> (return -EAGAIN if cache hit is hole/delayed)
Are you sure about this case? -EAGAIN looks wrong here - we have the valid
information cached and provide it to the caller without blocking. So at
least from the POV of ext4_map_blocks() there's no reason to return -EAGAIN.
Honza
> 2. After cache miss: return -EAGAIN
> (instead of 0, indicating need for disk lookup)
>
> The only existing caller using EXT4_GET_BLOCKS_CACHED_NOWAIT is the
> ext4_get_link() -> ext4_getblk() path. Although ext4_getblk() now
> takes a different return branch (err < 0 instead of err == 0) and
> propagates -EAGAIN instead of NULL, ext4_get_link() converts both
> cases to -ECHILD via IS_ERR_OR_NULL(), so the final error seen by
> the VFS remains unchanged.
>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
> ---
> fs/ext4/inode.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 832794294ccf..03adbca3ec78 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -760,7 +760,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> }
>
> if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
> - return retval;
> + return retval ? retval : -EAGAIN;
> +
> #ifdef ES_AGGRESSIVE_TEST
> ext4_map_blocks_es_recheck(handle, inode, map,
> &orig_map, flags);
> @@ -776,7 +777,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> * cannot find extent in the cache.
> */
> if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
> - return 0;
> + return -EAGAIN;
>
> /*
> * Try to see if we can get the block without requesting a new
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 5/8] ext4: use kiocb_modified instead of file_modified in DIO/DAX write path
From: Jan Kara @ 2026-06-18 13:56 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, peng_wang
In-Reply-To: <20260618125735.4156639-6-libaokun@linux.alibaba.com>
On Thu 18-06-26 20:57:32, Baokun Li wrote:
> file_modified() passes flags=0 which drops IOCB_NOWAIT, causing
> file_update_time() to sleep in ext4_journal_start() via
> ext4_dirty_inode() even in non-blocking contexts.
>
> kiocb_modified(iocb) propagates iocb->ki_flags so that
> generic_update_time() correctly returns -EAGAIN when IOCB_NOWAIT
> is set and ->dirty_inode could block, matching the behavior
> already adopted by XFS, FUSE, and ext2.
>
> Affected paths:
> - ext4_dio_write_checks(): DIO NOWAIT write
> - ext4_write_checks(): shared by buffered (rejects NOWAIT upfront)
> and DAX write (supports NOWAIT)
>
> ext4_fallocate() in extents.c is not affected as it has no kiocb.
>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Indeed, good catch! Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/file.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 2681f148e7b8..5ffc1afd8050 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -307,7 +307,7 @@ static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
> if (count <= 0)
> return count;
>
> - ret = file_modified(iocb->ki_filp);
> + ret = kiocb_modified(iocb);
> if (ret)
> return ret;
>
> @@ -465,7 +465,7 @@ static const struct iomap_dio_ops ext4_dio_write_ops = {
> *
> * The decision is layered, evaluated in this order:
> *
> - * 1. If file_modified() needs to update security info (!IS_NOSEC), upgrade
> + * 1. If kiocb_modified() needs to update security info (!IS_NOSEC), upgrade
> * to the exclusive lock -- the security update itself requires it,
> * regardless of whether the write extends the file or is aligned.
> *
> @@ -555,7 +555,7 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
> *dio_flags = IOMAP_DIO_FORCE_WAIT;
> }
>
> - ret = file_modified(file);
> + ret = kiocb_modified(iocb);
> if (ret < 0)
> goto out;
>
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 2/8] ext4: drain in-flight DIO before buffered write fallback
From: Jan Kara @ 2026-06-18 13:54 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, peng_wang
In-Reply-To: <20260618125735.4156639-3-libaokun@linux.alibaba.com>
On Thu 18-06-26 20:57:29, Baokun Li wrote:
> generic/746 started failing intermittently on ext3 (no-extent inodes).
> The test triggers 'Page cache invalidation failure on direct I/O'
> warnings and subsequent fsync returns -EIO. Adding a 50ms delay
> between ext4_buffered_write_iter() and filemap_write_and_wait_range()
> in ext4_dio_write_iter() makes the race almost always reproducible.
>
> On no-extent inodes, DIO writes to holes cannot use unwritten extents,
> so ext4_iomap_alloc() leaves m_flags=0 and ext4_map_blocks() returns 0.
> The iomap layer then returns -ENOTBLK, causing fallback to buffered I/O.
>
> The fallback path in ext4_dio_write_iter() calls
> ext4_buffered_write_iter() which dirties pages, then does flush and
> invalidate. However, there's an unprotected window between
> ext4_buffered_write_iter() returning (with inode lock released) and
> the subsequent flush+invalidate.
>
> Concurrent async DIO completions from other threads can run
> kiocb_invalidate_post_direct_write() during this window. If pages have
> been re-dirtied, post-invalidation finds dirty pages and triggers the
> warning, setting -EIO in the error sequence.
>
> Consider a file with two 4k extents: [hole][written]. Thread A does
> DIO to the written extent, while thread B does DIO spanning both:
>
> kworker A (4k DIO, allocated block) kworker B (8k DIO, 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: lock released */
> ext4_buffered_write_iter()
> inode_lock(exclusive)
> generic_perform_write()
> -> dirty pages [0, 8k]
> inode_unlock(exclusive)
>
> /* pages dirty, no lock */
> [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!
> -> dio_warn_stale_pagecache()
> -> errseq_set(-EIO)
>
> This issue can be triggered through normal I/O paths, not just
> intentionally overlapping DIO writes from userspace. For example,
> generic/746 uses a loop device where multiple kworkers issue concurrent
> I/O to the backing file. Additionally, when block_size < folio_size,
> non-overlapping DIO writes that share a large folio can also trigger
> the race.
>
> Add inode_dio_wait() in ext4_buffered_write_iter() before
> generic_perform_write() to drain all in-flight DIO. This ensures
> that all DIO clears existing pages before submitting IO (via
> kiocb_invalidate_pages()), and all BIO waits for all DIO to
> complete (via inode_dio_wait()), thus eliminating the race.
>
> Fixes: 378f32bab371 ("ext4: introduce direct I/O write using iomap infrastructure")
> Suggested-by: Zhang Yi <yi.zhang@huawei.com>
> Link: https://patch.msgid.link/d1adcf7c-c276-458d-9cac-68a4410f7626@gmail.com
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/file.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index eb1a323962b1..9f9bc0b13772 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -313,6 +313,12 @@ static ssize_t ext4_buffered_write_iter(struct kiocb *iocb,
> if (ret <= 0)
> goto out;
>
> + /*
> + * Prevent concurrent DIO and BIO to the same file range.
> + * Wait for all in-flight DIO to complete before dirtying pages.
> + */
> + inode_dio_wait(inode);
> +
> ret = generic_perform_write(iocb, from);
>
> out:
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v2 1/8] ext4: prevent sleeping allocation in NOWAIT write path
From: Jan Kara @ 2026-06-18 13:52 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, yi.zhang, ojaswin,
ritesh.list, peng_wang, Sashiko
In-Reply-To: <20260618125735.4156639-2-libaokun@linux.alibaba.com>
On Thu 18-06-26 20:57:28, Baokun Li wrote:
> Block allocation requires journal access which may sleep, violating
> NOWAIT semantics. Return -EAGAIN early when IOMAP_NOWAIT is set,
> allowing the caller to retry without the NOWAIT constraint.
>
> This ensures that write paths using IOMAP_NOWAIT (e.g., DIO with
> RWF_NOWAIT) will not block on journal operations when blocks need
> to be allocated.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260611163441.2431805-1-libaokun@linux.alibaba.com?part=1
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c2c2d6ac7f3d..832794294ccf 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3672,6 +3672,9 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
> int ret, dio_credits, m_flags = 0, retries = 0;
> bool force_commit = false;
>
> + if (flags & IOMAP_NOWAIT)
> + return -EAGAIN;
> +
> /*
> * Trim the mapping request to the maximum value that we can map at
> * once for direct I/O.
> --
> 2.43.7
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 18/23] ext4: wait for ordered I/O in the iomap buffered I/O path
From: Jan Kara @ 2026-06-18 13:48 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-19-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:38, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> For append writes, wait for ordered I/O to complete before updating
> i_disksize. This ensures that zeroed data is flushed to disk before the
> metadata update, preventing stale data from being exposed during
> unaligned post-EOF append writes.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Frankly, this all looks too complex to me. Plus your are adding 32-bytes to
struct ext4_inode_info which isn't great either. Why don't you just do
filemap_fdatawait() for the byte at old i_disksize and be done with it?
I believe we have to simplify this. All this complexity (and thus
maintenance burden) across several patches for the corner case of zeroing
tail block on extention is in my opinion difficult to justify.
Honza
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 078feda47e36..9ce2128eea3e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1195,6 +1195,15 @@ struct ext4_inode_info {
> #ifdef CONFIG_FS_ENCRYPTION
> struct fscrypt_inode_info *i_crypt_info;
> #endif
> +
> + /*
> + * Track ordered zeroed data during post-EOF append writes, fallocate,
> + * and truncate-up operations. These parameters are used only in the
> + * iomap buffered I/O path.
> + */
> + ext4_lblk_t i_ordered_lblk;
> + ext4_lblk_t i_ordered_len;
> + wait_queue_head_t i_ordered_wq;
> };
>
> /*
> @@ -3858,6 +3867,8 @@ extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
> __u64 len, __u64 *moved_len);
>
> /* page-io.c */
> +#define EXT4_IOMAP_IOEND_ORDER_IO 1UL /* This I/O is an ordered one */
> +
> extern int __init ext4_init_pageio(void);
> extern void ext4_exit_pageio(void);
> extern ext4_io_end_t *ext4_init_io_end(struct inode *inode, gfp_t flags);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e013aeb03d7b..11fb369efeb1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4345,6 +4345,7 @@ static int ext4_iomap_writeback_submit(struct iomap_writepage_ctx *wpc,
> {
> struct iomap_ioend *ioend = wpc->wb_ctx;
> struct ext4_inode_info *ei = EXT4_I(ioend->io_inode);
> + ext4_lblk_t start, end, order_lblk, order_len;
>
> /*
> * After I/O completion, a worker needs to be scheduled when:
> @@ -4357,6 +4358,30 @@ static int ext4_iomap_writeback_submit(struct iomap_writepage_ctx *wpc,
> test_opt(ioend->io_inode->i_sb, DATA_ERR_ABORT))
> ioend->io_bio.bi_end_io = ext4_iomap_end_bio;
>
> + /*
> + * Mark the I/O as ordered. Ordered I/O requires separate endio
> + * handling and must not be merged with regular I/O operations.
> + */
> + order_len = READ_ONCE(ei->i_ordered_len);
> + if (order_len) {
> + /*
> + * Pair with smp_store_release() in ext4_block_zero_eof().
> + * Ensure we see the updated i_ordered_lblk that was written
> + * before the release store to i_ordered_len.
> + */
> + smp_rmb();
> + order_lblk = READ_ONCE(ei->i_ordered_lblk);
> + start = ioend->io_offset >> ioend->io_inode->i_blkbits;
> + end = EXT4_B_TO_LBLK(ioend->io_inode,
> + ioend->io_offset + ioend->io_size);
> +
> + if (start <= order_lblk && end >= order_lblk + order_len) {
> + ioend->io_bio.bi_end_io = ext4_iomap_end_bio;
> + ioend->io_private = (void *)EXT4_IOMAP_IOEND_ORDER_IO;
> + ioend->io_flags |= IOMAP_IOEND_BOUNDARY;
> + }
> + }
> +
> return iomap_ioend_writeback_submit(wpc, error);
> }
>
> @@ -4746,8 +4771,10 @@ static int ext4_iomap_submit_zero_block(struct inode *inode,
> loff_t from, loff_t end)
> {
> struct address_space *mapping = inode->i_mapping;
> + struct ext4_inode_info *ei = EXT4_I(inode);
> struct folio *folio;
> bool do_submit = false;
> + int ret;
>
> folio = filemap_lock_folio(mapping, from >> PAGE_SHIFT);
> if (IS_ERR(folio))
> @@ -4757,14 +4784,50 @@ static int ext4_iomap_submit_zero_block(struct inode *inode,
> folio_wait_writeback(folio);
> WARN_ON_ONCE(folio_test_writeback(folio));
>
> - if (likely(folio_test_dirty(folio)))
> + /*
> + * Mark the ordered range. It will be cleared upon I/O completion
> + * in ext4_iomap_end_bio(). Any operation that extends i_disksize
> + * (including append write end io past the zeroed boundary,
> + * truncate up and append fallocate) must wait for this I/O to
> + * complete before updating i_disksize.
> + *
> + * When multiple overlapping unaligned EOF writes are in flight, we
> + * only need to track and wait for the first one. Subsequent writes
> + * will zero the gap in memory and ensure that the zeroed data is
> + * written out along with the valid data in the same block before
> + * i_disksize is updated.
> + */
> + if (likely(folio_test_dirty(folio) &&
> + READ_ONCE(ei->i_ordered_len) == 0)) {
> + WRITE_ONCE(ei->i_ordered_lblk,
> + from >> inode->i_blkbits);
> + /*
> + * Pairs with smp_rmb() in ext4_iomap_writeback_submit()
> + * and ext4_iomap_wb_ordered_wait(). Ensure the updated
> + * i_ordered_lblk is visible when i_ordered_len becomes
> + * non-zero.
> + */
> + smp_store_release(&ei->i_ordered_len, 1);
> do_submit = true;
> + }
> folio_unlock(folio);
> folio_put(folio);
>
> /* Submit zeroed block. */
> - if (do_submit)
> - return filemap_fdatawrite_range(mapping, from, end - 1);
> + if (do_submit) {
> + ret = filemap_fdatawrite_range(mapping, from, end - 1);
> + if (ret) {
> + /*
> + * Pairs with wait_event() in
> + * ext4_iomap_wb_ordered_wait(). Ensure
> + * i_ordered_len = 0 is visible before waking up
> + * waiters.
> + */
> + smp_store_release(&ei->i_ordered_len, 0);
> + wake_up_all(&ei->i_ordered_wq);
> + return ret;
> + }
> + }
> return 0;
> }
>
> @@ -4827,10 +4890,13 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> * data=ordered mode. We submit zeroed range directly here.
> * Do not wait for I/O completion for performance.
> *
> - * TODO: Any operation that extends i_disksize (including
> - * append write end io past the zeroed boundary, truncate up,
> - * and append fallocate) must wait for the relevant I/O to
> - * complete before updating i_disksize.
> + * The end_io handler ext4_iomap_wb_ordered_wait() will wait
> + * for I/O completion before updating i_disksize if the write
> + * extends beyond the zeroed boundary.
> + *
> + * TODO: Any other operation that extends i_disksize
> + * (including truncate up and append fallocate) must wait for
> + * the relevant I/O to complete before updating i_disksize.
> */
> } else if (ext4_inode_buffered_iomap(inode)) {
> err = ext4_iomap_submit_zero_block(inode, from, end);
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 3050c887329f..ad05ebb49bf6 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -613,6 +613,46 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio,
> return 0;
> }
>
> +/*
> + * If the old disk size is not block size aligned and the current
> + * writeback range is entirely beyond the old EOF block, we should
> + * wait for the zeroed data written in ext4_block_zero_eof() to be
> + * written out, otherwise, it may expose stale data in that block.
> + */
> +static void ext4_iomap_wb_ordered_wait(struct inode *inode,
> + loff_t pos, loff_t end)
> +{
> + struct ext4_inode_info *ei = EXT4_I(inode);
> + unsigned int blocksize = i_blocksize(inode);
> + loff_t disksize = READ_ONCE(ei->i_disksize);
> + ext4_lblk_t order_lblk, order_len;
> +
> + /*
> + * Waiting for ordered I/O is unnecessary when:
> + * - The on-disk size is block-aligned (no stale data exists).
> + * - The write start is within the block of the old EOF
> + * (overwriting, or appending to a block that already contains
> + * valid data).
> + */
> + if (!(disksize & (blocksize - 1)) ||
> + pos < round_up(disksize, blocksize))
> + return;
> +
> + order_len = READ_ONCE(ei->i_ordered_len);
> + if (!order_len)
> + return;
> +
> + /*
> + * Pair with smp_store_release() in ext4_iomap_end_bio() and
> + * ext4_block_zero_eof(). Ensure we see the updated i_ordered_lblk
> + * that was written before the release store to i_ordered_len.
> + */
> + smp_rmb();
> + order_lblk = READ_ONCE(ei->i_ordered_lblk);
> + if ((pos >> inode->i_blkbits) >= order_lblk + order_len)
> + wait_event(ei->i_ordered_wq, READ_ONCE(ei->i_ordered_len) == 0);
> +}
> +
> static int ext4_iomap_wb_update_disksize(handle_t *handle, struct inode *inode,
> loff_t end)
> {
> @@ -656,6 +696,9 @@ static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
> goto out;
> }
>
> + /* Wait ordered zero data to be written out. */
> + ext4_iomap_wb_ordered_wait(inode, pos, pos + size);
> +
> /* We may need to convert one extent and dirty the inode. */
> credits = ext4_chunk_trans_blocks(inode,
> EXT4_MAX_BLOCKS(size, pos, inode->i_blkbits));
> @@ -717,8 +760,25 @@ void ext4_iomap_end_bio(struct bio *bio)
> struct inode *inode = ioend->io_inode;
> struct ext4_inode_info *ei = EXT4_I(inode);
> struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> + unsigned long io_mode = (unsigned long)ioend->io_private;
> unsigned long flags;
>
> + /*
> + * This is an ordered I/O, clear the ordered range set in
> + * ext4_block_zero_eof() and wake up all waiters that will update
> + * the inode i_disksize.
> + */
> + if (io_mode == EXT4_IOMAP_IOEND_ORDER_IO) {
> + /*
> + * Pairs with wait_event() in ext4_iomap_wb_ordered_wait().
> + * Ensure i_ordered_len = 0 is visible before waking up
> + * waiters.
> + */
> + smp_store_release(&ei->i_ordered_len, 0);
> + wake_up_all(&ei->i_ordered_wq);
> + goto defer;
> + }
> +
> /* Needs to convert unwritten extents or update the i_disksize. */
> if ((ioend->io_flags & IOMAP_IOEND_UNWRITTEN) ||
> ioend->io_offset + ioend->io_size > READ_ONCE(ei->i_disksize))
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 62bfe05a64bc..9c0a00e716f3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1444,6 +1444,9 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
> ext4_fc_init_inode(&ei->vfs_inode);
> spin_lock_init(&ei->i_fc_lock);
> mmb_init(&ei->i_metadata_bhs, &ei->vfs_inode.i_data);
> + ei->i_ordered_lblk = 0;
> + ei->i_ordered_len = 0;
> + init_waitqueue_head(&ei->i_ordered_wq);
> return &ei->vfs_inode;
> }
>
> @@ -1480,12 +1483,20 @@ static void ext4_destroy_inode(struct inode *inode)
> dump_stack();
> }
>
> - if (!(EXT4_SB(inode->i_sb)->s_mount_state & EXT4_ERROR_FS) &&
> - WARN_ON_ONCE(EXT4_I(inode)->i_reserved_data_blocks))
> - ext4_msg(inode->i_sb, KERN_ERR,
> - "Inode %llu (%p): i_reserved_data_blocks (%u) not cleared!",
> - inode->i_ino, EXT4_I(inode),
> - EXT4_I(inode)->i_reserved_data_blocks);
> + if (!(EXT4_SB(inode->i_sb)->s_mount_state & EXT4_ERROR_FS)) {
> + if (WARN_ON_ONCE(EXT4_I(inode)->i_reserved_data_blocks))
> + ext4_msg(inode->i_sb, KERN_ERR,
> + "Inode %llu (%p): i_reserved_data_blocks (%u) not cleared!",
> + inode->i_ino, EXT4_I(inode),
> + EXT4_I(inode)->i_reserved_data_blocks);
> +
> + if (WARN_ON_ONCE(EXT4_I(inode)->i_ordered_len))
> + ext4_msg(inode->i_sb, KERN_ERR,
> + "Inode %llu (%p): i_ordered_lblk (%u) and i_ordered_len (%u) not cleared!",
> + inode->i_ino, EXT4_I(inode),
> + EXT4_I(inode)->i_ordered_lblk,
> + EXT4_I(inode)->i_ordered_len);
> + }
> }
>
> static void ext4_shutdown(struct super_block *sb)
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [GIT PULL] ext4 changes for 7.2-rc1
From: Theodore Ts'o @ 2026-06-18 13:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Kernel Developers List, Ext4 Developers List
The following changes since commit 5200f5f493f79f14bbdc349e402a40dfb32f23c8:
Linux 7.1-rc4 (2026-05-17 13:59:58 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git tags/ext4_for_linus-7.2-rc1
for you to fetch changes up to c143957520c6c9b5cd72e0de8b52b814f0c576fe:
ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT (2026-06-10 10:53:50 -0400)
----------------------------------------------------------------
Various ext4 updates for 7.2-rc1:
* A major rework of the fast commit mechanism to avoid lock
contention and deadlocks. We also export snapshot statistics
in /proc/fs/ext4/*/fc_info.
* Performance optimization for directory hash computation by
processing input in 4-byte chunks and removing function pointers,
along with new KUnit tests for directory hash.
* Cleanups in JBD2 to remove special slabs and use kmalloc() instead.
* Various bug fixes, including:
- Early validation of donor superblock in EXT4_IOC_MOVE_EXT to avoid
cross-fs deadlock
- Fix for a kernel BUG in ext4_write_inline_data_end under
data=journal
- Fix for a NULL dereference in jbd2_journal_dirty_metadata when
handle is aborted
- Fix for an underflow in JBD2 fast commit block initialization check
- Fix for LOGFLUSH shutdown ordering to ensure ordered data writeback
- Miscellaneous fixes for error path return values and KUnit assertions.
----------------------------------------------------------------
Abdellah Ouhbi (1):
ext4: Use %pe to print PTR_ERR()
Aditya Prakash Srivastava (1):
ext4: fix kernel BUG in ext4_write_inline_data_end
Deepanshu Kartikey (1):
jbd2: check for aborted handle in jbd2_journal_dirty_metadata()
Guan-Chun Wu (2):
ext4: add Kunit coverage for directory hash computation
ext4: improve str2hashbuf by processing 4-byte chunks and removing function pointers
Hongling Zeng (1):
ext4: fix ERR_PTR(0) in ext4_mkdir()
Junrui Luo (1):
jbd2: fix integer underflow in jbd2_journal_initialize_fast_commit()
Li Chen (8):
ext4: fix fast commit wait/wake bit mapping on 64-bit
ext4: fast commit: snapshot inode state before writing log
ext4: lockdep: handle i_data_sem subclassing for special inodes
ext4: fast commit: avoid waiting for FC_COMMITTING
ext4: fast commit: avoid self-deadlock in inode snapshotting
ext4: fast commit: avoid i_data_sem by dropping ext4_map_blocks() in snapshots
ext4: fast commit: add lock_updates tracepoint
ext4: fast commit: export snapshot stats in fc_info
Matthew Wilcox (Oracle) (2):
ext4: remove mention of PageWriteback
jbd2: remove special jbd2 slabs
Ryota Sakamoto (1):
ext4: replace KUnit tests for memcmp() with KUNIT_ASSERT_MEMEQ()
Yun Zhou (1):
ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT
Zhang Yi (1):
ext4: fix LOGFLUSH shutdown ordering to allow ordered-mode data writeback
fs/ext4/Makefile | 2 +-
fs/ext4/ext4.h | 93 ++++-
fs/ext4/extents.c | 4 +-
fs/ext4/fast_commit.c | 784 ++++++++++++++++++++++++++++++++---------
fs/ext4/hash-test.c | 567 +++++++++++++++++++++++++++++
fs/ext4/hash.c | 68 ++--
fs/ext4/inode.c | 54 ++-
fs/ext4/ioctl.c | 15 +-
fs/ext4/mballoc-test.c | 9 +-
fs/ext4/namei.c | 6 +-
fs/ext4/page-io.c | 2 +-
fs/ext4/super.c | 13 +-
fs/jbd2/commit.c | 8 +-
fs/jbd2/journal.c | 127 +------
fs/jbd2/transaction.c | 17 +-
include/linux/jbd2.h | 3 -
include/trace/events/ext4.h | 61 ++++
17 files changed, 1495 insertions(+), 338 deletions(-)
create mode 100644 fs/ext4/hash-test.c
^ permalink raw reply
* Re: [PATCH v4 17/23] ext4: submit zeroed post-EOF data immediately in the iomap buffered I/O path
From: Jan Kara @ 2026-06-18 12:59 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-18-yi.zhang@huaweicloud.com>
On Mon 11-05-26 15:23:37, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In the generic buffered_head I/O path, we rely on the data=order mode to
> ensure that the zeroed EOF block data is written before updating
> i_disksize, thus preventing stale data from being exposed.
>
> However, the iomap buffered I/O path cannot use this mechanism. Instead,
> we issue the I/O immediately after performing the zero operation
> (without synchronous waiting for performance). This can reduce the risk
> of exposing stale data, but it does not guarantee that the zero data
> will be flushed to disk before the metadata of i_disksize is updated.
> The subsequent patches will wait for this I/O to complete before
> updating i_disksize.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Two nits below:
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 239d387ffaf2..e013aeb03d7b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4742,6 +4742,32 @@ static int ext4_block_zero_range(struct inode *inode,
> zero_written);
> }
>
> +static int ext4_iomap_submit_zero_block(struct inode *inode,
> + loff_t from, loff_t end)
> +{
> + struct address_space *mapping = inode->i_mapping;
> + struct folio *folio;
> + bool do_submit = false;
> +
> + folio = filemap_lock_folio(mapping, from >> PAGE_SHIFT);
> + if (IS_ERR(folio))
> + /* Already writeback and clear? */
^^^ Already written back and reclaimed
> + return PTR_ERR(folio) == -ENOENT ? 0 : PTR_ERR(folio);
> +
> + folio_wait_writeback(folio);
> + WARN_ON_ONCE(folio_test_writeback(folio));
> +
> + if (likely(folio_test_dirty(folio)))
> + do_submit = true;
> + folio_unlock(folio);
> + folio_put(folio);
So how is what you do here more efficient than just:
filemap_fdatawrite_range(mapping, from, end - 1)
? That will also do nothing if the folio isn't dirty, won't it?
Honza
> +
> + /* Submit zeroed block. */
> + if (do_submit)
> + return filemap_fdatawrite_range(mapping, from, end - 1);
> + return 0;
> +}
> +
> /*
> * Zero out a mapping from file offset 'from' up to the end of the block
> * which corresponds to 'from' or to the given 'end' inside this block.
> @@ -4765,8 +4791,10 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> if (IS_ENCRYPTED(inode) && !fscrypt_has_encryption_key(inode))
> return 0;
>
> - if (length > blocksize - offset)
> + if (length > blocksize - offset) {
> length = blocksize - offset;
> + end = from + length;
> + }
>
> err = ext4_block_zero_range(inode, from, length,
> &did_zero, &zero_written);
> @@ -4781,18 +4809,34 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> * TODO: In the iomap path, handle this by updating i_disksize to
> * i_size after the zeroed data has been written back.
> */
> - if (ext4_should_order_data(inode) &&
> - did_zero && zero_written && !IS_DAX(inode)) {
> - handle_t *handle;
> + if (did_zero && zero_written && !IS_DAX(inode)) {
> + if (ext4_should_order_data(inode)) {
> + handle_t *handle;
>
> - handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> - if (IS_ERR(handle))
> - return PTR_ERR(handle);
> + handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
>
> - err = ext4_jbd2_inode_add_write(handle, inode, from, length);
> - ext4_journal_stop(handle);
> - if (err)
> - return err;
> + err = ext4_jbd2_inode_add_write(handle, inode, from,
> + length);
> + ext4_journal_stop(handle);
> + if (err)
> + return err;
> + /*
> + * inodes using the iomap buffered I/O path do not use the
> + * data=ordered mode. We submit zeroed range directly here.
> + * Do not wait for I/O completion for performance.
> + *
> + * TODO: Any operation that extends i_disksize (including
> + * append write end io past the zeroed boundary, truncate up,
> + * and append fallocate) must wait for the relevant I/O to
> + * complete before updating i_disksize.
> + */
> + } else if (ext4_inode_buffered_iomap(inode)) {
> + err = ext4_iomap_submit_zero_block(inode, from, end);
> + if (err)
> + return err;
> + }
> }
>
> return 0;
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [PATCH v2 8/8] ext4: handle IOCB_NOWAIT in ext4_dio_needs_zeroing() with cache-only lookup
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
Add a nowait parameter to ext4_dio_needs_zeroing() and pass
EXT4_GET_BLOCKS_CACHED_NOWAIT flag to ext4_map_blocks() when
IOCB_NOWAIT is set. This ensures the needs_zeroing check only uses
cached extent info. If cache misses, ext4_map_blocks() returns
-EAGAIN, causing ext4_dio_needs_zeroing() to conservatively return
true (needs zeroing). The caller then tries to upgrade to exclusive
lock, which returns -EAGAIN for NOWAIT, avoiding potential sleep on
down_read(i_data_sem).
The caller in ext4_dio_write_checks() is updated to pass the
IOCB_NOWAIT flag from the kiocb.
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/file.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 5ffc1afd8050..44d1658d2b5a 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -228,7 +228,8 @@ ext4_extending_io(struct inode *inode, loff_t offset, size_t len)
* unwritten conversion for middle blocks are protected by i_data_sem
* and inode_dio_begin().
*/
-static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
+static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len,
+ bool nowait)
{
struct ext4_map_blocks map;
unsigned int blkbits = inode->i_blkbits;
@@ -236,10 +237,14 @@ static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
bool head_partial, tail_partial;
ext4_lblk_t head_lblk, tail_lblk;
int err;
+ int map_flags = 0;
if (pos + len > i_size_read(inode))
return true;
+ if (nowait)
+ map_flags = EXT4_GET_BLOCKS_CACHED_NOWAIT;
+
head_partial = (pos & blockmask) != 0;
tail_partial = ((pos + len) & blockmask) != 0;
head_lblk = pos >> blkbits;
@@ -249,7 +254,7 @@ static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
if (head_partial) {
map.m_lblk = head_lblk;
map.m_len = tail_lblk - head_lblk + 1;
- err = ext4_map_blocks(NULL, inode, &map, 0);
+ err = ext4_map_blocks(NULL, inode, &map, map_flags);
if (err <= 0 || !(map.m_flags & EXT4_MAP_MAPPED))
return true;
/* If this mapping already covers the tail block, we're done. */
@@ -261,7 +266,7 @@ static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
if (tail_partial) {
map.m_lblk = tail_lblk;
map.m_len = 1;
- err = ext4_map_blocks(NULL, inode, &map, 0);
+ err = ext4_map_blocks(NULL, inode, &map, map_flags);
if (err <= 0 || !(map.m_flags & EXT4_MAP_MAPPED))
return true;
}
@@ -516,7 +521,8 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
* under shared lock is safe.
*/
if (ext4_unaligned_io(inode, from, offset))
- needs_zeroing = ext4_dio_needs_zeroing(inode, offset, count);
+ needs_zeroing = ext4_dio_needs_zeroing(inode, offset, count,
+ iocb->ki_flags & IOCB_NOWAIT);
/* Determine whether we need to upgrade to an exclusive lock. */
if (*ilock_shared &&
--
2.43.7
^ permalink raw reply related
* [PATCH v2 7/8] ext4: handle IOMAP_NOWAIT in ext4_iomap_begin() with cache-only lookup
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
Pass EXT4_GET_BLOCKS_CACHED_NOWAIT flag to ext4_map_blocks() when
IOMAP_NOWAIT is set, ensuring that extent lookups only use the cached
extent status tree. If the cache misses, ext4_map_blocks() returns
-EAGAIN instead of sleeping on down_read(i_data_sem) to read extent
tree from disk.
This applies to both write and read paths in ext4_iomap_begin(),
allowing DIO/DAX operations with RWF_NOWAIT to avoid blocking on
extent tree lookups.
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/inode.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 03adbca3ec78..09f85cd6c118 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3781,6 +3781,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
struct ext4_map_blocks map;
u8 blkbits = inode->i_blkbits;
unsigned int orig_mlen;
+ int map_flags = 0;
if ((offset >> blkbits) > EXT4_MAX_LOGICAL_BLOCK)
return -EINVAL;
@@ -3795,6 +3796,12 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
map.m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
orig_mlen = map.m_len;
+ /*
+ * In NOWAIT context, only use cached extent info. If es cache misses,
+ * return -EAGAIN to avoid sleeping on down_read(i_data_sem).
+ */
+ if (flags & IOMAP_NOWAIT)
+ map_flags = EXT4_GET_BLOCKS_CACHED_NOWAIT;
if (flags & IOMAP_WRITE) {
/*
@@ -3804,7 +3811,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
* especially in multi-threaded overwrite requests.
*/
if (offset + length <= i_size_read(inode)) {
- ret = ext4_map_blocks(NULL, inode, &map, 0);
+ ret = ext4_map_blocks(NULL, inode, &map, map_flags);
/*
* For DAX we convert extents to initialized ones before
* copying the data, otherwise we do it after I/O so
@@ -3825,7 +3832,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
}
ret = ext4_iomap_alloc(inode, &map, flags);
} else {
- ret = ext4_map_blocks(NULL, inode, &map, 0);
+ ret = ext4_map_blocks(NULL, inode, &map, map_flags);
}
if (ret < 0)
--
2.43.7
^ permalink raw reply related
* [PATCH v2 6/8] ext4: return -EAGAIN from ext4_map_blocks() in NOWAIT cache miss
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
Make ext4_map_blocks() return -EAGAIN instead of 0 when
EXT4_GET_BLOCKS_CACHED_NOWAIT is set and the extent status cache
misses. This allows callers to easily distinguish between a successful
cache lookup (positive return value) and a cache miss requiring disk
access (-EAGAIN), simplifying error handling in NOWAIT paths.
The change affects two locations:
1. After cache hit: return retval ? retval : -EAGAIN
(return -EAGAIN if cache hit is hole/delayed)
2. After cache miss: return -EAGAIN
(instead of 0, indicating need for disk lookup)
The only existing caller using EXT4_GET_BLOCKS_CACHED_NOWAIT is the
ext4_get_link() -> ext4_getblk() path. Although ext4_getblk() now
takes a different return branch (err < 0 instead of err == 0) and
propagates -EAGAIN instead of NULL, ext4_get_link() converts both
cases to -ECHILD via IS_ERR_OR_NULL(), so the final error seen by
the VFS remains unchanged.
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/inode.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 832794294ccf..03adbca3ec78 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -760,7 +760,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
}
if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
- return retval;
+ return retval ? retval : -EAGAIN;
+
#ifdef ES_AGGRESSIVE_TEST
ext4_map_blocks_es_recheck(handle, inode, map,
&orig_map, flags);
@@ -776,7 +777,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
* cannot find extent in the cache.
*/
if (flags & EXT4_GET_BLOCKS_CACHED_NOWAIT)
- return 0;
+ return -EAGAIN;
/*
* Try to see if we can get the block without requesting a new
--
2.43.7
^ permalink raw reply related
* [PATCH v2 4/8] ext4: base unaligned DIO lock decision on partial block zeroing
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
For unaligned DIO writes, the previous ext4_overwrite_io() required the
entire range to fall within a single written extent. This was overly
conservative: the DIO layer only performs partial block zeroing for the
head and tail blocks when they are partially covered by the write.
Middle blocks that are fully covered are written as whole blocks
without any zeroing, so they are safe regardless of extent state.
Therefore exclusive lock is only required when partial block zeroing
will actually happen:
- The head partial block (if any) lands on a hole or unwritten extent.
- The tail partial block (if any) lands on a hole or unwritten extent.
Middle full-cover blocks can be in any state (hole, unwritten, or
written) - block allocation under shared lock is safe per the previous
patch's analysis (inode_dio_begin + i_data_sem protection).
Replace ext4_overwrite_io() with ext4_dio_needs_zeroing(), which
directly answers the question driving the lock decision. It uses at
most two ext4_map_blocks() calls: one for the head partial block (also
catching the case where it spans through the tail), and one for the
tail partial block if not already covered.
This enables shared lock for previously-rejected scenarios such as:
- Unaligned write spanning written extent + mid-range hole + written
extent at the tail.
- Unaligned write where the partial blocks land on written extents but
the middle has unwritten extents.
Performance:
Hardware: /dev/sda (rotational disk, ~1 GB/s sustained write)
Filesystem: ext4 default mkfs
Unaligned DIO writes (14336 bytes at +512 within each 16K stripe).
Each stripe is laid out as [written][unwritten][unwritten][written],
so the head and tail partial blocks land on written extents but the
middle is unwritten. Metric: IOPS.
JOBS Before After speedup
---- -------- --------- -------
1 15,547 17,381 1.12x
2 15,910 34,172 2.15x
4 15,014 57,567 3.83x
8 15,022 81,947 5.46x
16 14,586 99,126 6.80x
32 14,047 92,519 6.59x
Wall time at JOBS=32: 149.3s (Before) -> 22.7s (After), 6.58x faster.
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/file.c | 108 +++++++++++++++++++++++++++++++++----------------
1 file changed, 73 insertions(+), 35 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 886b73247aab..2681f148e7b8 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -213,31 +213,60 @@ ext4_extending_io(struct inode *inode, loff_t offset, size_t len)
return false;
}
-/* Is IO overwriting allocated or initialized blocks? */
-static bool ext4_overwrite_io(struct inode *inode,
- loff_t pos, loff_t len, bool *unwritten)
+/*
+ * Does an unaligned DIO write require partial block zeroing?
+ *
+ * Partial block zeroing is performed only for the head and tail blocks
+ * when they are partially covered by the write and the underlying extent
+ * is a hole or unwritten. Middle blocks (fully covered by the write)
+ * are written as whole blocks without zeroing.
+ *
+ * When zeroing is required, two concurrent unaligned DIO writes to the
+ * same partial block can race and corrupt each other's data, so the
+ * caller must take the exclusive i_rwsem and drain in-flight DIO. When
+ * zeroing is not required, shared lock is safe -- block allocation and
+ * unwritten conversion for middle blocks are protected by i_data_sem
+ * and inode_dio_begin().
+ */
+static bool ext4_dio_needs_zeroing(struct inode *inode, loff_t pos, loff_t len)
{
struct ext4_map_blocks map;
unsigned int blkbits = inode->i_blkbits;
- int err, blklen;
+ unsigned long blockmask = inode->i_sb->s_blocksize - 1;
+ bool head_partial, tail_partial;
+ ext4_lblk_t head_lblk, tail_lblk;
+ int err;
if (pos + len > i_size_read(inode))
- return false;
+ return true;
- map.m_lblk = pos >> blkbits;
- map.m_len = EXT4_MAX_BLOCKS(len, pos, blkbits);
- blklen = map.m_len;
+ head_partial = (pos & blockmask) != 0;
+ tail_partial = ((pos + len) & blockmask) != 0;
+ head_lblk = pos >> blkbits;
+ tail_lblk = (pos + len - 1) >> blkbits;
+
+ /* Check the head partial block. */
+ if (head_partial) {
+ map.m_lblk = head_lblk;
+ map.m_len = tail_lblk - head_lblk + 1;
+ err = ext4_map_blocks(NULL, inode, &map, 0);
+ if (err <= 0 || !(map.m_flags & EXT4_MAP_MAPPED))
+ return true;
+ /* If this mapping already covers the tail block, we're done. */
+ if (!tail_partial || map.m_lblk + err > tail_lblk)
+ return false;
+ }
- err = ext4_map_blocks(NULL, inode, &map, 0);
- if (err != blklen)
- return false;
- /*
- * 'err==len' means that all of the blocks have been preallocated,
- * regardless of whether they have been initialized or not. We need to
- * check m_flags to distinguish the unwritten extents.
- */
- *unwritten = !(map.m_flags & EXT4_MAP_MAPPED);
- return true;
+ /* Check the tail partial block. */
+ if (tail_partial) {
+ map.m_lblk = tail_lblk;
+ map.m_len = 1;
+ err = ext4_map_blocks(NULL, inode, &map, 0);
+ if (err <= 0 || !(map.m_flags & EXT4_MAP_MAPPED))
+ return true;
+ }
+
+ return false;
}
static ssize_t ext4_generic_write_checks(struct kiocb *iocb,
@@ -452,9 +481,10 @@ static const struct iomap_dio_ops ext4_dio_write_ops = {
* i_data_sem serializes concurrent extent tree modifications.
*
* 4. Otherwise, the write is unaligned and non-extending. Shared lock is
- * only safe for pure written-extent overwrites. Unwritten extents or
- * holes require exclusive lock because concurrent partial block zeroing
- * in the DIO layer could corrupt data.
+ * safe unless the DIO layer needs to perform partial block zeroing --
+ * i.e. the head or tail partial block sits on a hole or unwritten
+ * extent. In that case upgrade to the exclusive lock and drain
+ * in-flight DIO to avoid races with concurrent partial block zeroing.
*/
static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
bool *ilock_shared, bool *extend,
@@ -465,7 +495,7 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
loff_t offset;
size_t count;
ssize_t ret;
- bool overwrite = true, unaligned_io, unwritten = false;
+ bool needs_zeroing = false;
restart:
ret = ext4_generic_write_checks(iocb, from);
@@ -475,21 +505,22 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
offset = iocb->ki_pos;
count = ret;
- unaligned_io = ext4_unaligned_io(inode, from, offset);
*extend = ext4_extending_io(inode, offset, count);
/*
- * For unaligned writes we need to know the extent state to determine
- * whether shared lock is safe. For aligned writes we skip this check
- * entirely since allocation under shared lock is safe.
+ * For unaligned writes, check whether partial block zeroing will be
+ * needed. If so, exclusive lock is required to serialize against
+ * concurrent DIO that could race with the zeroing.
+ *
+ * For aligned writes we skip this check entirely since allocation
+ * under shared lock is safe.
*/
- if (unaligned_io)
- overwrite = ext4_overwrite_io(inode, offset, count, &unwritten);
+ if (ext4_unaligned_io(inode, from, offset))
+ needs_zeroing = ext4_dio_needs_zeroing(inode, offset, count);
/* Determine whether we need to upgrade to an exclusive lock. */
if (*ilock_shared &&
- ((!IS_NOSEC(inode) || *extend ||
- (unaligned_io && (!overwrite || unwritten))))) {
+ (!IS_NOSEC(inode) || *extend || needs_zeroing)) {
if (iocb->ki_flags & IOCB_NOWAIT) {
ret = -EAGAIN;
goto out;
@@ -503,16 +534,23 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
/*
* Now that locking is settled, determine dio flags and exclusivity
* requirements. We don't use DIO_OVERWRITE_ONLY because we enforce
- * behavior already. The inode lock is already held exclusive if the
- * write is unaligned non-overwrite or extending, so drain all
- * outstanding dio and set the force wait dio flag.
+ * behavior already. When holding the exclusive lock for a write that
+ * needs partial block zeroing or is extending the file, we must wait
+ * for the I/O to complete synchronously:
+ *
+ * - needs_zeroing: drain in-flight DIO whose end_io could race with
+ * our partial block zeroing, and force synchronous completion so we
+ * don't leave in-flight zeroing bios for the next writer to drain.
+ *
+ * - extend: the caller must update i_disksize after I/O completion,
+ * which requires the data to be on disk first.
*/
- if (!*ilock_shared && (unaligned_io || *extend)) {
+ if (!*ilock_shared && (needs_zeroing || *extend)) {
if (iocb->ki_flags & IOCB_NOWAIT) {
ret = -EAGAIN;
goto out;
}
- if (unaligned_io && (!overwrite || unwritten))
+ if (needs_zeroing)
inode_dio_wait(inode);
*dio_flags = IOMAP_DIO_FORCE_WAIT;
}
--
2.43.7
^ permalink raw reply related
* [PATCH v2 0/8] ext4: allow more DIO writes under shared i_rwsem
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
Changes since v1:
* Collect RVB from Honza and Yi. (Thank you for your review!)
* Added Patch 1 to fix NOWAIT issues reported by Sashiko.
* Added Patch 2 to fix ext3 DIO and DIO fallback data race issue.
(Patch 4 increases the probability of this race)
* Added Patches 5-8 to fix other NOWAIT issues discovered during
investigation.
v1: https://patch.msgid.link/20260611163441.2431805-1-libaokun@linux.alibaba.com
======
Hi all,
This series relaxes the i_rwsem requirements of ext4_dio_write_iter()
so that more direct I/O writes can proceed under the shared lock.
It continues the work started by Peng Wang's RFC [1]; I'm taking
over this effort going forward.
ext4_dio_write_checks() currently calls ext4_overwrite_io() to decide
whether the shared lock is sufficient. Its single ext4_map_blocks()
lookup only sees the first contiguous extent of the same type, which
forces the exclusive lock for two cases that are actually safe under
the shared lock (see individual patches for the full safety
argument):
1. Aligned writes spanning multiple already-allocated extents (e.g.
written + unwritten, or two discontiguous written extents).
2. Unaligned writes whose head/tail partial blocks land on written
extents but the fully-covered middle blocks include hole or
unwritten extents.
Patch 1 fixes a NOWAIT issue where ext4_iomap_alloc() may sleep when
IOMAP_NOWAIT is set.
Patch 2 fixes a data race between DIO completion and buffered I/O
fallback on ext3 (no-extent inodes). This race was made more likely
by Patch 4.
Patch 3 skips the ext4_overwrite_io() pre-check entirely for aligned
non-extending writes, letting them proceed under the shared lock
regardless of extent state.
Patch 4 replaces ext4_overwrite_io() with ext4_dio_needs_zeroing(),
which directly answers the question driving the lock decision. It
checks only the head and tail partial blocks (at most two
ext4_map_blocks() calls), and ignores the state of middle blocks.
Patch 5 fixes a NOWAIT issue by using kiocb_modified instead of
file_modified in DIO/DAX write paths.
Patch 6 makes ext4_map_blocks() to return -EAGAIN instead of 0 when
EXT4_GET_BLOCKS_CACHED_NOWAIT is set and cache lookup misses.
Patch 7 adds cache-only lookup support to ext4_iomap_begin() for
IOMAP_NOWAIT requests.
Patch 8 adds cache-only lookup support to ext4_dio_needs_zeroing()
for IOCB_NOWAIT requests.
Testing
=======
"kvm-xfstests -c ext4/all -g auto" passes with no new failures.
Performance
===========
Hardware: /dev/sda (rotational disk, ~1 GB/s sustained write)
Filesystem: ext4 default mkfs
Test 1: aligned 8K DIO writes spanning written+unwritten extent
boundaries. Each thread writes its own 1G region sequentially; the
file is rebuilt between runs so every block is written exactly once.
Metric: IOPS.
JOBS base +patch 3 +patch 3+4 speedup
---- --------- -------- ---------- -------
1 42,322 43,329 43,087 1.02x
2 68,516 70,677 66,958 1.03x
4 62,489 97,072 101,468 1.62x
8 58,701 110,819 113,679 1.94x
16 58,569 116,392 115,272 1.97x
32 60,860 117,244 119,621 1.97x
Wall time at JOBS=32: 69.2s (base) -> 35.4s (patched), 1.96x faster.
Test 2: unaligned DIO writes (14336 bytes at +512 within each 16K
stripe). Each stripe is laid out as [written][unwritten][unwritten]
[written], so the head and tail partial blocks land on written
extents but the middle is unwritten. Metric: IOPS.
JOBS base +patch 3 +patch 3+4 speedup
---- --------- -------- ---------- -------
1 15,547 15,975 17,381 1.12x
2 15,910 14,808 34,172 2.15x
4 15,014 14,828 57,567 3.83x
8 15,022 14,648 81,947 5.46x
16 14,586 14,262 99,126 6.80x
32 14,047 13,809 92,519 6.59x
Wall time at JOBS=32: 149.3s (base) -> 22.7s (patched), 6.58x faster.
In test 2, patch 3 alone has no effect (slight noise) because patch 3
only touches the aligned write path. Patch 4 introduces
ext4_dio_needs_zeroing() which precisely identifies when partial
block zeroing is required, allowing the shared lock for the much
larger set of unaligned writes that don't actually trigger zeroing.
Comments and questions are, as always, welcome.
Thanks,
Baokun
[1]: https://patch.msgid.link/20260607124935.6168-1-peng_wang@linux.alibaba.com
Baokun Li (8):
ext4: prevent sleeping allocation in NOWAIT write path
ext4: drain in-flight DIO before buffered write fallback
ext4: skip overwrite check for aligned non-extending DIO writes
ext4: base unaligned DIO lock decision on partial block zeroing
ext4: use kiocb_modified instead of file_modified in DIO/DAX write
path
ext4: return -EAGAIN from ext4_map_blocks() in NOWAIT cache miss
ext4: handle IOMAP_NOWAIT in ext4_iomap_begin() with cache-only lookup
ext4: handle IOCB_NOWAIT in ext4_dio_needs_zeroing() with cache-only
lookup
fs/ext4/file.c | 148 +++++++++++++++++++++++++++++++++---------------
fs/ext4/inode.c | 19 +++++--
2 files changed, 118 insertions(+), 49 deletions(-)
--
2.43.7
^ permalink raw reply
* [PATCH v2 1/8] ext4: prevent sleeping allocation in NOWAIT write path
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang, Sashiko
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
Block allocation requires journal access which may sleep, violating
NOWAIT semantics. Return -EAGAIN early when IOMAP_NOWAIT is set,
allowing the caller to retry without the NOWAIT constraint.
This ensures that write paths using IOMAP_NOWAIT (e.g., DIO with
RWF_NOWAIT) will not block on journal operations when blocks need
to be allocated.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260611163441.2431805-1-libaokun@linux.alibaba.com?part=1
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/inode.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c2c2d6ac7f3d..832794294ccf 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3672,6 +3672,9 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
int ret, dio_credits, m_flags = 0, retries = 0;
bool force_commit = false;
+ if (flags & IOMAP_NOWAIT)
+ return -EAGAIN;
+
/*
* Trim the mapping request to the maximum value that we can map at
* once for direct I/O.
--
2.43.7
^ permalink raw reply related
* [PATCH v2 5/8] ext4: use kiocb_modified instead of file_modified in DIO/DAX write path
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
file_modified() passes flags=0 which drops IOCB_NOWAIT, causing
file_update_time() to sleep in ext4_journal_start() via
ext4_dirty_inode() even in non-blocking contexts.
kiocb_modified(iocb) propagates iocb->ki_flags so that
generic_update_time() correctly returns -EAGAIN when IOCB_NOWAIT
is set and ->dirty_inode could block, matching the behavior
already adopted by XFS, FUSE, and ext2.
Affected paths:
- ext4_dio_write_checks(): DIO NOWAIT write
- ext4_write_checks(): shared by buffered (rejects NOWAIT upfront)
and DAX write (supports NOWAIT)
ext4_fallocate() in extents.c is not affected as it has no kiocb.
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/file.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 2681f148e7b8..5ffc1afd8050 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -307,7 +307,7 @@ static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
if (count <= 0)
return count;
- ret = file_modified(iocb->ki_filp);
+ ret = kiocb_modified(iocb);
if (ret)
return ret;
@@ -465,7 +465,7 @@ static const struct iomap_dio_ops ext4_dio_write_ops = {
*
* The decision is layered, evaluated in this order:
*
- * 1. If file_modified() needs to update security info (!IS_NOSEC), upgrade
+ * 1. If kiocb_modified() needs to update security info (!IS_NOSEC), upgrade
* to the exclusive lock -- the security update itself requires it,
* regardless of whether the write extends the file or is aligned.
*
@@ -555,7 +555,7 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
*dio_flags = IOMAP_DIO_FORCE_WAIT;
}
- ret = file_modified(file);
+ ret = kiocb_modified(iocb);
if (ret < 0)
goto out;
--
2.43.7
^ permalink raw reply related
* [PATCH v2 3/8] ext4: skip overwrite check for aligned non-extending DIO writes
From: Baokun Li @ 2026-06-18 12:57 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, yi.zhang, ojaswin, ritesh.list,
peng_wang
In-Reply-To: <20260618125735.4156639-1-libaokun@linux.alibaba.com>
Currently, ext4_dio_write_checks() calls ext4_overwrite_io() to
determine if a write is a pure overwrite, and upgrades to exclusive
i_rwsem if not. However, ext4_overwrite_io() uses a single
ext4_map_blocks() call which only returns the first contiguous extent of
the same type. A write spanning multiple pre-allocated extents (e.g.
written + unwritten, or two physically discontiguous written extents)
produces a false negative, forcing an unnecessary exclusive lock upgrade.
After commit 5d87c7fca2c1 ("ext4: avoid starting handle when dio
writing an unwritten extent") and commit 012924f0eeef ("ext4: remove
useless ext4_iomap_overwrite_ops"), ext4_iomap_begin()'s fast path
accepts both EXT4_MAP_MAPPED and EXT4_MAP_UNWRITTEN without starting a
journal transaction. The iomap iteration naturally handles multi-extent
ranges: each call returns the mapping for the current segment, and
unwritten-to-written conversion is deferred to ext4_dio_write_end_io().
This means the common case of mixed written/unwritten extents never
reaches ext4_iomap_alloc() at all.
Even for the less common case where the range contains a hole and
ext4_iomap_alloc() is needed, exclusive i_rwsem is still unnecessary for
aligned non-extending writes:
- truncate/punch_hole are kept out: they require exclusive i_rwsem
(blocked by our shared lock during allocation), and inode_dio_begin()
keeps their inode_dio_wait() blocked until in-flight bios complete.
- i_data_sem write-lock inside ext4_map_blocks() serializes concurrent
extent tree modifications (parallel writers to the same hole).
- The journal handle is per-thread and does not require i_rwsem
exclusion.
- i_disksize and orphan list are not involved in non-extending writes.
Skip the ext4_overwrite_io() check entirely for aligned writes by
initializing overwrite to true and only calling ext4_overwrite_io() for
unaligned writes. Unaligned writes still need the extent state check
because concurrent partial block zeroing in the DIO layer requires
exclusive serialization unless the range is a pure written-extent
overwrite.
Performance:
Hardware: /dev/sda (rotational disk, ~1 GB/s sustained write)
Filesystem: ext4 default mkfs
Aligned 8K DIO writes spanning written+unwritten extent boundaries.
Each thread writes its own 1G region sequentially; the file is rebuilt
between runs so every block is written exactly once. Metric: IOPS.
JOBS Before After speedup
---- -------- --------- -------
1 42,322 43,329 1.02x
2 68,516 70,677 1.03x
4 62,489 97,072 1.55x
8 58,701 110,819 1.89x
16 58,569 116,392 1.99x
32 60,860 117,244 1.93x
Wall time at JOBS=32: 69.2s (Before) -> 35.4s (After), 1.96x faster.
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
fs/ext4/file.c | 52 +++++++++++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 9f9bc0b13772..886b73247aab 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -434,16 +434,27 @@ static const struct iomap_dio_ops ext4_dio_write_ops = {
* condition requires an exclusive inode lock. If yes, then we restart the
* whole operation by releasing the shared lock and acquiring exclusive lock.
*
- * - For unaligned_io we never take shared lock as it may cause data corruption
- * when two unaligned IO tries to modify the same block e.g. while zeroing.
+ * The decision is layered, evaluated in this order:
*
- * - For extending writes case we don't take the shared lock, since it requires
- * updating inode i_disksize and/or orphan handling with exclusive lock.
+ * 1. If file_modified() needs to update security info (!IS_NOSEC), upgrade
+ * to the exclusive lock -- the security update itself requires it,
+ * regardless of whether the write extends the file or is aligned.
*
- * - shared locking will only be true mostly with overwrites, including
- * initialized blocks and unwritten blocks.
+ * 2. If the write extends i_size or i_disksize, upgrade to the exclusive
+ * lock to safely update i_disksize and the orphan list, regardless of
+ * alignment.
*
- * - Otherwise we will switch to exclusive i_rwsem lock.
+ * 3. Otherwise, for aligned non-extending writes, shared lock is always
+ * sufficient regardless of extent state (written, unwritten, or hole).
+ * truncate/punch_hole cannot run while we hold the shared i_rwsem
+ * (they need it exclusively); after we release it, inode_dio_begin()
+ * keeps their inode_dio_wait() blocked until in-flight bios complete.
+ * i_data_sem serializes concurrent extent tree modifications.
+ *
+ * 4. Otherwise, the write is unaligned and non-extending. Shared lock is
+ * only safe for pure written-extent overwrites. Unwritten extents or
+ * holes require exclusive lock because concurrent partial block zeroing
+ * in the DIO layer could corrupt data.
*/
static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
bool *ilock_shared, bool *extend,
@@ -454,7 +465,7 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
loff_t offset;
size_t count;
ssize_t ret;
- bool overwrite, unaligned_io, unwritten;
+ bool overwrite = true, unaligned_io, unwritten = false;
restart:
ret = ext4_generic_write_checks(iocb, from);
@@ -466,22 +477,19 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
unaligned_io = ext4_unaligned_io(inode, from, offset);
*extend = ext4_extending_io(inode, offset, count);
- overwrite = ext4_overwrite_io(inode, offset, count, &unwritten);
/*
- * Determine whether we need to upgrade to an exclusive lock. This is
- * required to change security info in file_modified(), for extending
- * I/O, any form of non-overwrite I/O, and unaligned I/O to unwritten
- * extents (as partial block zeroing may be required).
- *
- * Note that unaligned writes are allowed under shared lock so long as
- * they are pure overwrites. Otherwise, concurrent unaligned writes risk
- * data corruption due to partial block zeroing in the dio layer, and so
- * the I/O must occur exclusively.
+ * For unaligned writes we need to know the extent state to determine
+ * whether shared lock is safe. For aligned writes we skip this check
+ * entirely since allocation under shared lock is safe.
*/
+ if (unaligned_io)
+ overwrite = ext4_overwrite_io(inode, offset, count, &unwritten);
+
+ /* Determine whether we need to upgrade to an exclusive lock. */
if (*ilock_shared &&
- ((!IS_NOSEC(inode) || *extend || !overwrite ||
- (unaligned_io && unwritten)))) {
+ ((!IS_NOSEC(inode) || *extend ||
+ (unaligned_io && (!overwrite || unwritten))))) {
if (iocb->ki_flags & IOCB_NOWAIT) {
ret = -EAGAIN;
goto out;
@@ -496,8 +504,8 @@ static ssize_t ext4_dio_write_checks(struct kiocb *iocb, struct iov_iter *from,
* Now that locking is settled, determine dio flags and exclusivity
* requirements. We don't use DIO_OVERWRITE_ONLY because we enforce
* behavior already. The inode lock is already held exclusive if the
- * write is non-overwrite or extending, so drain all outstanding dio and
- * set the force wait dio flag.
+ * write is unaligned non-overwrite or extending, so drain all
+ * outstanding dio and set the force wait dio flag.
*/
if (!*ilock_shared && (unaligned_io || *extend)) {
if (iocb->ki_flags & IOCB_NOWAIT) {
--
2.43.7
^ 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