From: Jens Axboe <axboe@kernel.dk>
To: Kent Overstreet <kent.overstreet@linux.dev>
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/7] block layer patches for bcachefs
Date: Tue, 30 May 2023 10:50:55 -0600 [thread overview]
Message-ID: <2a56b6d4-5f24-9738-ec83-cefb20998c8c@kernel.dk> (raw)
In-Reply-To: <ZHYfGvPJFONm58dA@moria.home.lan>
On 5/30/23 10:06?AM, Kent Overstreet wrote:
> On Tue, May 30, 2023 at 08:22:50AM -0600, Jens Axboe wrote:
>> On 5/26/23 2:44?PM, Kent Overstreet wrote:
>>> On Fri, May 26, 2023 at 08:35:23AM -0600, Jens Axboe wrote:
>>>> On 5/25/23 3:48?PM, Kent Overstreet wrote:
>>>>> Jens, here's the full series of block layer patches needed for bcachefs:
>>>>>
>>>>> Some of these (added exports, zero_fill_bio_iter?) can probably go with
>>>>> the bcachefs pull and I'm just including here for completeness. The main
>>>>> ones are the bio_iter patches, and the __invalidate_super() patch.
>>>>>
>>>>> The bio_iter series has a new documentation patch.
>>>>>
>>>>> I would still like the __invalidate_super() patch to get some review
>>>>> (from VFS people? unclear who owns this).
>>>>
>>>> I wanted to check the code generation for patches 4 and 5, but the
>>>> series doesn't seem to apply to current -git nor my for-6.5/block.
>>>> There's no base commit in this cover letter either, so what is this
>>>> against?
>>>>
>>>> Please send one that applies to for-6.5/block so it's a bit easier
>>>> to take a closer look at this.
>>>
>>> Here you go:
>>> git pull https://evilpiepirate.org/git/bcachefs.git block-for-bcachefs
>>
>> Thanks
>>
>> The re-exporting of helpers is somewhat odd - why is bcachefs special
>> here and needs these, while others do not?
>
> It's not iomap based.
>
>> But the main issue for me are the iterator changes, which mostly just
>> seems like unnecessary churn. What's the justification for these? The
>> commit messages don;t really have any. Doesn't seem like much of a
>> simplification, and in fact it's more code than before and obviously
>> more stack usage as well.
>
> I need bio_for_each_folio().
>
> The approach taken by the bcachefs IO paths is to first build up bios,
> then walk the extents btree to determine where to send them, splitting
> as needed.
>
> For reading into the page cache we additionally need to initialize our
> private state based on what we're reading from that says what's on
> disk (unallocated, reservation, or normal allocation) and how many
> replicas. This is used for both i_blocks accounting and for deciding
> when we need to get a disk reservation. Since we're doing this post
> split, it needs bio_for_each_folio, not the _all variant.
>
> Yes, the iterator changes are a bit more code - but it's split up into
> better helpers now, the pointer arithmetic before was a bit dense; I
> found the result to be more readable. I'm surprised at more stack
> usage; I would have expected _less_ for bio_for_each_page_all() since
> it gets rid of a pointer into the bvec_iter_all. How did you measure
> that?
Sorry typo, I meant text. Just checked stack and it looks identical, but
things like blk-map grows ~6% more text, and bio ~3%. Didn't check all
of them, but at least those two are consistent across x86-64 and
aarch64. Ditto on the data front. Need to take a closer look at where
exactly that is coming from, and what that looks like.
--
Jens Axboe
next prev parent reply other threads:[~2023-05-30 16:51 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-25 21:48 [PATCH 0/7] block layer patches for bcachefs Kent Overstreet
2023-05-25 21:48 ` [PATCH 1/7] block: Add some exports " Kent Overstreet
2023-05-25 21:48 ` [PATCH 2/7] block: Allow bio_iov_iter_get_pages() with bio->bi_bdev unset Kent Overstreet
2023-05-25 21:48 ` [PATCH 3/7] block: Bring back zero_fill_bio_iter Kent Overstreet
2023-05-25 21:48 ` [PATCH 4/7] block: Rework bio_for_each_segment_all() Kent Overstreet
2023-05-25 21:48 ` [PATCH 5/7] block: Rework bio_for_each_folio_all() Kent Overstreet
2023-05-26 0:36 ` Dave Chinner
2023-05-26 0:50 ` Kent Overstreet
2023-05-25 21:48 ` [PATCH 6/7] block: Add documentation for bio iterator macros Kent Overstreet
2023-05-25 21:48 ` [PATCH 7/7] block: Don't block on s_umount from __invalidate_super() Kent Overstreet
2023-05-26 14:35 ` [PATCH 0/7] block layer patches for bcachefs Jens Axboe
2023-05-26 20:44 ` Kent Overstreet
2023-05-30 14:22 ` Jens Axboe
2023-05-30 16:06 ` Kent Overstreet
2023-05-30 16:50 ` Jens Axboe [this message]
2023-05-30 23:30 ` Kent Overstreet
2023-06-04 23:38 ` Kent Overstreet
2023-06-05 16:49 ` Jens Axboe
2023-06-05 21:26 ` Kent Overstreet
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=2a56b6d4-5f24-9738-ec83-cefb20998c8c@kernel.dk \
--to=axboe@kernel.dk \
--cc=kent.overstreet@linux.dev \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).