* [PATCH 1/8] ext4: add extra_flags param to ext4_convert_unwritten_extents()
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 2/8] ext4: fix endio zeroout corruption by tracking unwritten to-be-converted IOs Ojaswin Mujoo
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
Let callers pass in additional flags for ext4_map_blocks(). This is in
preparation of the upcoming changes.
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/ext4.h | 3 ++-
fs/ext4/extents.c | 8 ++++----
fs/ext4/file.c | 2 +-
fs/ext4/page-io.c | 2 +-
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8d3af062a830..b72f90b6375b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3971,7 +3971,8 @@ extern long ext4_fallocate(struct file *file, int mode, loff_t offset,
loff_t len);
extern int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
loff_t offset, ssize_t len,
- ext4_lblk_t *converted);
+ ext4_lblk_t *converted,
+ int extra_flags);
extern int ext4_convert_unwritten_extents_atomic(handle_t *handle,
struct inode *inode, loff_t offset, ssize_t len);
extern int ext4_convert_unwritten_io_end_vec(handle_t *handle,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 7193429a05e1..a229983f91dd 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4704,7 +4704,7 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
ret = ext4_convert_unwritten_extents(handle,
inode, (loff_t)map.m_lblk << blkbits,
(loff_t)map.m_len << blkbits,
- &converted);
+ &converted, 0);
if (ret)
map.m_len = converted;
@@ -5152,7 +5152,7 @@ int ext4_convert_unwritten_extents_atomic(handle_t *handle, struct inode *inode,
*/
int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
loff_t offset, ssize_t len,
- ext4_lblk_t *converted)
+ ext4_lblk_t *converted, int extra_flags)
{
ext4_lblk_t max_blocks, conv_blocks = 0;
int ret = 0, ret2 = 0, ret3 = 0;
@@ -5195,7 +5195,7 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
*/
ret = ext4_map_blocks(handle, inode, &map,
EXT4_GET_BLOCKS_IO_CONVERT_EXT |
- EXT4_EX_NOCACHE);
+ EXT4_EX_NOCACHE | extra_flags);
if (ret <= 0) {
ext4_warning(inode->i_sb,
"inode #%llu: block %u: len %u: ext4_map_blocks returned %d",
@@ -5248,7 +5248,7 @@ int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end)
list_for_each_entry(io_end_vec, &io_end->list_vec, list) {
ret = ext4_convert_unwritten_extents(handle, io_end->inode,
io_end_vec->offset,
- io_end_vec->size, NULL);
+ io_end_vec->size, NULL, 0);
if (ret)
break;
}
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 50d3c92709c8..06ef7a9b9f83 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -457,7 +457,7 @@ static int ext4_dio_write_end_io(struct kiocb *iocb, ssize_t size,
size);
else if (!error && size && flags & IOMAP_DIO_UNWRITTEN)
error = ext4_convert_unwritten_extents(NULL, inode, pos, size,
- NULL);
+ NULL, 0);
if (error)
return error;
/*
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 734d856a6d47..4c5c77da2a5e 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -685,7 +685,7 @@ static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
ret = ext4_convert_unwritten_extents(handle, inode, pos,
- size, NULL);
+ size, NULL, 0);
out_journal:
err = ext4_journal_stop(handle);
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 2/8] ext4: fix endio zeroout corruption by tracking unwritten to-be-converted IOs
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 1/8] ext4: add extra_flags param to ext4_convert_unwritten_extents() Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 3/8] ext4: add tracing for i_conv_pending_ios tracking Ojaswin Mujoo
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
In case endio conversion fails due to factors like ENOSPC or ENOMEM, ext4
uses a fallback where we zeroout the unwritten extents instead of
splitting. However, not all unwritten extents can be zeroed out
unconditionally as they might have data underneath which is pending endio
conversion. Zeroing out such an extent can cause silent corruption.
To fix this we track the IOs which are about to undergo conversion in
ei->i_conv_pending_ios. The zeroout code then uses this counter to
determine if it's safe to zeroout.
Design Notes:
**Locking**
The only mutual exclusion we need is between an IO that is about to be
submitted over an unwritten extent and zeroout. This is done using a
dedicated rwsem, ei->i_conv_pending_sem with the following rules:
1. Incrementing the counter must down_read(i_conv_pending_sem).
2. A zeroout must check the counter and zero data under
down_write(i_conv_pending_sem)
3. Decrementing the counter needs no lock. At worst, this makes zeroout
bail out due to a stale, elevated counter value, which is acceptable
4. Changes to i_conv_pending_ios should use atomic helpers.
This design provides us the required exclusion while ensuring that fast
paths don't see any heavy lock contention. Atomic counters help parallel
modifications to be serialized.
** Tolerance for false positives **
We trade false positives for simplicity of implementation because having a
false positive here will just error the endio which is still better than
silently corrupting the data. We can always optimize our tracking to make
it more accurate in the future.
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/ext4.h | 19 ++++++++++
fs/ext4/extents-test.c | 2 +
fs/ext4/extents.c | 78 ++++++++++++++++++++++++++++++++------
fs/ext4/inode.c | 92 +++++++++++++++++++++++++++++++++++++++++++++
fs/ext4/super.c | 12 ++++++
include/trace/events/ext4.h | 1 +
6 files changed, 192 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b72f90b6375b..1a01b1f809f7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -736,6 +736,11 @@ enum {
* Look EXT4_MAP_QUERY_LAST_IN_LEAF.
*/
#define EXT4_GET_BLOCKS_QUERY_LAST_IN_LEAF 0x1000
+/*
+ * Caller may convert an unwritten extent (possibly zeroing parts of it)
+ * without elevating the i_conv_pending_ios count to account for itself.
+ */
+#define EXT4_GET_BLOCKS_NO_CONV_ACC 0x2000
/*
* The bit position of these flags must not overlap with any of the
@@ -1235,6 +1240,15 @@ struct ext4_inode_info {
#ifdef CONFIG_FS_ENCRYPTION
struct fscrypt_inode_info *i_crypt_info;
#endif
+ /*
+ * Number of in-flight IOs pending unwritten->written conversion. Used
+ * by the zeroout fallback to avoid zeroing out data of such IOs.
+ * Incrementing needs down_read(i_conv_pending_sem), zeroout needs
+ * down_write(i_conv_pending_sem), decrementing needs no lock. See
+ * comment above ext4_conv_pending_inc().
+ */
+ atomic_t i_conv_pending_ios;
+ struct rw_semaphore i_conv_pending_sem;
};
/*
@@ -3983,6 +3997,11 @@ extern int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
struct ext4_map_blocks *map, int flags);
extern int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
struct ext4_map_blocks *map, int flags);
+extern void ext4_conv_pending_inc(struct inode *inode);
+extern void ext4_conv_pending_sum(struct inode *inode, int n);
+extern int ext4_conv_pending_inc_trylock(struct inode *inode);
+extern void ext4_conv_pending_dec(struct inode *inode);
+extern void ext4_conv_pending_sub(struct inode *inode, int n);
extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
int num,
struct ext4_ext_path *path);
diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index c3836ecb89f9..4d63c2050135 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -271,6 +271,8 @@ static int extents_kunit_init(struct kunit *test)
ext4_es_init_tree(&ei->i_es_tree);
rwlock_init(&ei->i_es_lock);
+ atomic_set(&ei->i_conv_pending_ios, 0);
+ init_rwsem(&ei->i_conv_pending_sem);
INIT_LIST_HEAD(&ei->i_es_list);
ei->i_es_all_nr = 0;
ei->i_es_shk_nr = 0;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index a229983f91dd..d255884cf075 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3311,6 +3311,40 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
return path;
}
+/*
+ * Returns true with i_conv_pending_sem held for write if it's safe to zeroout,
+ * else drops the lock and returns false. The caller must call
+ * ext4_conv_pending_zeroout_unlock() once the zeroout IO is complete.
+ *
+ * Callers are expected to have elevated i_conv_pending_ios to account for
+ * themselves, so a count of 1 is their own. Callers that haven't must pass
+ * EXT4_GET_BLOCKS_NO_CONV_ACC in flags, in which case any count is another IO.
+ *
+ * Refer to comment above ext4_conv_pending_inc() for details.
+ */
+static bool ext4_conv_pending_zeroout_lock(struct inode *inode, int flags)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+ int pending, thresh;
+
+ thresh = (flags & EXT4_GET_BLOCKS_NO_CONV_ACC) ? 0 : 1;
+
+ down_write(&ei->i_conv_pending_sem);
+ pending = atomic_read(&ei->i_conv_pending_ios);
+
+ WARN_ON_ONCE(pending < 0);
+ if (pending > thresh || pending < 0) {
+ up_write(&ei->i_conv_pending_sem);
+ return false;
+ }
+ return true;
+}
+
+static void ext4_conv_pending_zeroout_unlock(struct inode *inode)
+{
+ up_write(&EXT4_I(inode)->i_conv_pending_sem);
+}
+
static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
struct ext4_ext_path *path,
struct ext4_map_blocks *map, int flags)
@@ -3328,6 +3362,9 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
ee_len = ext4_ext_get_actual_len(ex);
is_unwrit = ext4_ext_is_unwritten(ex);
+ if (!ext4_conv_pending_zeroout_lock(inode, flags))
+ return -EINVAL;
+
if (flags & EXT4_GET_BLOCKS_CONVERT) {
/*
* EXT4_GET_BLOCKS_CONVERT: Caller wants the range specified by
@@ -3338,9 +3375,11 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
loff_t map_end = (loff_t) map->m_lblk + map->m_len;
loff_t ex_end = (loff_t) ee_block + ee_len;
- if (!is_unwrit)
+ if (!is_unwrit) {
/* Shouldn't happen. Just exit */
- return -EINVAL;
+ err = -EINVAL;
+ goto out_unlock;
+ }
/* zeroout left */
if (map->m_lblk > ee_block) {
@@ -3350,7 +3389,7 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
err = ext4_issue_zeroout(inode, lblk, pblk, len);
if (err)
/* ZEROOUT failed, just return original error */
- return err;
+ goto out_unlock;
}
/* zeroout right */
@@ -3361,7 +3400,7 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
err = ext4_issue_zeroout(inode, lblk, pblk, len);
if (err)
/* ZEROOUT failed, just return original error */
- return err;
+ goto out_unlock;
}
} else if (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN) {
/*
@@ -3370,9 +3409,11 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
* Zeroout the map range leaving rest as it is.
*/
- if (is_unwrit)
+ if (is_unwrit) {
/* Shouldn't happen. Just exit */
- return -EINVAL;
+ err = -EINVAL;
+ goto out_unlock;
+ }
lblk = map->m_lblk;
len = map->m_len;
@@ -3380,16 +3421,23 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
err = ext4_issue_zeroout(inode, lblk, pblk, len);
if (err)
/* ZEROOUT failed, just return original error */
- return err;
+ goto out_unlock;
} else {
/*
* We no longer perform unwritten to unwritten splits in IO paths.
* Hence this should not happen.
*/
WARN_ON_ONCE(true);
- return -EINVAL;
+ err = -EINVAL;
+ goto out_unlock;
}
+ /*
+ * The zeroout IO is done, no need to hold off new unwritten IOs while we
+ * update the extent tree.
+ */
+ ext4_conv_pending_zeroout_unlock(inode);
+
err = ext4_ext_get_access(handle, inode, path + depth);
if (err)
return err;
@@ -3401,6 +3449,10 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
return err;
return 0;
+
+out_unlock:
+ ext4_conv_pending_zeroout_unlock(inode);
+ return err;
}
/*
@@ -3743,7 +3795,8 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
split_map.m_lblk = map->m_lblk;
split_map.m_len = map->m_len;
- if (max_zeroout && (*allocated > split_map.m_len)) {
+ if (max_zeroout && (*allocated > split_map.m_len) &&
+ ext4_conv_pending_zeroout_lock(inode, flags)) {
if (*allocated <= max_zeroout) {
/* case 3 or 5 */
zero_ex1.ee_block =
@@ -3756,7 +3809,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
split_map.m_len - ee_block);
err = ext4_ext_zeroout(inode, &zero_ex1);
if (err)
- goto fallback;
+ goto zeroout_unlock;
split_map.m_len = *allocated;
}
if (split_map.m_lblk - ee_block + split_map.m_len <
@@ -3770,16 +3823,17 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
ext4_ext_pblock(ex));
err = ext4_ext_zeroout(inode, &zero_ex2);
if (err)
- goto fallback;
+ goto zeroout_unlock;
}
split_map.m_len += split_map.m_lblk - ee_block;
split_map.m_lblk = ee_block;
*allocated = map->m_len;
}
+zeroout_unlock:
+ ext4_conv_pending_zeroout_unlock(inode);
}
-fallback:
path = ext4_split_convert_extents(handle, inode, &split_map, path,
flags | EXT4_GET_BLOCKS_CONVERT, NULL);
if (IS_ERR(path))
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 6ede3644a738..280ba3d359e3 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -669,6 +669,98 @@ int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
return retval;
}
+static void _ext4_conv_pending_mod(struct inode *inode, int n)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+
+ atomic_add(n, &ei->i_conv_pending_ios);
+}
+
+/*
+ * In case endio conversion fails due to factors like ENOSPC or ENOMEM, ext4
+ * uses a fallback where we zeroout the unwritten extents instead of splitting.
+ * However, not all unwritten extents can be zeroed out unconditionally as
+ * they might have data underneath which is pending endio conversion. Zeroing
+ * out such an extent can cause silent corruption.
+ *
+ * To fix this we track the IOs which are about to undergo conversion in
+ * ei->i_conv_pending_ios. The zeroout code then uses this counter to
+ * determine if it's safe to zeroout.
+ *
+ * Design Notes:
+ *
+ * **Locking**
+ *
+ * The only mutual exclusion we need is between an IO that is about to be
+ * submitted over an unwritten extent and zeroout. This is done using a
+ * dedicated rwsem, ei->i_conv_pending_sem with the following rules:
+ *
+ * 1. Incrementing the counter must down_read(i_conv_pending_sem).
+ * 2. A zeroout must check the counter and zero data under
+ * down_write(i_conv_pending_sem)
+ * 3. Decrementing the counter needs no lock. At worst, this makes zeroout bail
+ * out due to a stale, elevated counter value, which is acceptable
+ * 4. Changes to i_conv_pending_ios should use atomic helpers.
+ *
+ * This design provides us the required exclusion while ensuring that fast paths
+ * don't see any heavy lock contention. Atomic counters help parallel
+ * modifications to be serialized.
+ *
+ * ** Tolerance for false positives **
+ *
+ * We trade false positives for simplicity of implementation because having a
+ * false positive here will just error the endio which is still better than
+ * silently corrupting the data. We can always optimize our tracking to make it
+ * more accurate in the future.
+ */
+void ext4_conv_pending_inc(struct inode *inode)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+
+ down_read(&ei->i_conv_pending_sem);
+ _ext4_conv_pending_mod(inode, 1);
+ up_read(&ei->i_conv_pending_sem);
+}
+
+void ext4_conv_pending_sum(struct inode *inode, int n)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+
+ down_read(&ei->i_conv_pending_sem);
+ _ext4_conv_pending_mod(inode, n);
+ up_read(&ei->i_conv_pending_sem);
+}
+
+/*
+ * Same as ext4_conv_pending_inc() but doesn't sleep if a zeroout is in
+ * progress. Returns -EAGAIN and leaves the counter untouched in that case.
+ */
+int ext4_conv_pending_inc_trylock(struct inode *inode)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+
+ if (!down_read_trylock(&ei->i_conv_pending_sem))
+ return -EAGAIN;
+
+ _ext4_conv_pending_mod(inode, 1);
+ up_read(&ei->i_conv_pending_sem);
+ return 0;
+}
+
+void ext4_conv_pending_dec(struct inode *inode)
+{
+ _ext4_conv_pending_mod(inode, -1);
+}
+
+void ext4_conv_pending_sub(struct inode *inode, int n)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+
+ down_read(&ei->i_conv_pending_sem);
+ _ext4_conv_pending_mod(inode, -n);
+ up_read(&ei->i_conv_pending_sem);
+}
+
int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
struct ext4_map_blocks *map, int flags)
{
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7d8aedfbbb87..866a6a70535e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1429,6 +1429,8 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
ei->i_es_shrink_lblk = 0;
ei->i_es_seq = 0;
ei->i_reserved_data_blocks = 0;
+ atomic_set(&ei->i_conv_pending_ios, 0);
+ init_rwsem(&ei->i_conv_pending_sem);
spin_lock_init(&(ei->i_block_reservation_lock));
ext4_init_pending_tree(&ei->i_pending_tree);
#ifdef CONFIG_QUOTA
@@ -1505,6 +1507,16 @@ static void ext4_destroy_inode(struct inode *inode)
ext4_msg(inode->i_sb, KERN_ERR,
"Inode %llu (%p): EXT4_STATE_DISKSIZE_GROW_PENDING not cleared!",
inode->i_ino, EXT4_I(inode));
+
+ if (!(EXT4_SB(inode->i_sb)->s_mount_state & EXT4_ERROR_FS) &&
+ !ext4_emergency_state(inode->i_sb)) {
+ int pending = atomic_read(&EXT4_I(inode)->i_conv_pending_ios);
+
+ if (WARN_ON_ONCE(pending))
+ ext4_msg(inode->i_sb, KERN_ERR,
+ "Inode %llu (%p): i_conv_pending_ios (%d) not cleared!",
+ inode->i_ino, EXT4_I(inode), pending);
+ }
}
static void ext4_shutdown(struct super_block *sb)
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index 92048da9dae4..7d255c47c446 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -43,6 +43,7 @@ struct partial_cluster;
{ EXT4_GET_BLOCKS_CONVERT, "CONVERT" }, \
{ EXT4_GET_BLOCKS_METADATA_NOFAIL, "METADATA_NOFAIL" }, \
{ EXT4_GET_BLOCKS_NO_NORMALIZE, "NO_NORMALIZE" }, \
+ { EXT4_GET_BLOCKS_NO_CONV_ACC, "NO_CONV_ACC" }, \
{ EXT4_GET_BLOCKS_CONVERT_UNWRITTEN, "CONVERT_UNWRITTEN" }, \
{ EXT4_GET_BLOCKS_ZERO, "ZERO" }, \
{ EXT4_GET_BLOCKS_IO_SUBMIT, "IO_SUBMIT" }, \
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/8] ext4: add tracing for i_conv_pending_ios tracking
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 1/8] ext4: add extra_flags param to ext4_convert_unwritten_extents() Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 2/8] ext4: fix endio zeroout corruption by tracking unwritten to-be-converted IOs Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 4/8] ext4: don't zeroout if we have inflight unwritten IOs in non buffered-iomap path Ojaswin Mujoo
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
Add tracing to track changes to i_conv_pending_ios count.
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/inode.c | 11 ++++++++++-
include/trace/events/ext4.h | 24 ++++++++++++++++++++++++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 280ba3d359e3..8b5cb37cdd6d 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -673,7 +673,16 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
{
struct ext4_inode_info *ei = EXT4_I(inode);
- atomic_add(n, &ei->i_conv_pending_ios);
+ /*
+ * Only use the full ordering variant when tracing is needed.
+ */
+ if (trace_ext4_conv_pending_ios_enabled()) {
+ int new_ios = atomic_add_return(n, &ei->i_conv_pending_ios);
+
+ trace_ext4_conv_pending_ios(inode, n > 0, new_ios - n, new_ios);
+ } else {
+ atomic_add(n, &ei->i_conv_pending_ios);
+ }
}
/*
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index 7d255c47c446..f1754b2e836c 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -3367,6 +3367,30 @@ TRACE_EVENT(ext4_block_zero_eof,
__entry->i_disksize, __entry->did_zero, __entry->zero_written)
);
+/* i_conv_pending_ios accounting tracepoint */
+TRACE_EVENT(ext4_conv_pending_ios,
+ TP_PROTO(struct inode *inode, bool add, int old_ios, int new_ios),
+ TP_ARGS(inode, add, old_ios, new_ios),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(u64, ino)
+ __field(bool, add)
+ __field(int, old_ios)
+ __field(int, new_ios)
+ ),
+ TP_fast_assign(
+ __entry->dev = inode->i_sb->s_dev;
+ __entry->ino = inode->i_ino;
+ __entry->add = add;
+ __entry->old_ios = old_ios;
+ __entry->new_ios = new_ios;
+ ),
+ TP_printk("dev %d:%d ino %llu %s i_conv_pending_ios %d -> %d",
+ MAJOR(__entry->dev), MINOR(__entry->dev), __entry->ino,
+ __entry->add ? "inc" : "dec",
+ __entry->old_ios, __entry->new_ios)
+);
+
#endif /* _TRACE_EXT4_H */
/* This part must be outside protection */
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 4/8] ext4: don't zeroout if we have inflight unwritten IOs in non buffered-iomap path
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
` (2 preceding siblings ...)
2026-11-02 18:26 ` [PATCH 3/8] ext4: add tracing for i_conv_pending_ios tracking Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 5/8] ext4: don't zeroout if we have inflight direct IOs Ojaswin Mujoo
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
When endio conversion of an unwritten extent fails (e.g. ENOSPC while
trying to split the extent), ext4 falls back to zeroing out the unwritten
portions of the extent and marking it as written. This is safe only when no
other in-flight writes have valid data underneath other parts of the same
unwritten extent, otherwise the zeroout would silently corrupt that data.
Commit ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O")
changed the dioread_nolock writeback (non buffered iomap) and dio path to
submit I/O without pre-splitting extents, deferring the split to endio.
This means multiple concurrent writes can share a single unwritten extent,
and a failed split on one of them could zeroout data belonging to another.
Fix this for dioread_nolock non buffered iomap writeback by making sure the
writeback path increments the i_conv_pending_ios counter when it encounters
an unwritten io_end, and decrements it after all the io_end_vecs have been
converted. The existing checks in zeroout paths then use this counter to
suppress the zeroout fallback whenever there are other unwritten IOs
pending conversion, preventing silent data corruption.
Fixes: ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O")
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/ext4.h | 7 ++++---
fs/ext4/extents.c | 29 +++++++++++++++++++++++------
fs/ext4/page-io.c | 6 ++++++
3 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1a01b1f809f7..44cd9f12d350 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4113,14 +4113,15 @@ extern int ext4_resize_end(struct super_block *sb, bool update_backups);
static inline void ext4_set_io_unwritten_flag(struct ext4_io_end *io_end)
{
- if (!(io_end->flag & EXT4_IO_END_UNWRITTEN))
+ if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) {
io_end->flag |= EXT4_IO_END_UNWRITTEN;
+ ext4_conv_pending_inc(io_end->inode);
+ }
}
static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
{
- if (io_end->flag & EXT4_IO_END_UNWRITTEN)
- io_end->flag &= ~EXT4_IO_END_UNWRITTEN;
+ io_end->flag &= ~EXT4_IO_END_UNWRITTEN;
}
extern const struct iomap_ops ext4_iomap_ops;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d255884cf075..96d8ad012b2a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5286,6 +5286,19 @@ int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end)
{
int ret = 0, err = 0;
struct ext4_io_end_vec *io_end_vec;
+ int n = 0;
+
+ /*
+ * We take a single i_conv_pending_ios count for every unwritten io_end
+ * however each io_vec also needs its count to prevent a neighboring
+ * io_end_vec zeroing its data. This can't be accurately done during
+ * submission so do it here.
+ */
+ list_for_each_entry(io_end_vec, &io_end->list_vec, list)
+ n++;
+
+ if (n > 1)
+ ext4_conv_pending_sum(io_end->inode, n - 1);
/*
* This is somewhat ugly but the idea is clear: When transaction is
@@ -5295,16 +5308,20 @@ int ext4_convert_unwritten_io_end_vec(handle_t *handle, ext4_io_end_t *io_end)
if (handle) {
handle = ext4_journal_start_reserved(handle,
EXT4_HT_EXT_CONVERT);
- if (IS_ERR(handle))
+ if (IS_ERR(handle)) {
+ ext4_conv_pending_sub(io_end->inode, n);
return PTR_ERR(handle);
+ }
}
list_for_each_entry(io_end_vec, &io_end->list_vec, list) {
- ret = ext4_convert_unwritten_extents(handle, io_end->inode,
- io_end_vec->offset,
- io_end_vec->size, NULL, 0);
- if (ret)
- break;
+ if (!ret)
+ ret = ext4_convert_unwritten_extents(handle,
+ io_end->inode,
+ io_end_vec->offset,
+ io_end_vec->size,
+ NULL, 0);
+ ext4_conv_pending_dec(io_end->inode);
}
if (handle)
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 4c5c77da2a5e..00abef904702 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -190,6 +190,12 @@ static int ext4_end_io_end(ext4_io_end_t *io_end)
if (test_opt(sb, DATA_ERR_ABORT))
jbd2_journal_abort(EXT4_SB(sb)->s_journal, ret);
+
+ /*
+ * Conversion was skipped so drop the count that submission took
+ */
+ if (io_end->flag & EXT4_IO_END_UNWRITTEN)
+ ext4_conv_pending_dec(inode);
} else {
ret = ext4_convert_unwritten_io_end_vec(handle, io_end);
}
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 5/8] ext4: don't zeroout if we have inflight direct IOs
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
` (3 preceding siblings ...)
2026-11-02 18:26 ` [PATCH 4/8] ext4: don't zeroout if we have inflight unwritten IOs in non buffered-iomap path Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 6/8] ext4: make sure zero range WRITE_ZEROES uses NO_CONV_ACC flag Ojaswin Mujoo
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
When endio conversion of an unwritten extent fails (e.g. ENOSPC while
trying to split the extent), ext4 falls back to zeroing out the unwritten
portions of the extent and marking it as written. This is safe only when no
other in-flight writes have valid data underneath other parts of the same
unwritten extent, otherwise the zeroout would silently corrupt that data.
Commit ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O")
changed the dio path to submit I/O without pre-splitting extents, deferring
the split to endio. This means multiple concurrent writes can share a
single unwritten extent, and a failed split on one of them could zeroout
data belonging to another.
Buffered IO fixes this by counting such IOs on unwritten extents in
i_conv_pending_ios and decrementing them after conversion. However, due to
the way dio iomap is designed, it is not very straightforward for the FS
layer to increment once per unwritten IO.
Hence, to keep it simple use the existing inode->i_dio_count which is
incremented/decremented once per dio. zeroout code can now use
i_dio_count with i_conv_pending_ios to decide if zeroout is safe to do.
The only thing missing is the mutual exclusion of i_dio_count increment vs
the zeroout decision. To fix that, add ext4_conv_pending_barrier() in
ext4_iomap_begin() which just rotates i_conv_pending_sem before returning
an unwritten mapping for a write. This ensures that either a racing zeroout
has finished before we issue an IO over unwritten extent, or zeroout starts
after our dio is accounted for.
This design is overly cautious in disabling zeroout (eg, we will disable it
for all inflight dio, not just dio over unwritten extents) but it keeps the
code simple.
Fixes: ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O")
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/ext4.h | 2 ++
fs/ext4/extents.c | 9 ++++++++-
fs/ext4/inode.c | 43 ++++++++++++++++++++++++++++++++++++++++---
3 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 44cd9f12d350..bf390b3b5606 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4002,6 +4002,8 @@ extern void ext4_conv_pending_sum(struct inode *inode, int n);
extern int ext4_conv_pending_inc_trylock(struct inode *inode);
extern void ext4_conv_pending_dec(struct inode *inode);
extern void ext4_conv_pending_sub(struct inode *inode, int n);
+extern void ext4_conv_pending_barrier(struct inode *inode);
+extern int ext4_conv_pending_barrier_trylock(struct inode *inode);
extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
int num,
struct ext4_ext_path *path);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 96d8ad012b2a..e4e65f3f576d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3333,10 +3333,17 @@ static bool ext4_conv_pending_zeroout_lock(struct inode *inode, int flags)
pending = atomic_read(&ei->i_conv_pending_ios);
WARN_ON_ONCE(pending < 0);
- if (pending > thresh || pending < 0) {
+ if (pending < 0) {
up_write(&ei->i_conv_pending_sem);
return false;
}
+
+ pending += atomic_read(&inode->i_dio_count);
+ if (pending > thresh) {
+ up_write(&ei->i_conv_pending_sem);
+ return false;
+ }
+
return true;
}
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8b5cb37cdd6d..9d7d4da0071f 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -692,9 +692,9 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
* they might have data underneath which is pending endio conversion. Zeroing
* out such an extent can cause silent corruption.
*
- * To fix this we track the IOs which are about to undergo conversion in
- * ei->i_conv_pending_ios. The zeroout code then uses this counter to
- * determine if it's safe to zeroout.
+ * To fix this we track the no. of buffered IOs which are about to undergo
+ * conversion in ei->i_conv_pending_ios. The zeroout code then uses this counter
+ * to determine if it's safe to zeroout.
*
* Design Notes:
*
@@ -715,6 +715,9 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
* don't see any heavy lock contention. Atomic counters help parallel
* modifications to be serialized.
*
+ * Note that dio does not use i_conv_pending_ios; instead we use i_dio_count to
+ * disable zeroout if there are any other inflight dios.
+ *
* ** Tolerance for false positives **
*
* We trade false positives for simplicity of implementation because having a
@@ -756,6 +759,30 @@ int ext4_conv_pending_inc_trylock(struct inode *inode)
return 0;
}
+/*
+ * Rotate i_conv_pending_sem. This guarantees mutual exclusion against a
+ * concurrent endio zeroout, which holds i_conv_pending_sem exclusive. This
+ * barrier ensures either the zeroout has completed before we return, or it will
+ * start after we have accounted for the io and will see it while zeroing out.
+ * This can be used for paths that might not be changing ei->i_conv_pending_ios
+ */
+void ext4_conv_pending_barrier(struct inode *inode)
+{
+ down_read(&EXT4_I(inode)->i_conv_pending_sem);
+ up_read(&EXT4_I(inode)->i_conv_pending_sem);
+}
+
+/*
+ * Same as ext4_conv_pending_barrier() but doesn't sleep on i_conv_pending_sem.
+ */
+int ext4_conv_pending_barrier_trylock(struct inode *inode)
+{
+ if (!down_read_trylock(&EXT4_I(inode)->i_conv_pending_sem))
+ return -EAGAIN;
+ up_read(&EXT4_I(inode)->i_conv_pending_sem);
+ return 0;
+}
+
void ext4_conv_pending_dec(struct inode *inode)
{
_ext4_conv_pending_mod(inode, -1);
@@ -4073,6 +4100,16 @@ int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
if (ret < 0)
return ret;
out:
+ if ((flags & IOMAP_WRITE) && (map.m_flags & EXT4_MAP_UNWRITTEN)) {
+ if (flags & IOMAP_NOWAIT) {
+ ret = ext4_conv_pending_barrier_trylock(inode);
+ if (ret)
+ return ret;
+ } else {
+ ext4_conv_pending_barrier(inode);
+ }
+ }
+
/*
* Sometimes I/O to an encrypted file has to be broken up to guarantee
* DUN contiguity. Handle this by limiting the length of the mapping
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 6/8] ext4: make sure zero range WRITE_ZEROES uses NO_CONV_ACC flag
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
` (4 preceding siblings ...)
2026-11-02 18:26 ` [PATCH 5/8] ext4: don't zeroout if we have inflight direct IOs Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 7/8] ext4: don't zeroout if we have inflight unwritten IOs in buffered iomap path Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 8/8] ext4: optimize inflight dio tracking for zeroout Ojaswin Mujoo
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
A zero range WRITE_ZEROES splits and converts an unwrit -> written extent
but it doesn't need any protection against the zeroout fallback in
ext4_split_extent_zeroout() because it's data is just zeroes anyways.
Hence, indicate to the zeroout code that the caller doesn't account for
itself in ei->i_conv_pending_ios.
Fixes: ea96cb5c4ae3 ("ext4: don't split extent before submitting I/O")
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/extents.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index e4e65f3f576d..d546001fedc6 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4765,7 +4765,7 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
ret = ext4_convert_unwritten_extents(handle,
inode, (loff_t)map.m_lblk << blkbits,
(loff_t)map.m_len << blkbits,
- &converted, 0);
+ &converted, EXT4_GET_BLOCKS_NO_CONV_ACC);
if (ret)
map.m_len = converted;
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 7/8] ext4: don't zeroout if we have inflight unwritten IOs in buffered iomap path
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
` (5 preceding siblings ...)
2026-11-02 18:26 ` [PATCH 6/8] ext4: make sure zero range WRITE_ZEROES uses NO_CONV_ACC flag Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
2026-11-02 18:26 ` [PATCH 8/8] ext4: optimize inflight dio tracking for zeroout Ojaswin Mujoo
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
The i_conv_pending_ios counter tracks the number of unwritten IOs that
are currently undergoing endio conversion, and is used to prevent
ext4_can_zeroout() from zeroing out unwritten extents that have valid
data pending conversion.
Enable the tracking for the buffered iomap writeback path by incrementing
i_conv_pending_ios once per submitted unwritten ioend in
ext4_iomap_writeback_submit() and decrement it after extent conversion.
Fixes: 8c4f6e337634 ("ext4: introduce a mount option for iomap buffered I/O path")
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/inode.c | 3 +++
fs/ext4/page-io.c | 19 +++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9d7d4da0071f..0c924ad8210b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4585,6 +4585,9 @@ static int ext4_iomap_writeback_submit(struct iomap_writepage_ctx *wpc,
if (ioend->io_bio.bi_end_io)
bio_clear_flag(&ioend->io_bio, BIO_COMPLETE_IN_TASK);
+ if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
+ ext4_conv_pending_inc(inode);
+
return iomap_ioend_writeback_submit(wpc, error);
}
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 00abef904702..e99816017a18 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -652,6 +652,21 @@ static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
int credits;
int ret, err;
+ /*
+ * We increment conv_pending for each IO we submit but endio might have
+ * merged them into a single ioend. The data of all merged sub-ioends is
+ * covered by a single conversion, so drop their counts keeping just
+ * the one count for the single ioend we will try to convert.
+ */
+ if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN) {
+ struct iomap_ioend *sub;
+
+ list_for_each_entry(sub, &ioend->io_list, io_list) {
+ if (sub->io_flags & IOMAP_IOEND_UNWRITTEN)
+ ext4_conv_pending_dec(inode);
+ }
+ }
+
ret = blk_status_to_errno(ioend->io_bio.bi_status);
if (is_disksize_grow)
trace_ext4_iomap_wb_disksize_pending_complete(ioend->io_inode,
@@ -704,6 +719,10 @@ static void ext4_iomap_finish_ioend(struct iomap_ioend *ioend)
inode->i_ino, ret);
}
out:
+ /* Drop the count of the head ioend now that conversion is done. */
+ if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
+ ext4_conv_pending_dec(inode);
+
iomap_finish_ioends(ioend, ret);
}
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 8/8] ext4: optimize inflight dio tracking for zeroout
2026-11-02 18:26 [PATCH 0/8] ext4: fix endio zeroout corruption by tracking to-be-converted unwritten IOs Ojaswin Mujoo
` (6 preceding siblings ...)
2026-11-02 18:26 ` [PATCH 7/8] ext4: don't zeroout if we have inflight unwritten IOs in buffered iomap path Ojaswin Mujoo
@ 2026-11-02 18:26 ` Ojaswin Mujoo
7 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-11-02 18:26 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ritesh Harjani (IBM),
Zhang Yi
Cc: linux-ext4, linux-kernel
Earlier zeroout used inode->i_dio_count to detect in-flight DIOs
that might have valid data underneath an unwritten extent. However,
i_dio_count accounts for every DIO on the inode, including reads and writes
that never touch an unwritten extent, which makes it prone to false
positives and needlessly suppresses the zeroout fallback.
Optimize this by moving DIO accounting to i_conv_pending_ios, which is only
incremented for DIOs that actually encounter an unwritten extent, and
decremented once at endio after the conversion is done.
The main issue is that ext4_iomap_begin() doesn't have any way to track
something only once per dio, hence, we use a custom ext4_iomap_next() that
utilizes iter->private to ensure we increment the i_conv_pending_ios only
once per a dio that encounters unwritten extents.
Now that dio has moved to i_conv_pending_ios, we need to pass NO_CONV_ACC
get blocks flag to ext4_ext_convert_to_initialized(). This path is
reachable from dio but doesn't need to account in i_conv_pending_ios
because we don't issue any IO before conversion, hence the flag helps
zeroout to make the right call.
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/ext4/ext4.h | 2 --
fs/ext4/extents.c | 11 ++-----
fs/ext4/file.c | 12 +++++++-
fs/ext4/inode.c | 85 ++++++++++++++++++++++++++++---------------------------
4 files changed, 58 insertions(+), 52 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bf390b3b5606..44cd9f12d350 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4002,8 +4002,6 @@ extern void ext4_conv_pending_sum(struct inode *inode, int n);
extern int ext4_conv_pending_inc_trylock(struct inode *inode);
extern void ext4_conv_pending_dec(struct inode *inode);
extern void ext4_conv_pending_sub(struct inode *inode, int n);
-extern void ext4_conv_pending_barrier(struct inode *inode);
-extern int ext4_conv_pending_barrier_trylock(struct inode *inode);
extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
int num,
struct ext4_ext_path *path);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d546001fedc6..3150754b9766 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3333,13 +3333,7 @@ static bool ext4_conv_pending_zeroout_lock(struct inode *inode, int flags)
pending = atomic_read(&ei->i_conv_pending_ios);
WARN_ON_ONCE(pending < 0);
- if (pending < 0) {
- up_write(&ei->i_conv_pending_sem);
- return false;
- }
-
- pending += atomic_read(&inode->i_dio_count);
- if (pending > thresh) {
+ if (pending > thresh || pending < 0) {
up_write(&ei->i_conv_pending_sem);
return false;
}
@@ -4117,7 +4111,8 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
* discovered unwritten extent to written.
*/
path = ext4_ext_convert_to_initialized(handle, inode, map, path,
- flags, allocated);
+ flags | EXT4_GET_BLOCKS_NO_CONV_ACC,
+ allocated);
if (IS_ERR(path))
return path;
ext4_update_inode_fsync_trans(handle, inode, 1);
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 06ef7a9b9f83..52b1fb99484d 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -458,6 +458,15 @@ static int ext4_dio_write_end_io(struct kiocb *iocb, ssize_t size,
else if (!error && size && flags & IOMAP_DIO_UNWRITTEN)
error = ext4_convert_unwritten_extents(NULL, inode, pos, size,
NULL, 0);
+
+ /*
+ * If this dio covered an unwritten extent, we would have incremented
+ * i_conv_pending_ios in ext4_iomap_next(). Decrement it now that
+ * the conversion has completed (or failed).
+ */
+ if (flags & IOMAP_DIO_UNWRITTEN)
+ ext4_conv_pending_dec(inode);
+
if (error)
return error;
/*
@@ -599,6 +608,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
size_t count = iov_iter_count(from);
bool extend = false;
bool ilock_shared = true;
+ bool conv_pending_inc = false;
int dio_flags = 0;
/*
@@ -664,7 +674,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
}
ret = iomap_dio_rw(iocb, from, &ext4_iomap_ops, &ext4_dio_write_ops,
- dio_flags, NULL, 0);
+ dio_flags, &conv_pending_inc, 0);
if (ret == -ENOTBLK)
ret = 0;
if (extend) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0c924ad8210b..46f91d6d8ed5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -692,9 +692,9 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
* they might have data underneath which is pending endio conversion. Zeroing
* out such an extent can cause silent corruption.
*
- * To fix this we track the no. of buffered IOs which are about to undergo
- * conversion in ei->i_conv_pending_ios. The zeroout code then uses this counter
- * to determine if it's safe to zeroout.
+ * To fix this we track the IOs which are about to undergo conversion in
+ * ei->i_conv_pending_ios. The zeroout code then uses this counter to
+ * determine if it's safe to zeroout.
*
* Design Notes:
*
@@ -715,9 +715,6 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
* don't see any heavy lock contention. Atomic counters help parallel
* modifications to be serialized.
*
- * Note that dio does not use i_conv_pending_ios; instead we use i_dio_count to
- * disable zeroout if there are any other inflight dios.
- *
* ** Tolerance for false positives **
*
* We trade false positives for simplicity of implementation because having a
@@ -759,30 +756,6 @@ int ext4_conv_pending_inc_trylock(struct inode *inode)
return 0;
}
-/*
- * Rotate i_conv_pending_sem. This guarantees mutual exclusion against a
- * concurrent endio zeroout, which holds i_conv_pending_sem exclusive. This
- * barrier ensures either the zeroout has completed before we return, or it will
- * start after we have accounted for the io and will see it while zeroing out.
- * This can be used for paths that might not be changing ei->i_conv_pending_ios
- */
-void ext4_conv_pending_barrier(struct inode *inode)
-{
- down_read(&EXT4_I(inode)->i_conv_pending_sem);
- up_read(&EXT4_I(inode)->i_conv_pending_sem);
-}
-
-/*
- * Same as ext4_conv_pending_barrier() but doesn't sleep on i_conv_pending_sem.
- */
-int ext4_conv_pending_barrier_trylock(struct inode *inode)
-{
- if (!down_read_trylock(&EXT4_I(inode)->i_conv_pending_sem))
- return -EAGAIN;
- up_read(&EXT4_I(inode)->i_conv_pending_sem);
- return 0;
-}
-
void ext4_conv_pending_dec(struct inode *inode)
{
_ext4_conv_pending_mod(inode, -1);
@@ -4100,16 +4073,6 @@ int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
if (ret < 0)
return ret;
out:
- if ((flags & IOMAP_WRITE) && (map.m_flags & EXT4_MAP_UNWRITTEN)) {
- if (flags & IOMAP_NOWAIT) {
- ret = ext4_conv_pending_barrier_trylock(inode);
- if (ret)
- return ret;
- } else {
- ext4_conv_pending_barrier(inode);
- }
- }
-
/*
* Sometimes I/O to an encrypted file has to be broken up to guarantee
* DUN contiguity. Handle this by limiting the length of the mapping
@@ -4132,7 +4095,47 @@ int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
return 0;
}
-static DEFINE_IOMAP_ITER_NEXT(ext4_iomap_next, ext4_iomap_begin);
+static int ext4_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
+ struct iomap *srcmap)
+{
+ int ret = iomap_iter_next(iter, iomap, srcmap, ext4_iomap_begin, NULL);
+
+ /*
+ * For dio, increment i_conv_pending_ios the first time we see an
+ * unwritten mapping for this IO, so that it cleanly balances with the
+ * single endio call. We use iter->private to track state to ensure we
+ * only increment once per dio regardless of how many extents it spans.
+ * The decrement happens in ext4_dio_write_end_io() if
+ * IOMAP_DIO_UNWRITTEN is set.
+ *
+ * Hack alert: iomap_dio_bio_iter() fails a misaligned mapping with
+ * -EINVAL before it sets IOMAP_DIO_UNWRITTEN, so endio would skip the
+ * decrement. Don't increment for such a mapping. The alignment check
+ * mirrors the one in iomap_dio_bio_iter()
+ */
+ if (ret > 0 && iter->private &&
+ (iter->flags & IOMAP_WRITE) && (iomap->type == IOMAP_UNWRITTEN) &&
+ IS_ALIGNED(iter->pos | iomap_length(iter),
+ bdev_logical_block_size(iomap->bdev))) {
+ bool *inc_done = (bool *)iter->private;
+
+ if (!*inc_done) {
+ /* NOWAIT dio must not sleep on i_conv_pending_sem */
+ if (iter->flags & IOMAP_NOWAIT) {
+ int err;
+
+ err = ext4_conv_pending_inc_trylock(iter->inode);
+ if (err)
+ return err;
+ } else {
+ ext4_conv_pending_inc(iter->inode);
+ }
+ *inc_done = true;
+ }
+ }
+
+ return ret;
+}
const struct iomap_ops ext4_iomap_ops = {
.iomap_next = ext4_iomap_next,
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread