Re: [PATCH 0/7] block layer patches for bcachefs

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: Mon, 5 Jun 2023 10:49:37 -0600	[thread overview]
Message-ID: <b1b43d30-8c7c-1a71-0ead-8b967b8af0a4@kernel.dk> (raw)
In-Reply-To: <ZH0gjyuBgYzqhZh7@moria.home.lan>

On 6/4/23 5:38?PM, Kent Overstreet wrote:
> On Tue, May 30, 2023 at 10:50:55AM -0600, Jens Axboe wrote:
>> 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.
> 
> A good chunk of that is because I added warnings and assertions for
> e.g. running past the end of the bvec array. These bugs are rare and
> shouldn't happen with normal iterator usage (e.g. the bio_for_each_*
> macros), but I'd like to keep them as a debug mode thing.
> 
> But we don't yet have CONFIG_BLOCK_DEBUG - perhaps we should.

Let's split those out then, especially as we don't have a BLOCK_DEBUG
option right now.

> With those out, I see a code size decrease in bio.c, which makes sense -
> gcc ought to be able to generate slightly better code when it's dealing
> with pure values, provided everything is inlined and there's no aliasing
> considerations.
> 
> Onto blk-map.c:
> 
> bio_copy_kern_endio_read() increases in code size, but if I change
> memcpy_from_bvec() to take the bvec by val instead of by ref it's
> basically the same code size. There's no disadvantage to changing
> memcpy_from_bvec() to pass by val.
> 
> bio_copy_(to|from)_iter() is a wtf, though - gcc is now spilling the
> constructed bvec to the stack; my best guess is it's a register pressure
> thing (but we shouldn't be short registers here!).
> 
> So, since the fastpath stuff in bio.c gets smaller and blk-map.c is not
> exactly fastpath stuff I'm not inclined to fight with gcc on this one -
> let me know if that works for you.
> 
> Branch is updated - I split out the new assertions into a separate patch
> that adds CONFIG_BLK_DEBUG, and another patch for mempcy_(to|from)_bio()
> for a small code size decrease.
> 
> https://evilpiepirate.org/git/bcachefs.git/log/?h=block-for-bcachefs
> or
> git pull http://evilpiepirate.org/git/bcachefs.git block-for-bcachefs

Cn you resend just the iterator changes in their current form? The
various re-exports are a separate discussion, I think we should focus on
the iterator bits first.

-- 
Jens Axboe