Re: [PATCH RFC v4 1/3] block: add BIO_COMPLETE_IN_TASK for task-context completion

[Tal Zussman <tz2294@columbia.edu>]

On 3/27/26 2:01 AM, Christoph Hellwig wrote:
> On Wed, Mar 25, 2026 at 02:43:00PM -0400, Tal Zussman wrote:
>> +static void bio_complete_work_fn(struct work_struct *w)
>> +{
>> +	struct bio_complete_batch *batch;
>> +	struct bio_list list;
>> +
>> +again:
>> +	local_lock_irq(&bio_complete_batch.lock);
>> +	batch = this_cpu_ptr(&bio_complete_batch);
>> +	list = batch->list;
>> +	bio_list_init(&batch->list);
>> +	local_unlock_irq(&bio_complete_batch.lock);
>> +
>> +	while (!bio_list_empty(&list)) {
>> +		struct bio *bio = bio_list_pop(&list);
>> +		bio->bi_end_io(bio);
>> +	}
> 
> bio_list_pop already does a NULL check, so this could be:
> 
> 	while ((bio = bio_list_pop(&batch->list)))
> 		bio->bi_end_io(bio);
> 
> In fact that same pattern is repeated later, so maybe just add a helper
> for it?  But I think Dave's idea of just using a llist (and adding a
> new llist member to the bio for this) seems sensible.  Just don't forget
> the llist_reverse_order call to avoid reordering.

Will switch to llist.

>> +
>> +	local_lock_irq(&bio_complete_batch.lock);
>> +	batch = this_cpu_ptr(&bio_complete_batch);
>> +	if (!bio_list_empty(&batch->list)) {
>> +		local_unlock_irq(&bio_complete_batch.lock);
>> +
>> +		if (!need_resched())
>> +			goto again;
>> +
>> +		schedule_work_on(smp_processor_id(), &batch->work);
>> +		return;
>> +	}
>> +	local_unlock_irq(&bio_complete_batch.lock);
> 
> I don't really understand this requeue logic.  Can you explain it?

If new bios were queued while we were processing this batch, we loop back
and process them, if we don't need to reschedule. If we do need to
reschedule, we re-queue the work item and return.

>> +	schedule_work_on(smp_processor_id(), &batch->work);
> 
> We'll probably want a dedicated workqueue here to avoid deadlocks
> vs other system wq uses.
> 
>> +static int bio_complete_batch_cpu_dead(unsigned int cpu)
>> +{
>> +	struct bio_complete_batch *batch = per_cpu_ptr(&bio_complete_batch, cpu);
> 
> Overly long line.
> 

Will fix.

Thanks,
Tal