Re: [PATCH 10/10] btrfs: remove bioc->stripes_pending

[Christoph Hellwig <hch@lst.de>]

On Mon, Jun 20, 2022 at 11:18:03AM +0300, Nikolay Borisov wrote:
> What if the the currently completed stripe bio is the last - then bio_endio 
> would be called for orig_bio, which will executed bi_end_io() in softirq 
> context, but for reads we want to execute this in process context (as per 
> the below code) ?

> The old code guaranteed that btrfs_end_bioc() is executed when the last 
> stripe bio was completed. With this new scheme, say we have 3 bios - 2 
> cloned, 1 being the orig, what guarantees that the orig_bio won't be 
> finished before the remaining 2 (or 1) cloned/stripe bios thus calling 
> btrfs_end_bio() on orig bio with __bi_remaining potentially being 2 or 1 
> and finally calling orig_bio->bi_end_io() again with __bi_remaining being 
> elevated?

This is what the bio_inc_remaining after the bio_alloc_clone takes care
of.  With that the remaining count in the original btrfs_bio is
incremented, which ensures that ->end_io for the originl bio is only
called when all other bios and the original one have completed.

Take a look at bio_endio and bio_remaining_done for the glory details.

>>   	if (clone) {
>> -		bio = bio_alloc_clone(dev->bdev, orig_bio, GFP_NOFS, &fs_bio_set);
>> +		bio = bio_alloc_clone(NULL, orig_bio, GFP_NOFS, &fs_bio_set);
>> +		bio_inc_remaining(orig_bio);
>
> When cloning why aren't you passing dev->bdev but instead set it after the 
> checks via bio_set_dev ? Is it because of the
>
> if (bio->bi_bdev && bio_flagged(bio, BIO_TRACE_COMPLETION))
>
> check inside bio_endio in case bio_io_error is called in submit_stripe_bio?

It is because we don't know if we have a valid bdev until after
the checks a few line down that call bio_io_error.  We need those
checks to be done later now so that all error handling goes through
the bio end_io handler.