* Re: [PATCH] ext4: complete dropbehind writeback in task context
2026-10-12 18:04 [PATCH] ext4: complete dropbehind writeback in task context Tal Zussman
@ 2026-10-12 18:19 ` sashiko-bot
2026-10-13 11:59 ` Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-12 18:19 UTC (permalink / raw)
To: Tal Zussman; +Cc: tytso, linux-ext4
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: data race on io_end->flag corrupts UNWRITTEN and FAILED state
--
commit 578221fefbc4e5e8f3b366b7ce8ee33f43bb2e34
Author: Tal Zussman <tz2294@columbia.edu>
ext4: complete dropbehind writeback in task context
This commit introduces task-context bio completion for bios carrying
dropbehind folios. It sets BIO_COMPLETE_IN_TASK on these bios during
io_submit_add_bh() to ensure their invalidation runs from a worker rather
than interrupt context.
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 734d856a6d478..e556598ce9e14 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
[ ... ]
> @@ -458,6 +458,16 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
> io_submit_init_bio(io, inode, folio, bh);
> if (!bio_add_folio(io->io_bio, folio, bh->b_size, bh_offset(bh)))
> goto submit_and_retry;
> +
> + /*
> + * Dropbehind folios can only be invalidated in task context. An
> + * io_end with unwritten extents is completed from ext4's workqueue
> + * anyway, so only ask the block layer to defer the others.
> + */
> + if (folio_test_dropbehind(folio) &&
> + !(io->io_end->flag & EXT4_IO_END_UNWRITTEN))
> + bio_set_flag(io->io_bio, BIO_COMPLETE_IN_TASK);
> +
[Severity: High]
Does this lockless read of io_end->flag introduce a data race that could lead
to data loss?
During submission, the ext4_writepages thread can map multiple extents that
share a single ext4_io_end. If an early bio completes with an error while the
submission thread is still mapping a subsequent unwritten extent,
ext4_end_bio() can set EXT4_IO_END_FAILED concurrently:
fs/ext4/page-io.c:ext4_end_bio() {
...
io_end->flag |= EXT4_IO_END_FAILED;
...
}
Meanwhile, the submission thread might be setting EXT4_IO_END_UNWRITTEN:
fs/ext4/ext4.h:ext4_set_io_unwritten_flag() {
...
io_end->flag |= EXT4_IO_END_UNWRITTEN;
...
}
Since both paths use non-atomic bitwise OR without a shared lock, reading
the flag here in io_submit_add_bh() appears to rely on a racy state.
If the UNWRITTEN flag is lost due to a read-modify-write race,
ext4_io_end_defer_completion() will return false and Ext4 could drop the
io_end without converting the unwritten extents on disk. Can this lead to
silent data loss where written data reads as zeroes?
Could this lockless check be made safe against concurrent bio completions, or
perhaps atomic bitops should be used for io_end->flag?
> wbc_account_cgroup_owner(io->io_wbc, folio, bh->b_size);
> io->io_next_block++;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-ext4-dontcache-complete-in-task-v1-1-6ac801557041@columbia.edu?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ext4: complete dropbehind writeback in task context
2026-10-12 18:04 [PATCH] ext4: complete dropbehind writeback in task context Tal Zussman
2026-10-12 18:19 ` sashiko-bot
@ 2026-10-13 11:59 ` Christoph Hellwig
2026-10-13 17:36 ` Tal Zussman
2026-10-13 12:47 ` Jan Kara
2026-10-13 15:17 ` Baokun Li
3 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2026-10-13 11:59 UTC (permalink / raw)
To: Tal Zussman
Cc: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, Taotao Chen,
Christian Brauner, Christoph Hellwig, Jens Axboe,
Matthew Wilcox (Oracle), linux-ext4, linux-kernel
I wonder if we can add a test to xfstests using say the mincore command
in xfs_io to check if the pages actually get dropped after a trivial
RWF_DONTCACHE write to verify this behavior?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext4: complete dropbehind writeback in task context
2026-10-13 11:59 ` Christoph Hellwig
@ 2026-10-13 17:36 ` Tal Zussman
0 siblings, 0 replies; 7+ messages in thread
From: Tal Zussman @ 2026-10-13 17:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, Taotao Chen,
Christian Brauner, Jens Axboe, Matthew Wilcox (Oracle),
linux-ext4, linux-kernel
On 10/13/26 7:59 AM, Christoph Hellwig wrote:
> I wonder if we can add a test to xfstests using say the mincore command
> in xfs_io to check if the pages actually get dropped after a trivial
> RWF_DONTCACHE write to verify this behavior?
>
Looks doable, I can take a stab at that in the next week or so.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext4: complete dropbehind writeback in task context
2026-10-12 18:04 [PATCH] ext4: complete dropbehind writeback in task context Tal Zussman
2026-10-12 18:19 ` sashiko-bot
2026-10-13 11:59 ` Christoph Hellwig
@ 2026-10-13 12:47 ` Jan Kara
2026-10-13 17:32 ` Tal Zussman
2026-10-13 15:17 ` Baokun Li
3 siblings, 1 reply; 7+ messages in thread
From: Jan Kara @ 2026-10-13 12:47 UTC (permalink / raw)
To: Tal Zussman
Cc: Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, Taotao Chen,
Christian Brauner, Christoph Hellwig, Jens Axboe,
Matthew Wilcox (Oracle), linux-ext4, linux-kernel
On Mon 12-10-26 14:04:37, Tal Zussman wrote:
> ext4_end_bio() ends writeback directly from bio completion, which
> usually runs in interrupt context, unless the io_end has unwritten
> extents to convert and is completed from ext4's workqueue.
> folio_end_dropbehind() only invalidates the folios of an RWF_DONTCACHE
> write when it runs in task context, and silently skips them otherwise.
> So on ext4, dontcache writes that overwrite blocks that are already
> written, or that allocate blocks without unwritten extents, leave their
> folios in the page cache. Dropping them was best effort so far, as there
> was no way to get task context from bio completion.
>
> Use the task-context bio completion infrastructure instead: set
> BIO_COMPLETE_IN_TASK on bios that carry dropbehind folios in
> io_submit_add_bh(), so that bio_endio() runs their completion from a
> worker. ext4's iomap buffered I/O path already gets this from iomap, so
> only the buffer_head based writeback is affected. Skip the flag when the
> io_end already has unwritten extents, as ext4 then defers the completion
> itself. A later extent can still make the io_end unwritten after an
> earlier bio was built. That bio's completion is then deferred twice,
> which is not optimal, but harmless.
>
> This changes behavior only for bios that carry dropbehind folios: their
> completion now runs from a worker instead of in interrupt context.
>
> On the unpatched kernel a dontcache overwrite of already-written blocks,
> or a dontcache write into a new file on a dioread_lock mount, leaves
> every folio in the page cache (16384/16384 for a 64M file). With the
> patch, none remain in the page cache. A dontcache write into a new file
> with unwritten extents already drops its folios on both kernels, as
> expected, and a plain write keeps them.
>
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
I guess this is OK as a short term fix but having two different offloading
mechanisms for IO completion is just asking for trouble and we should use
BIO_COMPLETE_IN_TASK when unwritten extent conversion is needed as well.
But for this fix feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/page-io.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 734d856a6d47..e556598ce9e1 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -458,6 +458,16 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
> io_submit_init_bio(io, inode, folio, bh);
> if (!bio_add_folio(io->io_bio, folio, bh->b_size, bh_offset(bh)))
> goto submit_and_retry;
> +
> + /*
> + * Dropbehind folios can only be invalidated in task context. An
> + * io_end with unwritten extents is completed from ext4's workqueue
> + * anyway, so only ask the block layer to defer the others.
> + */
> + if (folio_test_dropbehind(folio) &&
> + !(io->io_end->flag & EXT4_IO_END_UNWRITTEN))
> + bio_set_flag(io->io_bio, BIO_COMPLETE_IN_TASK);
> +
> wbc_account_cgroup_owner(io->io_wbc, folio, bh->b_size);
> io->io_next_block++;
> }
>
> ---
> base-commit: 35aefefd7aba0801bab635b11943f932626be3b2
> change-id: 20260924-ext4-dontcache-complete-in-task-2340ed5f3114
>
> Best regards,
> --
> Tal Zussman <tz2294@columbia.edu>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext4: complete dropbehind writeback in task context
2026-10-13 12:47 ` Jan Kara
@ 2026-10-13 17:32 ` Tal Zussman
0 siblings, 0 replies; 7+ messages in thread
From: Tal Zussman @ 2026-10-13 17:32 UTC (permalink / raw)
To: Jan Kara
Cc: Theodore Ts'o, Andreas Dilger, Baokun Li, Ojaswin Mujoo,
Ritesh Harjani (IBM), Zhang Yi, Taotao Chen, Christian Brauner,
Christoph Hellwig, Jens Axboe, Matthew Wilcox (Oracle),
linux-ext4, linux-kernel
On 10/13/26 8:47 AM, Jan Kara wrote:
> On Mon 12-10-26 14:04:37, Tal Zussman wrote:
>> ext4_end_bio() ends writeback directly from bio completion, which
>> usually runs in interrupt context, unless the io_end has unwritten
>> extents to convert and is completed from ext4's workqueue.
>> folio_end_dropbehind() only invalidates the folios of an RWF_DONTCACHE
>> write when it runs in task context, and silently skips them otherwise.
>> So on ext4, dontcache writes that overwrite blocks that are already
>> written, or that allocate blocks without unwritten extents, leave their
>> folios in the page cache. Dropping them was best effort so far, as there
>> was no way to get task context from bio completion.
>>
>> Use the task-context bio completion infrastructure instead: set
>> BIO_COMPLETE_IN_TASK on bios that carry dropbehind folios in
>> io_submit_add_bh(), so that bio_endio() runs their completion from a
>> worker. ext4's iomap buffered I/O path already gets this from iomap, so
>> only the buffer_head based writeback is affected. Skip the flag when the
>> io_end already has unwritten extents, as ext4 then defers the completion
>> itself. A later extent can still make the io_end unwritten after an
>> earlier bio was built. That bio's completion is then deferred twice,
>> which is not optimal, but harmless.
>>
>> This changes behavior only for bios that carry dropbehind folios: their
>> completion now runs from a worker instead of in interrupt context.
>>
>> On the unpatched kernel a dontcache overwrite of already-written blocks,
>> or a dontcache write into a new file on a dioread_lock mount, leaves
>> every folio in the page cache (16384/16384 for a 64M file). With the
>> patch, none remain in the page cache. A dontcache write into a new file
>> with unwritten extents already drops its folios on both kernels, as
>> expected, and a plain write keeps them.
>>
>> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
>
> I guess this is OK as a short term fix but having two different offloading
> mechanisms for IO completion is just asking for trouble and we should use
> BIO_COMPLETE_IN_TASK when unwritten extent conversion is needed as well.
> But for this fix feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
Thanks! Agreed on the above. I saw that there was a TODO added in
ext4_iomap_writeback_submit() from the new iomap conversion series to
"switch all defer handlers to the generic bio complete in task framework",
so I assumed that'll be taken care of as part of the ongoing conversion.
- Tal
> Honza
>
>> ---
>> fs/ext4/page-io.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
>> index 734d856a6d47..e556598ce9e1 100644
>> --- a/fs/ext4/page-io.c
>> +++ b/fs/ext4/page-io.c
>> @@ -458,6 +458,16 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
>> io_submit_init_bio(io, inode, folio, bh);
>> if (!bio_add_folio(io->io_bio, folio, bh->b_size, bh_offset(bh)))
>> goto submit_and_retry;
>> +
>> + /*
>> + * Dropbehind folios can only be invalidated in task context. An
>> + * io_end with unwritten extents is completed from ext4's workqueue
>> + * anyway, so only ask the block layer to defer the others.
>> + */
>> + if (folio_test_dropbehind(folio) &&
>> + !(io->io_end->flag & EXT4_IO_END_UNWRITTEN))
>> + bio_set_flag(io->io_bio, BIO_COMPLETE_IN_TASK);
>> +
>> wbc_account_cgroup_owner(io->io_wbc, folio, bh->b_size);
>> io->io_next_block++;
>> }
>>
>> ---
>> base-commit: 35aefefd7aba0801bab635b11943f932626be3b2
>> change-id: 20260924-ext4-dontcache-complete-in-task-2340ed5f3114
>>
>> Best regards,
>> --
>> Tal Zussman <tz2294@columbia.edu>
>>
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ext4: complete dropbehind writeback in task context
2026-10-12 18:04 [PATCH] ext4: complete dropbehind writeback in task context Tal Zussman
` (2 preceding siblings ...)
2026-10-13 12:47 ` Jan Kara
@ 2026-10-13 15:17 ` Baokun Li
3 siblings, 0 replies; 7+ messages in thread
From: Baokun Li @ 2026-10-13 15:17 UTC (permalink / raw)
To: Tal Zussman
Cc: Theodore Ts'o, Jan Kara, Ojaswin Mujoo, Ritesh Harjani (IBM),
Zhang Yi, Andreas Dilger, Christoph Hellwig, Jens Axboe,
Taotao Chen, Matthew Wilcox (Oracle), linux-ext4, linux-kernel,
Christian Brauner
On 2026/10/13 02:04, Tal Zussman wrote:
> ext4_end_bio() ends writeback directly from bio completion, which
> usually runs in interrupt context, unless the io_end has unwritten
> extents to convert and is completed from ext4's workqueue.
> folio_end_dropbehind() only invalidates the folios of an RWF_DONTCACHE
> write when it runs in task context, and silently skips them otherwise.
> So on ext4, dontcache writes that overwrite blocks that are already
> written, or that allocate blocks without unwritten extents, leave their
> folios in the page cache. Dropping them was best effort so far, as there
> was no way to get task context from bio completion.
>
> Use the task-context bio completion infrastructure instead: set
> BIO_COMPLETE_IN_TASK on bios that carry dropbehind folios in
> io_submit_add_bh(), so that bio_endio() runs their completion from a
> worker. ext4's iomap buffered I/O path already gets this from iomap, so
> only the buffer_head based writeback is affected. Skip the flag when the
> io_end already has unwritten extents, as ext4 then defers the completion
> itself. A later extent can still make the io_end unwritten after an
> earlier bio was built. That bio's completion is then deferred twice,
> which is not optimal, but harmless.
>
> This changes behavior only for bios that carry dropbehind folios: their
> completion now runs from a worker instead of in interrupt context.
>
> On the unpatched kernel a dontcache overwrite of already-written blocks,
> or a dontcache write into a new file on a dioread_lock mount, leaves
> every folio in the page cache (16384/16384 for a 64M file). With the
> patch, none remain in the page cache. A dontcache write into a new file
> with unwritten extents already drops its folios on both kernels, as
> expected, and a plain write keeps them.
>
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
We should add a Fixes tag:
Fixes: ae21c0c0ac56 ("ext4: support uncached buffered I/O")
Otherwise looks good to me:
Reviewed-by: Baokun Li <libaokun@linux.alibaba.com>
> ---
> fs/ext4/page-io.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 734d856a6d47..e556598ce9e1 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -458,6 +458,16 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
> io_submit_init_bio(io, inode, folio, bh);
> if (!bio_add_folio(io->io_bio, folio, bh->b_size, bh_offset(bh)))
> goto submit_and_retry;
> +
> + /*
> + * Dropbehind folios can only be invalidated in task context. An
> + * io_end with unwritten extents is completed from ext4's workqueue
> + * anyway, so only ask the block layer to defer the others.
> + */
> + if (folio_test_dropbehind(folio) &&
> + !(io->io_end->flag & EXT4_IO_END_UNWRITTEN))
> + bio_set_flag(io->io_bio, BIO_COMPLETE_IN_TASK);
> +
> wbc_account_cgroup_owner(io->io_wbc, folio, bh->b_size);
> io->io_next_block++;
> }
>
> ---
> base-commit: 35aefefd7aba0801bab635b11943f932626be3b2
> change-id: 20260924-ext4-dontcache-complete-in-task-2340ed5f3114
>
> Best regards,
> --
> Tal Zussman <tz2294@columbia.edu>
^ permalink raw reply [flat|nested] 7+ messages in thread