From: Dominique Martinet <asmadeus@codewreck.org>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: "Marek Behún" <kabel@kernel.org>, "Qu Wenruo" <wqu@suse.com>,
linux-btrfs@vger.kernel.org, u-boot@lists.denx.de,
"Dominique Martinet" <dominique.martinet@atmark-techno.com>
Subject: Re: [PATCH U-BOOT 2/3] btrfs: btrfs_file_read: allow opportunistic read until the end
Date: Tue, 18 Apr 2023 11:41:27 +0900 [thread overview]
Message-ID: <ZD4DV49fqKmPjMjU@codewreck.org> (raw)
In-Reply-To: <6c82ddd9-0e3d-4213-5cd3-af7ad69ebe48@gmx.com>
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?
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:
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.
--
Dominique Martinet | Asmadeus
next prev parent reply other threads:[~2023-04-18 14:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-18 1:17 [PATCH U-BOOT 0/3] btrfs: fix and improve read code Dominique Martinet
2023-04-18 1:17 ` [PATCH U-BOOT 1/3] btrfs: fix offset within btrfs_read_extent_reg() Dominique Martinet
2023-04-18 1:58 ` Qu Wenruo
2023-04-18 2:05 ` Dominique Martinet
2023-04-18 2:16 ` Qu Wenruo
2023-04-18 2:20 ` Qu Wenruo
2023-04-18 1:17 ` [PATCH U-BOOT 2/3] btrfs: btrfs_file_read: allow opportunistic read until the end Dominique Martinet
2023-04-18 2:02 ` Qu Wenruo
2023-04-18 2:41 ` Dominique Martinet [this message]
2023-04-18 2:53 ` Qu Wenruo
2023-04-18 3:07 ` Dominique Martinet
2023-04-18 3:21 ` Qu Wenruo
2023-04-18 3:53 ` Dominique Martinet
2023-04-18 5:17 ` Dominique Martinet
2023-04-18 7:15 ` Qu Wenruo
2023-04-18 1:17 ` [PATCH U-BOOT 3/3] btrfs: btfs_file_read: zero trailing data if no extent was found Dominique Martinet
2023-04-18 2:04 ` Qu Wenruo
2023-04-18 2:43 ` Dominique Martinet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZD4DV49fqKmPjMjU@codewreck.org \
--to=asmadeus@codewreck.org \
--cc=dominique.martinet@atmark-techno.com \
--cc=kabel@kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=quwenruo.btrfs@gmx.com \
--cc=u-boot@lists.denx.de \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox