[PATCH U-BOOT 2/3] btrfs: btrfs_file_read: allow opportunistic read until the end

[Qu Wenruo]

On 2023/4/18 10:41, Dominique Martinet wrote:
> Qu Wenruo wrote on Tue, Apr 18, 2023 at 10:02:00AM +0800:
>>>    	/* Read the tailing unaligned part*/
>>
>> Can we remove this part completely?
>>
>> IIRC if we read until the target end, the unaligned end part can be
>> completely removed then.
> 
> The "Read the aligned part" loop stops at aligned_end:
>> while (cur < aligned_end)
> 
> So it should be possible that the last aligned extent we consider does
> not contain data until the end, e.g. an offset that ends with the
> aligned end:
> 
> 0            4096    4123
> [extent1-----|extent2]
> 
> In this case the main loop will only read extent1 and we need the
> "trailing unaligned part" if for extent2.
> 
> I have a feeling the loop could just be updated to go to the end
> `while (cur < end)` as it doesn't seem to care about the end
> alignment... Should I update v2 to do that instead?

Yeah, it would be very awesome if you can remove the tailing part 
completely.

> 
> 
> 
> This made me look at the "Read out the leading unaligned part" initial
> part, and its check only looks at the sector size but it probably has
> the same problem -- we want to make sure we read any leftover from a
> previous extent e.g. this file:

If you're talking about the same problem mentioned in patch 1, then yes, 
any read in the middle of an extent would cause problems.

No matter if it's aligned or not, as btrfs would convert any unaligned 
part into aligned read.

But if we fix the bug you mentioned, I see nothing can going wrong.

Unaligned read is converted into aligned one (then copy the target range 
into the dest buf), and aligned part (including the tailing unaligned 
part) would be properly handled then.

> 0              4096       8192
> [extent1------------------|extent2]
> and reading from 4k with a sectorsize of 4k is probably bad will enter
> the aligned main loop right away...
> And I think that'll fail?...
> Actually not quite sure what's expecting what to be aligned in there,
> but I just tried some partial reads from non-zero offsets and my board
> resets almost all the time so I guess I've found something else to dig
> into.
> This isn't a priority for me right now but I'll look a bit more when I
> have more time if you haven't first.
> 
My initial plan is to merge the tailing part into the aligned loop, but 
since you're already working in this part, feel free to do it.

And I'm very happy to provide any help on this.

Thanks,
Qu