From: Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
To: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org,
snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Subject: Re: Bcache v. whatever
Date: Fri, 26 Apr 2013 13:54:54 -0700 [thread overview]
Message-ID: <20130426205454.GG9931@google.com> (raw)
In-Reply-To: <20130426132438.4e48e0688fc3c3c298e71d7e-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
On Fri, Apr 26, 2013 at 01:24:38PM -0700, Andrew Morton wrote:
> > 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...
>
> 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.
Well, it also _used_ to be the case that if DEBUG and
CONFIG_DYNAMIC_DEBUG weren't defined then pr_debug() was defined as an
empty macro, so btree_to_text() wouldn't be called.
But, I just checked and it's been converted to an empty inline
function, and some of this debug code is called from fast paths where
the overhead really does matter :/ argh
> 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.
Well, since pr_debug() isn't an empty macro anymore there goes my main
reason for keeping these macros around.
seems like pr_debug() could still have the no_printk() call inside an
if (0) though...
argh. If there was some way to tell gcc "this function only modifies
this argument so you don't have to call it if the output is never used"
then I'd just convert it to the explicit bufs now. But I really can't
have btree_to_text()/bkey_to_text() getting called in non debug
kernels...
I'm going to mull it over a bit for now, and try and decide what's least
ugly.
WARNING: multiple messages have this Message-ID (diff)
From: Kent Overstreet <koverstreet@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-bcache@vger.kernel.org,
tj@kernel.org, axboe@kernel.dk,
James.Bottomley@hansenpartnership.com, snitzer@redhat.com
Subject: Re: Bcache v. whatever
Date: Fri, 26 Apr 2013 13:54:54 -0700 [thread overview]
Message-ID: <20130426205454.GG9931@google.com> (raw)
In-Reply-To: <20130426132438.4e48e0688fc3c3c298e71d7e@linux-foundation.org>
On Fri, Apr 26, 2013 at 01:24:38PM -0700, Andrew Morton wrote:
> > 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...
>
> 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.
Well, it also _used_ to be the case that if DEBUG and
CONFIG_DYNAMIC_DEBUG weren't defined then pr_debug() was defined as an
empty macro, so btree_to_text() wouldn't be called.
But, I just checked and it's been converted to an empty inline
function, and some of this debug code is called from fast paths where
the overhead really does matter :/ argh
> 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.
Well, since pr_debug() isn't an empty macro anymore there goes my main
reason for keeping these macros around.
seems like pr_debug() could still have the no_printk() call inside an
if (0) though...
argh. If there was some way to tell gcc "this function only modifies
this argument so you don't have to call it if the output is never used"
then I'd just convert it to the explicit bufs now. But I really can't
have btree_to_text()/bkey_to_text() getting called in non debug
kernels...
I'm going to mull it over a bit for now, and try and decide what's least
ugly.
next prev parent reply other threads:[~2013-04-26 20:54 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-14 22:32 Bcache v. whatever Kent Overstreet
2013-01-14 22:32 ` Kent Overstreet
2013-01-15 1:49 ` Greg KH
[not found] ` <20130115014931.GA19373-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2013-01-15 21:18 ` Kent Overstreet
2013-01-15 21:18 ` Kent Overstreet
2013-01-15 22:50 ` Greg KH
[not found] ` <20130114223202.GV26407-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2013-01-15 9:20 ` James Bottomley
2013-01-15 9:20 ` James Bottomley
[not found] ` <1358241656.2383.2.camel-sFMDBYUN5F8GjUHQrlYNx2Wm91YjaHnnhRte9Li2A+AAvxtiuMwx3w@public.gmane.org>
2013-01-15 20:59 ` Kent Overstreet
2013-01-15 20:59 ` Kent Overstreet
2013-01-15 23:15 ` James Bottomley
2013-01-15 23:33 ` Kent Overstreet
[not found] ` <20130115233347.GD26407-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2013-01-16 1:29 ` Alasdair G Kergon
2013-01-16 1:29 ` Alasdair G Kergon
[not found] ` <20130116012914.GA27245-FDJ95KluN3Z0klwcnFlA1dvLeJWuRmrY@public.gmane.org>
2013-01-17 23:43 ` Mike Snitzer
2013-01-17 23:43 ` Mike Snitzer
2013-01-15 21:25 ` [PATCH 1/4] Revert "rw_semaphore: remove up/down_read_non_owner" Kent Overstreet
2013-01-15 21:25 ` Kent Overstreet
2013-04-25 23:17 ` Bcache v. whatever Andrew Morton
2013-04-25 23:17 ` Andrew Morton
[not found] ` <20130425161704.3f0fc3b6af55cf75acbc9d9e-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2013-04-26 19:46 ` Kent Overstreet
2013-04-26 19:46 ` Kent Overstreet
[not found] ` <20130426194642.GC9931-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2013-04-26 20:24 ` Andrew Morton
2013-04-26 20:24 ` Andrew Morton
[not found] ` <20130426132438.4e48e0688fc3c3c298e71d7e-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2013-04-26 20:54 ` Kent Overstreet [this message]
2013-04-26 20:54 ` Kent Overstreet
2013-01-15 21:25 ` [PATCH 2/4] Export get_random_int() Kent Overstreet
2013-01-15 21:25 ` [PATCH 3/4] Export blk_fill_rwbs() Kent Overstreet
[not found] ` <1358285142-10576-3-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2013-02-01 3:48 ` Steven Rostedt
2013-02-01 3:48 ` Steven Rostedt
[not found] ` <1359690521.5642.10.camel-f9ZlEuEWxVcJvu8Pb33WZ0EMvNT87kid@public.gmane.org>
2013-02-01 3:57 ` Li Zefan
2013-02-01 3:57 ` Li Zefan
[not found] ` <510B3D2A.3040007-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-02-01 4:06 ` Steven Rostedt
2013-02-01 4:06 ` Steven Rostedt
2013-01-15 21:25 ` [PATCH 4/4] Export __lockdep_no_validate__ Kent Overstreet
[not found] <koverstreet@google.com>
2013-01-15 6:59 ` Bcache v. whatever Dr. Greg Wettstein
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=20130426205454.GG9931@google.com \
--to=koverstreet-hpiqsd4aklfqt0dzr+alfa@public.gmane.org \
--cc=James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org \
--cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
--cc=axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org \
--cc=linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.