[PATCH 0/6] ext4: fix buffered writeback error handling

[PATCH 0/6] ext4: fix buffered writeback error handling

[Julian Sun]

This series fixes error handling in ext4 buffered writeback. It prevents
stale data exposure when mapping or io_end_vec allocation fails, reports
unwritten conversion errors to fsync(), and preserves concurrent io_end
flag updates.

The patches are arranged as follows:

1. Move extent mapping error handling out of the loop, without changing
   behavior, so the following fixes can share the partial-folio path.
2. Submit mapped buffers when allocation of the next io_end_vec fails.
   Otherwise, their ranges remain eligible for unwritten conversion even
   though their data was never submitted.
3. Submit mapped buffers when mapping a later extent in the same folio
   fails with a fatal error, and discard buffers still needing mapping.
4. Drop the folio lookup reference if partial-writeback validation fails.
5. Prevent silent data loss by reporting unwritten extent conversion
   failures through fsync(), instead of returning success while direct
   reads still return zeros.
6. Use atomic bitops for io_end flags so failed-bio completion and
   writeback cannot overwrite each other's updates.

Sashiko helped identify these issues.

Julian Sun (6):
  ext4: move extent mapping error handling out of the loop
  ext4: submit mapped buffers when io_end_vec allocation fails
  ext4: prevent stale data exposure after fatal mapping errors
  ext4: put folio on partial writeback validation failure
  ext4: report unwritten extent conversion errors to fsync
  ext4: use atomic bitops for io_end flags

 fs/ext4/ext4.h    |  16 ++--
 fs/ext4/inode.c   | 188 +++++++++++++++++++++++++++++++---------------
 fs/ext4/page-io.c |  24 +++---
 3 files changed, 148 insertions(+), 80 deletions(-)


base-commit: 4403278e4ea2f654f8130a5c03eb8b4516a0f201
-- 
2.39.5

[PATCH 1/6] ext4: move extent mapping error handling out of the loop

[Julian Sun]

Move the error handling for mpage_map_one_extent() failures after the
mapping loop in mpage_map_and_submit_extent(). Keep buffer-processing
errors on the existing update_disksize path and preserve progress
tracking and return values.

