Re: [PATCH 8/8] btrfs: use btrfs_bio_for_each_sector in btrfs_check_read_dio_bio

Christoph Hellwig
2022-05-24T12:08:55Z

On Tue, May 24, 2022 at 04:21:38PM +0800, Qu Wenruo wrote:
>> The things like resetting initial_mirror, making the naming "initial"
>> meaningless.
>> And the reset on the length part is also very quirky.
>
> In fact, if you didn't do the initial_mirror and length change (which is
> a big disaster of readability, to change iterator in a loop, at least to
> me),

So what is the big problem there? Do I need more extensive documentation
or as there anything in this concept that is just too confusing.

> and rely on the VFS re-read behavior to fall back to sector by
> secot read, I would call it better readability...

I don't think relying on undocumented VFS behavior is a good idea. It
will also not work for direct I/O if any single direct I/O bio has
ever more than a single page, which is something btrfs badly needs
if it wants to get any kind of performance out of direct I/O.