Re: Bcache v. whatever
From: Andrew Morton <hidden>
Date: 2013-04-26 20:24:41
Also in:
lkml
On Fri, 26 Apr 2013 12:46:42 -0700 Kent Overstreet [off-list ref] wrote:
On Thu, Apr 25, 2013 at 04:17:04PM -0700, Andrew Morton wrote:quoted
On Mon, 14 Jan 2013 14:32:02 -0800 Kent Overstreet [off-list ref] wrote:...quoted
drivers/md/bcache/btree.c: In function `bch_btree_refill_keybuf': drivers/md/bcache/btree.c:2330: error: invalid operands to binary + due to #define pbtree(b) (&bch_pbtree(b).s[0]) I don't know why this is happening (presumably a gcc glitch), but returning an 80-byte struct by value from bch_pkey() and bch_pbtree() is just gruesome. The compiler has to allocate the space on the caller stack, pass a hidden pointer into the callee and the callee copies its return value into that caller stack slot. It's slow and consumes stack. Something different, please.Well, it is kind of... perverse but really the compiler's doing exactly what I would've had to do otherwise - stick a char buf[80] on the caller's stack and pass it to bch_pbtree(). With the caveat that I haven't looked at the generated code.
That's the more idiomatic way of doing things and yes, the code generation will be similarly awful.
As far as I can tell the only real improvement would be to add a %p format string to vsnprintf, but adding a global extension would obviously be inappropriate for this. It'd be really nice to have a mechanism for adding file/module private format strings to vsnprintf, but I haven't cared enough yet to implement it myself. Of course if you know a better solution I'm all ears. Uhm, as for the actual bug - that is a fairly ancient gcc, I wasn't aware we were supporting compilers that old but I'm sure you wouldn't be bugging me about it if we weren't...
Documentation/Changes is the official status. It says gcc-3.2+. We're
very slow in updating those version numbers because it's hard.
gcc-3.4.5 for mips fails in the same way.
Why are those things macros anyway? urgh, it's because we want to jam
a string into the caller's stack frame without declaring any of it.
Really I do think it would be better to do away with the C party tricks
and have callers do
char btree_buf[BTREE_BUF_SIZE];
btree_to_text(btree_buf, b);
pr_debug("%s\n", btree_buf);
Nice, simple, explicit, direct and stupid. It might generate
unused-var warnings if DEBUG is undefined but from my reading of
pr_debug() things will be OK.
Then we can poke around at btree_to_text() until gcc-3.4.5 is happy
with it.