Re: [PATCH] ext4: complete dropbehind writeback in task context

[Tal Zussman]

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@xxxxxxxxxxxx>
> 
> 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@xxxxxxx>
> 

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@xxxxxxxxxxxx>
>> 
> -- 
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
>