Re: [PATCH RFC v2 05/12] btrfs: add a helper to queue a corrupted sector for read repair

[Christoph Hellwig <hch@infradead.org>]

On Thu, Apr 28, 2022 at 01:20:37PM +0800, Qu Wenruo wrote:
> This function will get called very frequently, and I really want to avoid
> doing such re-search every time from the beginning of the original bio.
> 
> Maybe we can cache a bvec_iter and using the bi_size to check if the target
> offset is still beyond us (then advance), or re-wind and re-search from the
> beginning.
> 
> I guess there is no existing helper to do the same work, right?

It is basically impossible to review this because you just add a
standalone static helper without the callers.  Please split the
work into logical chunks that actually are reviewable.  Yes, that will
be a pretty large patch, but that's still much better than having to
jump around hal a dozen ones.

No, there is no way to efficiently look up what bvec in a bio some
offset falls into, because the bvecs are variable length.

It seems like the caller (at least the one added a little later, not
sure if there are more) is iterating over the bitmap and then calls
this for every bit set.  So for that you're much better off making
the __bio_for_each_segment the main loop, and then at the beginning of
the loop checking the bitmap if we need to handle this sector.


> > +	struct bio_list read_bios;

I'd just calls this bios.  Obviously they are used for reading here.

Also please be very careful about dead locks.  The mempool for the
btrfs_bios is small right now, you need to size it up by the
largerst number of bios that can be on this list.