This prepares for reusing partial-folio submission when buffer processing
fails. No functional change.

Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 fs/ext4/inode.c | 83 +++++++++++++++++++++++++------------------------
 1 file changed, 43 insertions(+), 40 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9a9ec02dce9f..bacead801709 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2467,46 +2467,8 @@ static int mpage_map_and_submit_extent(handle_t *handle,
 	io_end_vec->offset = EXT4_LBLK_TO_B(inode, map->m_lblk);
 	do {
 		err = mpage_map_one_extent(handle, mpd);
-		if (err < 0) {
-			struct super_block *sb = inode->i_sb;
-
-			if (ext4_emergency_state(sb))
-				goto invalidate_dirty_pages;
-			/*
-			 * Let the uper layers retry transient errors.
-			 * In the case of ENOSPC, if ext4_count_free_blocks()
-			 * is non-zero, a commit should free up blocks.
-			 */
-			if ((err == -ENOMEM) || (err == -EAGAIN) ||
-			    (err == -ENOSPC && ext4_count_free_clusters(sb))) {
-				/*
-				 * We may have already allocated extents for
-				 * some bhs inside the folio, issue the
-				 * corresponding data to prevent stale data.
-				 */
-				if (progress) {
-					if (mpage_submit_partial_folio(mpd))
-						goto invalidate_dirty_pages;
-					goto update_disksize;
-				}
-				return err;
-			}
-			ext4_msg(sb, KERN_CRIT,
-				 "Delayed block allocation failed for "
-				 "inode %llu at logical offset %llu with"
-				 " max blocks %u with error %d",
-				 inode->i_ino,
-				 (unsigned long long)map->m_lblk,
-				 (unsigned)map->m_len, -err);
-			ext4_msg(sb, KERN_CRIT,
-				 "This should not happen!! Data will "
-				 "be lost\n");
-			if (err == -ENOSPC)
-				ext4_print_free_blocks(inode);
-		invalidate_dirty_pages:
-			*give_up_on_write = true;
-			return err;
-		}
+		if (err < 0)
+			break;
 		progress = 1;
 		/*
 		 * Update buffer state, submit mapped pages, and get us new
@@ -2517,6 +2479,47 @@ static int mpage_map_and_submit_extent(handle_t *handle,
 			goto update_disksize;
 	} while (map->m_len);
 
+	if (err < 0) {
+		struct super_block *sb = inode->i_sb;
+
+		if (ext4_emergency_state(sb))
+			goto invalidate_dirty_pages;
+		/*
+		 * Let the uper layers retry transient errors.
+		 * In the case of ENOSPC, if ext4_count_free_blocks()
+		 * is non-zero, a commit should free up blocks.
+		 */
+		if ((err == -ENOMEM) || (err == -EAGAIN) ||
+		    (err == -ENOSPC && ext4_count_free_clusters(sb))) {
+			/*
+			 * We may have already allocated extents for
+			 * some bhs inside the folio, issue the
+			 * corresponding data to prevent stale data.
+			 */
+			if (progress) {
+				if (mpage_submit_partial_folio(mpd))
+					goto invalidate_dirty_pages;
+				goto update_disksize;
+			}
+			return err;
+		}
+		ext4_msg(sb, KERN_CRIT,
+			 "Delayed block allocation failed for "
+			 "inode %llu at logical offset %llu with"
+			 " max blocks %u with error %d",
+			 inode->i_ino,
+			 (unsigned long long)map->m_lblk,
+			 (unsigned)map->m_len, -err);
+		ext4_msg(sb, KERN_CRIT,
+			 "This should not happen!! Data will "
+			 "be lost\n");
+		if (err == -ENOSPC)
+			ext4_print_free_blocks(inode);
+invalidate_dirty_pages:
+		*give_up_on_write = true;
+		return err;
+	}
+
 update_disksize:
 	/*
 	 * Update on-disk size after IO is submitted.  Races with
-- 
2.39.5

Re: [PATCH 1/6] ext4: move extent mapping error handling out of the loop

[sashiko-bot]

> Move the error handling for mpage_map_one_extent() failures after the
> mapping loop in mpage_map_and_submit_extent(). Keep buffer-processing
> errors on the existing update_disksize path and preserve progress
> tracking and return values.
> 
> This prepares for reusing partial-folio submission when buffer processing
> fails. No functional change.
> 
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145241.2950379-1-sunjunchao@bytedance.com?part=1

[PATCH 2/6] ext4: submit mapped buffers when io_end_vec allocation fails

[Julian Sun]

When the filesystem block size is smaller than the folio size, a folio
can contain discontiguous unwritten ranges. mpage_process_folio() records
a mapped range for conversion before allocating an io_end_vec for the
next range.

If that allocation fails, mpage_map_and_submit_buffers() returns before
submitting the folio. Its caller skips partial-folio submission, leaving
the recorded range in io_end. Completion can then convert the range to
written without writing its data, allowing reads to return stale disk
contents.

Pass progress into buffer processing and set it as buffer mappings are
updated. Route errors after such progress through the existing
partial-folio submission path, so the mapped buffers are submitted before
io_end completion. Buffers still requiring mapping remain dirty for retry.

Fixes: 4d06bfb97ecb ("ext4: Add error handling for io_end_vec struct allocation")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 fs/ext4/inode.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bacead801709..cefc4b97ac41 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2226,7 +2226,7 @@ static int mpage_process_page_bufs(struct mpage_da_data *mpd,
  */
 static int mpage_process_folio(struct mpage_da_data *mpd, struct folio *folio,
 			      ext4_lblk_t *m_lblk, ext4_fsblk_t *m_pblk,
-			      bool *map_bh)
+			      bool *map_bh, int *progress)
 {
 	struct buffer_head *head, *bh;
 	ext4_io_end_t *io_end = mpd->io_submit.io_end;
@@ -2270,6 +2270,7 @@ static int mpage_process_folio(struct mpage_da_data *mpd, struct folio *folio,
 		}
 		clear_buffer_unwritten(bh);
 		io_end_size += i_blocksize(mpd->inode);
+		*progress = 1;
 	} while (lblk++, (bh = bh->b_this_page) != head);
 
 	io_end_vec->size += io_end_size;
@@ -2294,7 +2295,7 @@ static int mpage_process_folio(struct mpage_da_data *mpd, struct folio *folio,
  * mapped, we update @map to the next extent in the last page that needs
  * mapping. Otherwise we submit the page for IO.
  */
-static int mpage_map_and_submit_buffers(struct mpage_da_data *mpd)
+static int mpage_map_and_submit_buffers(struct mpage_da_data *mpd, int *progress)
 {
 	struct folio_batch fbatch;
 	unsigned nr, i;
@@ -2319,7 +2320,7 @@ static int mpage_map_and_submit_buffers(struct mpage_da_data *mpd)
 
 			lblk = EXT4_PG_TO_LBLK(inode, folio->index);
 			err = mpage_process_folio(mpd, folio, &lblk, &pblock,
-						 &map_bh);
+						 &map_bh, progress);
 			/*
 			 * If map_bh is true, means page may require further bh
 			 * mapping, or maybe the page was submitted for IO.
@@ -2469,14 +2470,16 @@ static int mpage_map_and_submit_extent(handle_t *handle,
 		err = mpage_map_one_extent(handle, mpd);
 		if (err < 0)
 			break;
-		progress = 1;
 		/*
 		 * Update buffer state, submit mapped pages, and get us new
 		 * extent to map
 		 */
-		err = mpage_map_and_submit_buffers(mpd);
-		if (err < 0)
+		err = mpage_map_and_submit_buffers(mpd, &progress);
+		if (err < 0) {
+			if (progress)
+				break;
 			goto update_disksize;
+		}
 	} while (map->m_len);
 
 	if (err < 0) {
-- 
2.39.5

Re: [PATCH 2/6] ext4: submit mapped buffers when io_end_vec allocation fails

[sashiko-bot]

> When the filesystem block size is smaller than the folio size, a folio
> can contain discontiguous unwritten ranges. mpage_process_folio() records
> a mapped range for conversion before allocating an io_end_vec for the
> next range.
> 
> If that allocation fails, mpage_map_and_submit_buffers() returns before
> submitting the folio. Its caller skips partial-folio submission, leaving
> the recorded range in io_end. Completion can then convert the range to
> written without writing its data, allowing reads to return stale disk
> contents.
> 
> Pass progress into buffer processing and set it as buffer mappings are
> updated. Route errors after such progress through the existing
> partial-folio submission path, so the mapped buffers are submitted before
> io_end completion. Buffers still requiring mapping remain dirty for retry.
> 
> Fixes: 4d06bfb97ecb ("ext4: Add error handling for io_end_vec struct allocation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145241.2950379-1-sunjunchao@bytedance.com?part=2

[PATCH 3/6] ext4: prevent stale data exposure after fatal mapping errors

[Julian Sun]

When the block size is smaller than the folio size, a folio can contain
several extents that need mapping. Mapping an earlier extent can succeed
before mapping a later extent fails with a fatal error such as -EIO.

mpage_map_and_submit_extent() submits the mapped part of the folio only
for retryable errors. On a fatal error it discards the whole folio,
including dirty buffers whose mapping has already succeeded. This loses
data that could still be submitted.

With dioread_nolock, the mapped ranges can also remain in io_end's
unwritten conversion list. Conversion can then mark those extents
written without their data ever being submitted, exposing stale data.

Also submit the mapped buffers after a fatal mapping error when the
filesystem is not in an emergency state. Discard buffers still marked
BH_Delay or BH_Unwritten so that writeback does not keep retrying them.
Clearing their dirty bits alone would leave stale buffer mappings and
delayed allocation reservations. Remove their extent status entries and
invalidate the buffers. Do this after folio_clear_dirty_for_io(), since
cleaning mmap writes can mark all buffers dirty again.

Update i_disksize using the mapped boundary before moving start_pos past
the folio. Advancing it earlier could include discarded data in the size
update. Advancing it afterwards makes mpage_release_unused_pages() skip
the handled folio and its extent status entries, including any entries
created by new writes after the folio was unlocked. Count the handled
folio against the writeback budget on fatal errors. Keep the existing
retry behavior for transient errors.

Fixes: c8cc88163f40 ("ext4: Add support for blocksize < pagesize in dioread_nolock")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 fs/ext4/inode.c | 108 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 83 insertions(+), 25 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index cefc4b97ac41..534833b13098 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2066,13 +2066,46 @@ static void mpage_folio_done(struct mpage_da_data *mpd, struct folio *folio)
 	folio_unlock(folio);
 }
 
-static void mpage_submit_folio(struct mpage_da_data *mpd, struct folio *folio)
+static void mpage_submit_folio(struct mpage_da_data *mpd, struct folio *folio,
+			       bool fatal)
 {
 	size_t len;
 	loff_t size;
 
 	WARN_ON_ONCE(folio_pos(folio) != mpd->start_pos);
 	folio_clear_dirty_for_io(folio);
+
+	/*
+	 * If a later mapping fails with a fatal error, write the buffers
+	 * ALREADY mapped and discard the data in buffers still needing
+	 * mapping. Do this after folio_clear_dirty_for_io(), which may
+	 * mark all buffers dirty again for mmap writes.
+	 */
+	if (fatal) {
+		struct buffer_head *bh, *head;
+		ext4_lblk_t start, length;
+		struct inode *inode = mpd->inode;
+
+		/*
+		 * Remove the ES entries, free the reserved space and clear the
+		 * buffer mappings before new writes can use this folio. Hold
+		 * i_data_sem to prevent races with delayed allocation lookups.
+		 */
+		down_write(&EXT4_I(inode)->i_data_sem);
+
+		bh = head = folio_buffers(folio);
+		do {
+			if (buffer_delay(bh) || buffer_unwritten(bh)) {
+				start = EXT4_B_TO_LBLK(inode, folio_pos(folio) + bh_offset(bh));
+				length = bh->b_size >> inode->i_blkbits;
+				ext4_es_remove_extent(inode, start, length);
+				block_invalidate_folio(folio, bh_offset(bh), bh->b_size);
+			}
+		} while ((bh = bh->b_this_page) != head);
+
+		up_write(&EXT4_I(inode)->i_data_sem);
+	}
+
 	/*
 	 * We have to be very careful here!  Nothing protects writeback path
 	 * against i_size changes and the page can be writeably mapped into
@@ -2198,7 +2231,7 @@ static int mpage_process_page_bufs(struct mpage_da_data *mpd,
 	} while (lblk++, (bh = bh->b_this_page) != head);
 	/* So far everything mapped? Submit the page for IO. */
 	if (mpd->map.m_len == 0) {
-		mpage_submit_folio(mpd, head->b_folio);
+		mpage_submit_folio(mpd, head->b_folio, false);
 		mpage_folio_done(mpd, head->b_folio);
 	}
 	if (lblk >= blocks) {
@@ -2329,7 +2362,7 @@ static int mpage_map_and_submit_buffers(struct mpage_da_data *mpd, int *progress
 			if (err < 0 || map_bh)
 				goto out;
 			/* Page fully mapped - let IO run! */
-			mpage_submit_folio(mpd, folio);
+			mpage_submit_folio(mpd, folio, false);
 			mpage_folio_done(mpd, folio);
 		}
 		folio_batch_release(&fbatch);
@@ -2394,14 +2427,15 @@ static int mpage_map_one_extent(handle_t *handle, struct mpage_da_data *mpd)
 }
 
 /*
- * This is used to submit mapped buffers in a single folio that is not fully
- * mapped for various reasons, such as insufficient space or journal credits.
+ * Write the buffers that are ready in a partly mapped folio. On a fatal
+ * error, drop the buffers that still need mapping instead of retrying them.
+ * Return the byte offset just past the folio on success, or a negative error.
  */
-static int mpage_submit_partial_folio(struct mpage_da_data *mpd)
+static loff_t mpage_submit_partial_folio(struct mpage_da_data *mpd, bool fatal)
 {
 	struct inode *inode = mpd->inode;
 	struct folio *folio;
-	loff_t pos;
+	loff_t pos, end;
 
 	folio = filemap_get_folio(inode->i_mapping,
 				  mpd->start_pos >> PAGE_SHIFT);
@@ -2416,18 +2450,21 @@ static int mpage_submit_partial_folio(struct mpage_da_data *mpd)
 			 !folio_contains(folio, pos >> PAGE_SHIFT)))
 		return -EINVAL;
 
-	mpage_submit_folio(mpd, folio);
+	end = folio_next_pos(folio);
+	mpage_submit_folio(mpd, folio, fatal);
 	/*
 	 * Update start_pos to prevent this folio from being released in
-	 * mpage_release_unused_pages(), it will be reset to the aligned folio
-	 * pos when this folio is written again in the next round. Additionally,
-	 * do not update wbc->nr_to_write here, as it will be updated once the
-	 * entire folio has finished processing.
+	 * mpage_release_unused_pages(). For retryable errors, it will be reset
+	 * to the aligned folio pos when this folio is written again in the next
+	 * round. Count the folio now only for fatal errors; otherwise the
+	 * retry will count it once the entire folio has finished processing.
 	 */
 	mpd->start_pos = pos;
+	if (fatal)
+		mpd->wbc->nr_to_write -= folio_nr_pages(folio);
 	folio_unlock(folio);
 	folio_put(folio);
-	return 0;
+	return end;
 }
 
 /*
@@ -2461,6 +2498,8 @@ static int mpage_map_and_submit_extent(handle_t *handle,
 	int progress = 0;
 	ext4_io_end_t *io_end = mpd->io_submit.io_end;
 	struct ext4_io_end_vec *io_end_vec;
+	bool fatal = false;
+	loff_t next_folio_pos = 0;
 
 	io_end_vec = ext4_alloc_io_end_vec(io_end);
 	if (IS_ERR(io_end_vec))
@@ -2494,18 +2533,28 @@ static int mpage_map_and_submit_extent(handle_t *handle,
 		 */
 		if ((err == -ENOMEM) || (err == -EAGAIN) ||
 		    (err == -ENOSPC && ext4_count_free_clusters(sb))) {
-			/*
-			 * We may have already allocated extents for
-			 * some bhs inside the folio, issue the
-			 * corresponding data to prevent stale data.
-			 */
-			if (progress) {
-				if (mpage_submit_partial_folio(mpd))
-					goto invalidate_dirty_pages;
-				goto update_disksize;
-			}
-			return err;
+			/* Already false, but set it here to make the choice clear. */
+			fatal = false;
+		} else {
+			fatal = true;
+		}
+
+		/*
+		 * We may have already allocated extents for
+		 * some bhs inside the folio, issue the
+		 * corresponding data to prevent stale data.
+		 */
+		if (progress) {
+			next_folio_pos = mpage_submit_partial_folio(mpd, fatal);
+			if (next_folio_pos < 0)
+				goto invalidate_dirty_pages;
+
+			goto update_disksize;
 		}
