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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZH0gjyuBgYzqhZh7@moria.home.lan>
Date:   Sun, 4 Jun 2023 19:38:55 -0400
From:   Kent Overstreet <kent.overstreet@...ux.dev>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 0/7] block layer patches for bcachefs

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.

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