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

[Christoph Hellwig <hch@lst.de>]

On Mon, Jun 20, 2022 at 12:34:48PM +0300, Nikolay Borisov wrote:
> write to a RAID1 fs. So we have to do 2 writes - 1 of the orig bio, 1 of a 
> stripe bio so we make 1 clone of the orig bio, call bio_inc_remaining and 
> orig_bio->__bi_remaining is 2. So the 2 bios get submitted.

Yes.

> If the stripe bio is completed first 'if (bio != orig_bio) {' check in 
> btrfs_end_bio ensures that the function is really a noop. Subsequently when 
> orig_bio is completed it proceeds to executed:
>
> orig_bio->bi_end_io(orig_bio);

Yes.

> Consider the same scenario, but this time if orig_bio is completed first 
> then we proceed directly calling orig_bio->bi_end_io(orig_bio);

No, it doesn't.  When the low-level block driver calls bio_endio for
orig_bio, bio_endio calls bio_remaining_done which does an atomic
atomic_dec_and_test, which returns false because __bi_remaining is still
1 after the call.  That causes bio_endio to just return and not call
->bi_end_io.  Only when the cloned bio completes and calls bio_endio on
orig_bio again (from btrfs_end_bio) that atomic_dec_and_test now
returns true and ->bi_end_io is called on the orig_bio.

> bypassing 
> bio_endio and the __bi_remaining checks etc. So shouldn't this
> orig_bio->bi_end_io(orig_bio);  be converted to bio_endio call ?

The orig_bio->bi_end_io(orig_bio) from btrfs_end_bio and
btrfs_end_bio_work is weird.  It used to be a bio_endio call before
this series, but that is actualy wrong as the bio has already been
completed.  The bio is done and we just need to propagate it to the
submitter of the original btrfs_bio.  The right thing would be to
call this as:

	bioc->end_io(orig_bio);

but doing so would break the repair code that has way too much
magic behavior for its own sake.