Re: Bcache v. whatever

[Andrew Morton]

On Fri, 26 Apr 2013 12:46:42 -0700 Kent Overstreet <koverstreet@xxxxxxxxxx> wrote:

> On Thu, Apr 25, 2013 at 04:17:04PM -0700, Andrew Morton wrote:
> > On Mon, 14 Jan 2013 14:32:02 -0800 Kent Overstreet <koverstreet@xxxxxxxxxx> wrote:
> > 
> ...
> > 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/