+
+		if (!fatal)
+			return err;
+
 		ext4_msg(sb, KERN_CRIT,
 			 "Delayed block allocation failed for "
 			 "inode %llu at logical offset %llu with"
@@ -2549,6 +2598,15 @@ static int mpage_map_and_submit_extent(handle_t *handle,
 		if (!err)
 			err = err2;
 	}
+	if (fatal) {
+		/*
+		 * We have submitted the mapped buffers and dropped the rest.
+		 * After updating i_disksize, move start_pos past this folio so
+		 * mpage_release_unused_pages() skips its buffers and ES entries.
+		 */
+		mpd->start_pos = next_folio_pos;
+		*give_up_on_write = true;
+	}
 	return err;
 }
 
@@ -2731,7 +2789,7 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
 			 * through a pin.
 			 */
 			if (!mpd->can_map) {
-				mpage_submit_folio(mpd, folio);
+				mpage_submit_folio(mpd, folio, false);
 				err = 0;
 				/* Pending dirtying of journalled data? */
 				if (folio_test_checked(folio)) {
-- 
2.39.5

Re: [PATCH 3/6] ext4: prevent stale data exposure after fatal mapping errors

[sashiko-bot]

> When the block size is smaller than the folio size, a folio can contain
> several extents that need mapping. Mapping an earlier extent can succeed
> before mapping a later extent fails with a fatal error such as -EIO.
> 
> mpage_map_and_submit_extent() submits the mapped part of the folio only
> for retryable errors. On a fatal error it discards the whole folio,
> including dirty buffers whose mapping has already succeeded. This loses
> data that could still be submitted.
> 
> With dioread_nolock, the mapped ranges can also remain in io_end's
> unwritten conversion list. Conversion can then mark those extents
> written without their data ever being submitted, exposing stale data.
> 
> Also submit the mapped buffers after a fatal mapping error when the
> filesystem is not in an emergency state. Discard buffers still marked
> [ ... ]
> retry behavior for transient errors.
> 
> Fixes: c8cc88163f40 ("ext4: Add support for blocksize < pagesize in dioread_nolock")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145241.2950379-1-sunjunchao@bytedance.com?part=3

[PATCH 4/6] ext4: put folio on partial writeback validation failure

[Julian Sun]

mpage_submit_partial_folio() takes a reference with filemap_get_folio(),
but returns without dropping it if the mapping position fails the
WARN_ON_ONCE() check. Release the lookup reference before returning
-EINVAL. Leave the folio locked for the caller's cleanup.

Fixes: ded2d726a304 ("ext4: fix stale data if it bail out of the extents mapping loop")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 fs/ext4/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 534833b13098..90be191715d7 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2447,8 +2447,10 @@ static loff_t mpage_submit_partial_folio(struct mpage_da_data *mpd, bool fatal)
 	 */
 	pos = ((loff_t)mpd->map.m_lblk) << inode->i_blkbits;
 	if (WARN_ON_ONCE((folio_pos(folio) == pos) ||
-			 !folio_contains(folio, pos >> PAGE_SHIFT)))
+			 !folio_contains(folio, pos >> PAGE_SHIFT))) {
+		folio_put(folio);
 		return -EINVAL;
+	}
 
 	end = folio_next_pos(folio);
 	mpage_submit_folio(mpd, folio, fatal);
