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 12:07:53 +0900 [thread overview]
Message-ID: <ZD4Jiagu0sWIPZDa@codewreck.org> (raw)
In-Reply-To: <fca50995-0745-4374-a64a-40378ea95262@gmx.com>
Qu Wenruo wrote on Tue, Apr 18, 2023 at 10:53:41AM +0800:
> > 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.
Ok, will give it a try.
I'll want to test this a bit so might take a day or two as I have other
work to finish first.
> > 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, was just thinking the leading part being a separate loop doesn't
seem to make sense either as the code shouldn't care about sector size
alignemnt but about full extents.
If the main loop handles everything correctly then the leading if can
also be removed along with "read_and_truncate_page" that would no longer
be used.
I'll give this a try as well and report back.
> No matter if it's aligned or not, as btrfs would convert any unaligned part
> into aligned read.
Yes I don't see where it would fail (except that my board does crash),
I guess that at this point I should spend some time building a qemu
u-boot and hooking up gdb will be faster..
> 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.
Yes, sure -- removing the if is easy, I'd just rather not make it fail
for someone else :)
--
Dominique Martinet | Asmadeus
next prev parent reply other threads:[~2023-04-18 14:38 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
2023-04-18 2:53 ` Qu Wenruo
2023-04-18 3:07 ` Dominique Martinet [this message]
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=ZD4Jiagu0sWIPZDa@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