-- 
2.39.5

Re: [PATCH 4/6] ext4: put folio on partial writeback validation failure

[sashiko-bot]

> mpage_submit_partial_folio() takes a reference with filemap_get_folio(),
> but returns without dropping it if the mapping position fails the
> WARN_ON_ONCE() check. Release the lookup reference before returning
> -EINVAL. Leave the folio locked for the caller's cleanup.
> 
> Fixes: ded2d726a304 ("ext4: fix stale data if it bail out of the extents mapping loop")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145241.2950379-1-sunjunchao@bytedance.com?part=4

[PATCH 5/6] ext4: report unwritten extent conversion errors to fsync

[Julian Sun]

Buffered writeback can complete data I/O successfully but fail to convert
unwritten extents, for example when allocating an extent path returns
ENOMEM. ext4_end_io_end() logs the failure and returns an error, but the
conversion worker discards that return value. As a result, fsync() can
report success while the extents remain unwritten and direct reads return
zeros instead of the data written.

Record the error in the inode mapping before ext4_release_io_end() ends
folio writeback. This lets fsync() observe the error after waiting for
writeback.

Fixes: b82e384c7bb9 ("ext4: optimize locking for end_io extent conversion")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 fs/ext4/page-io.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 0236b6b9785a..d25179938487 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -190,6 +190,10 @@ static int ext4_end_io_end(ext4_io_end_t *io_end)
 	} else {
 		ret = ext4_convert_unwritten_io_end_vec(handle, io_end);
 	}
+
+	if (ret < 0)
+		mapping_set_error(inode->i_mapping, ret);
+
 	if (ret < 0 && !ext4_emergency_state(sb) &&
 	    io_end->flag & EXT4_IO_END_UNWRITTEN) {
 		ext4_msg(sb, KERN_EMERG,
-- 
2.39.5

[PATCH 6/6] ext4: use atomic bitops for io_end flags

[Julian Sun]

Before EXT4_IO_END_FAILED was added, the writeback thread set
EXT4_IO_END_UNWRITTEN and final completion cleared it after the last
io_end reference was dropped. Adding EXT4_IO_END_FAILED introduced a
concurrent writer in failed-bio completion. Successful bio completions
do not update the flags.

An io_end can cover already-mapped folios submitted before a later
unwritten extent is mapped. An earlier bio can fail while writeback
first sets UNWRITTEN. The non-atomic updates can then lose a bit:

  Bio completion                  Writeback thread
  --------------                  ----------------
  read flag = 0
                                  attach reserved handle to io_end
                                  flag |= UNWRITTEN
  write flag = 0 | FAILED
  /* UNWRITTEN is lost */

With data_err=ignore, losing UNWRITTEN makes
ext4_io_end_defer_completion() return false. When the io_end reference
count reaches zero, ext4_end_io_end() and its reserved-handle cleanup
are therefore skipped.
ext4_release_io_end() only warns about the remaining handle before
freeing io_end. The leaked reservation keeps j_reserved_credits nonzero,
so jbd2_journal_lock_updates() can wait indefinitely.

The reverse interleaving can lose FAILED and skip a requested journal
abort.

Use atomic bit operations to fix the race.

Fixes: e856f93e0fb2 ("ext4: do not convert the unwritten extents if data writeback fails")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
 fs/ext4/ext4.h    | 16 ++++++----------
 fs/ext4/page-io.c | 20 +++++++++++---------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1ee5733e2b31..3332a19746eb 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -271,12 +271,10 @@ struct ext4_system_blocks {
 };
 
 /*
- * Flags for ext4_io_end->flags
+ * Bit numbers for ext4_io_end->flag
  */
-#define EXT4_IO_END_UNWRITTEN	0x0001
-#define EXT4_IO_END_FAILED	0x0002
-
-#define EXT4_IO_END_DEFER_COMPLETION (EXT4_IO_END_UNWRITTEN | EXT4_IO_END_FAILED)
+#define EXT4_IO_END_UNWRITTEN	0
+#define EXT4_IO_END_FAILED	1
 
 struct ext4_io_end_vec {
 	struct list_head list;		/* list of io_end_vec */
@@ -295,7 +293,7 @@ typedef struct ext4_io_end {
 	struct inode		*inode;		/* file being written to */
 	struct bio		*bio;		/* Linked list of completed
 						 * bios covering the extent */
-	unsigned int		flag;		/* unwritten or not */
+	unsigned long		flag;		/* EXT4_IO_END_* bits */
 	refcount_t		count;		/* reference counter */
 	struct list_head	list_vec;	/* list of ext4_io_end_vec */
 } ext4_io_end_t;
@@ -4030,14 +4028,12 @@ 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))
-		io_end->flag |= EXT4_IO_END_UNWRITTEN;
+	set_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag);
 }
 
 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;
+	clear_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag);
 }
 
 extern const struct iomap_ops ext4_iomap_ops;
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index d25179938487..917a052f9733 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -143,7 +143,7 @@ static void ext4_release_io_end(ext4_io_end_t *io_end)
 	struct bio *bio, *next_bio;
 
 	BUG_ON(!list_empty(&io_end->list));
-	BUG_ON(io_end->flag & EXT4_IO_END_UNWRITTEN);
+	BUG_ON(test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag));
 	WARN_ON(io_end->handle);
 
 	for (bio = io_end->bio; bio; bio = next_bio) {
@@ -180,7 +180,7 @@ static int ext4_end_io_end(ext4_io_end_t *io_end)
 	 * or stale data may be exposed.
 	 */
 	io_end->handle = NULL;  /* Following call will use up the handle */
-	if (unlikely(io_end->flag & EXT4_IO_END_FAILED)) {
+	if (unlikely(test_bit(EXT4_IO_END_FAILED, &io_end->flag))) {
 		ret = -EIO;
 		if (handle)
 			jbd2_journal_free_reserved(handle);
@@ -195,7 +195,7 @@ static int ext4_end_io_end(ext4_io_end_t *io_end)
 		mapping_set_error(inode->i_mapping, ret);
 
 	if (ret < 0 && !ext4_emergency_state(sb) &&
-	    io_end->flag & EXT4_IO_END_UNWRITTEN) {
+	    test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag)) {
 		ext4_msg(sb, KERN_EMERG,
 			 "failed to convert unwritten extents to written "
 			 "extents -- potential data loss!  "
@@ -232,11 +232,11 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
 
 static bool ext4_io_end_defer_completion(ext4_io_end_t *io_end)
 {
-	if (io_end->flag & EXT4_IO_END_UNWRITTEN &&
+	if (test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) &&
 	    !list_empty(&io_end->list_vec))
 		return true;
 	if (test_opt(io_end->inode->i_sb, DATA_ERR_ABORT) &&
-	    io_end->flag & EXT4_IO_END_FAILED &&
+	    test_bit(EXT4_IO_END_FAILED, &io_end->flag) &&
 	    !ext4_emergency_state(io_end->inode->i_sb))
 		return true;
 	return false;
@@ -251,8 +251,9 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)
 	unsigned long flags;
 
 	/* Only reserved conversions or pending IO errors will enter here. */
-	WARN_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION));
-	WARN_ON(io_end->flag & EXT4_IO_END_UNWRITTEN &&
+	WARN_ON(!test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) &&
+		!test_bit(EXT4_IO_END_FAILED, &io_end->flag));
+	WARN_ON(test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) &&
 		!io_end->handle && sbi->s_journal);
 	WARN_ON(!io_end->bio);
 
@@ -280,7 +281,8 @@ static int ext4_do_flush_completed_IO(struct inode *inode,
 
 	while (!list_empty(&unwritten)) {
 		io_end = list_entry(unwritten.next, ext4_io_end_t, list);
-		BUG_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION));
+		BUG_ON(!test_bit(EXT4_IO_END_UNWRITTEN, &io_end->flag) &&
+		       !test_bit(EXT4_IO_END_FAILED, &io_end->flag));
 		list_del_init(&io_end->list);
 
 		err = ext4_end_io_end(io_end);
@@ -366,7 +368,7 @@ static void ext4_end_bio(struct bio *bio)
 			     bio->bi_status, inode->i_ino,
 			     (unsigned long long)
 			     bi_sector >> (inode->i_blkbits - 9));
-		io_end->flag |= EXT4_IO_END_FAILED;
+		set_bit(EXT4_IO_END_FAILED, &io_end->flag);
 		mapping_set_error(inode->i_mapping,
 				blk_status_to_errno(bio->bi_status));
 	}
-- 
2.39.5

Re: [PATCH 6/6] ext4: use atomic bitops for io_end flags

[sashiko-bot]

> Before EXT4_IO_END_FAILED was added, the writeback thread set
> EXT4_IO_END_UNWRITTEN and final completion cleared it after the last
> io_end reference was dropped. Adding EXT4_IO_END_FAILED introduced a
> concurrent writer in failed-bio completion. Successful bio completions
> do not update the flags.
> 
> An io_end can cover already-mapped folios submitted before a later
> unwritten extent is mapped. An earlier bio can fail while writeback
> first sets UNWRITTEN. The non-atomic updates can then lose a bit:
> 
>   Bio completion                  Writeback thread
>   --------------                  ----------------
>   read flag = 0
>                                   attach reserved handle to io_end
>                                   flag |= UNWRITTEN
> [ ... ]
> Use atomic bit operations to fix the race.
> 
> Fixes: e856f93e0fb2 ("ext4: do not convert the unwritten extents if data writeback fails")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145241.2950379-1-sunjunchao@bytedance.com?part=6

Re: [PATCH 5/6] ext4: report unwritten extent conversion errors to fsync

[sashiko-bot]

> Buffered writeback can complete data I/O successfully but fail to convert
> unwritten extents, for example when allocating an extent path returns
> ENOMEM. ext4_end_io_end() logs the failure and returns an error, but the
> conversion worker discards that return value. As a result, fsync() can
> report success while the extents remain unwritten and direct reads return
> zeros instead of the data written.
> 
> Record the error in the inode mapping before ext4_release_io_end() ends
> folio writeback. This lets fsync() observe the error after waiting for
> writeback.
> 
> Fixes: b82e384c7bb9 ("ext4: optimize locking for end_io extent conversion")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022145241.2950379-1-sunjunchao@bytedance.com?part=5

Re: [PATCH 0/6] ext4: fix buffered writeback error handling

[Theodore Tso]

On Thu, Oct 22, 2026 at 10:52:35PM -0500, Julian Sun wrote:
> This series fixes error handling in ext4 buffered writeback. It prevents
> stale data exposure when mapping or io_end_vec allocation fails, reports
> unwritten conversion errors to fsync(), and preserves concurrent io_end
> flag updates.

Hi Julian,

Thanks for this series.  However, we have just landed a significant
change to the ext4 writeback to add support for fs/iomap's buffered
I/O.  It is currently not enabled by default, since there are a number
of features and mount options that aren't yet supported by the
buffered_iomap codepath.  (Thanks to Zhang Yi at Huawei for his work!)

I'd really appreciate it if you could rebase your patches on top of
the dev branch for the ext4 git tree (I've sent a pull request to
Linus so this should be in v7.4-rc1), and determine whether any of
these concerns are also applicable for the buffered_iomap codepath.

Many thanks,

						- Ted

Re: [PATCH 0/6] ext4: fix buffered writeback error handling

[Zhang Yi]

On 10/23/2026 12:05 AM, Theodore Tso wrote:
> On Thu, Oct 22, 2026 at 10:52:35PM -0500, Julian Sun wrote:
>> This series fixes error handling in ext4 buffered writeback. It prevents
>> stale data exposure when mapping or io_end_vec allocation fails, reports
>> unwritten conversion errors to fsync(), and preserves concurrent io_end
>> flag updates.
> 
> Hi Julian,
> 
> Thanks for this series.  However, we have just landed a significant
> change to the ext4 writeback to add support for fs/iomap's buffered
> I/O.  It is currently not enabled by default, since there are a number
> of features and mount options that aren't yet supported by the
> buffered_iomap codepath.  (Thanks to Zhang Yi at Huawei for his work!)

I'm glad to hear that the buffered_iomap codepath is going to be
upstreamed. I'd also like to thank Jan and Ojaswin for their careful
review and valuable suggestions, as well as everyone who has
contributed to this conversion in areas such as iomap and mm.

I will continue to put in effort going forward, and I hope to make
buffered_iomap the default path soon.

Cheers,
Yi.

> 
> I'd really appreciate it if you could rebase your patches on top of
> the dev branch for the ext4 git tree (I've sent a pull request to
> Linus so this should be in v7.4-rc1), and determine whether any of
> these concerns are also applicable for the buffered_iomap codepath.
> 
> Many thanks,
> 
> 						- Ted
> 

Re: [PATCH 0/6] ext4: fix buffered writeback error handling

[Ojaswin Mujoo]

On Fri, Oct 23, 2026 at 10:36:48AM +0800, Zhang Yi wrote:
> On 10/23/2026 12:05 AM, Theodore Tso wrote:
> > On Thu, Oct 22, 2026 at 10:52:35PM -0500, Julian Sun wrote:
> >> This series fixes error handling in ext4 buffered writeback. It prevents
> >> stale data exposure when mapping or io_end_vec allocation fails, reports
> >> unwritten conversion errors to fsync(), and preserves concurrent io_end
> >> flag updates.
> > 
> > Hi Julian,
> > 
> > Thanks for this series.  However, we have just landed a significant
> > change to the ext4 writeback to add support for fs/iomap's buffered
> > I/O.  It is currently not enabled by default, since there are a number
> > of features and mount options that aren't yet supported by the
> > buffered_iomap codepath.  (Thanks to Zhang Yi at Huawei for his work!)
> 
> I'm glad to hear that the buffered_iomap codepath is going to be
> upstreamed. I'd also like to thank Jan and Ojaswin for their careful
> review and valuable suggestions, as well as everyone who has
> contributed to this conversion in areas such as iomap and mm.

Hi Yi, thanks for your consistent effort over multiple years! Glad to
see it finally merged :)

Regards,
ojaswin
> 
> I will continue to put in effort going forward, and I hope to make
> buffered_iomap the default path soon.
> 
> Cheers,
> Yi.
> 
> > 
> > I'd really appreciate it if you could rebase your patches on top of
> > the dev branch for the ext4 git tree (I've sent a pull request to
> > Linus so this should be in v7.4-rc1), and determine whether any of
> > these concerns are also applicable for the buffered_iomap codepath.
> > 
> > Many thanks,
> > 
> > 						- Ted
> > 
>