linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Matthew Wilcox <matthew@wil.cx>
Cc: linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, Peter Anvin <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: the printk problem
Date: Fri, 4 Jul 2008 15:01:00 -0700	[thread overview]
Message-ID: <20080704150100.1f7b8a65.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080704204252.GM14894@parisc-linux.org>


(heck, let's cc lkml - avoid having to go through all this again)

On Fri, 4 Jul 2008 14:42:53 -0600 Matthew Wilcox <matthew@wil.cx> wrote:

> On Fri, Jul 04, 2008 at 01:27:16PM -0700, Andrew Morton wrote:
> > On Fri, 4 Jul 2008 13:02:05 -0700 (PDT) Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > > > so I think we could easily just say that we extend %p in various ways:
> > > > 
> > > >  - %pS - print pointer as a symbol
> > > > 
> > > > and leave tons of room for future extensions for different kinds of 
> > > > pointers. 
> > 
> > If this takes off we might want a register-your-printk-handler
> > interface.  Maybe.
> > 
> > We also jump through hoops to print things like sector_t and
> > resource_size_t.  They always need to be cast to `unsiged long long',
> > which generates additional stack space and text in some setups.
> 
> The thing is that GCC checks types.  So it's fine to add "print this
> pointer specially", but you can't in general add new printf arguments
> without also hacking GCC.  Unless you use -Wno-format, and require
> sparse to check special kernel types.

It would be excellent if gcc had an extension system so that you could
add new printf control chars and maybe even tell gcc how to check them.
But of course, if that were to happen, we couldn't use it for 4-5 years.

What I had initially proposed was to abuse %S, which takes a wchar_t*. 
gcc accepts `unsigned long *' for %S.

Then, we put the kernel-specific control char after the S, so we can
print an inode (rofl) with

	struct inode *inode;

	printk("here is an inode: %Si\n", (unsigned long *)inode);

Downsides are:

- there's a cast, so you could accidentally do

	printk("here is an inode: %Si\n", (unsigned long *)dentry);

- there's a cast, and they're ugly

- gcc cannot of course check that the arg matches the control string

Unfortunately (and this seems weird), gcc printf checking will not
accept a void* for %S: it _has_ to be wchar_t*, and the checker won't
permit void* substitution for that.

Anyway, Linus took all that and said "let's abuse %p instead".  It
_will_ accept any pointer so we can instead do:

	printk("here is an inode: %pi\n", inode);

which is nicer.


I think the main customers of this are print_symbol():

	printk("I am about to call %ps\n", fn);
	(*fn)();

and NIPQUAD and its ipv6 version.

We don't know how much interest there would be in churning NIPQUAD from
the net guys.  Interestingly, there's also %C (wint_t) which is a
32-bit quantity.  So we could just go and say "%C prints an ipv4
address" and be done with it.  But there's no way of doing that for
ipv6 addresses so things would become asymmetrical there.

Another customer is net mac addresses.  There are surely others.  One
which should have been in printf 30 years ago was %b: binary.


> > And then there's the perennial "need to cast u64 to unsigned long long
> > to print it".  If we were to do
> > 
> > 	printk("%SL", (void *)some_u64);
> > 
> > then that's still bloody ugly, but it'll save a little text-n-stack.
> 
> u64 is easy to fix, and I don't know why we haven't.  Just make it
> unsigned long long on all architectures.

Yeah.  Why don't we do that?

  reply	other threads:[~2008-07-04 22:01 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080625131101.GA6205@digi.com>
     [not found] ` <20080704104634.GA31634@digi.com>
     [not found]   ` <20080704111540.ddffd241.akpm@linux-foundation.org>
     [not found]     ` <alpine.LFD.1.10.0807041147450.2815@woody.linux-foundation.org>
2008-07-04 20:02       ` the printk problem Linus Torvalds
2008-07-04 20:27         ` Andrew Morton
2008-07-04 20:41           ` Linus Torvalds
2008-07-04 20:42           ` Matthew Wilcox
2008-07-04 22:01             ` Andrew Morton [this message]
2008-07-05  2:03               ` Matthew Wilcox
2008-07-22 10:05                 ` [PATCH] Make u64 long long on all architectures (was: the printk problem) Andrew Morton
2008-07-22 10:36                   ` Michael Ellerman
2008-07-22 10:53                     ` Andrew Morton
2008-07-22 11:36                     ` Benjamin Herrenschmidt
2008-07-22 11:35                   ` Benjamin Herrenschmidt
2008-07-05 10:20               ` the printk problem Denys Vlasenko
2008-07-05 11:33               ` Jan Engelhardt
2008-07-05 12:52                 ` Vegard Nossum
2008-07-05 13:24                   ` Jan Engelhardt
2008-07-05 13:50                     ` Vegard Nossum
2008-07-05 14:07                       ` Jan Engelhardt
2008-07-05 17:56                   ` Linus Torvalds
2008-07-05 18:40                     ` Jan Engelhardt
2008-07-05 18:44                       ` Linus Torvalds
2008-07-05 18:41                     ` Vegard Nossum
2008-07-05 18:52                       ` Matthew Wilcox
2008-07-06  0:02                         ` Pekka Enberg
2008-07-06  5:17                           ` Randy Dunlap
2008-07-04 22:58             ` Benjamin Herrenschmidt
2008-07-04 20:36         ` Matthew Wilcox
2008-07-08  1:44           ` Kyle McMartin
2008-07-04 23:00         ` Benjamin Herrenschmidt
2008-07-04 23:25           ` Linus Torvalds
2008-07-05 22:32             ` Linus Torvalds
2008-07-05 22:57               ` Arjan van de Ven
2008-07-06  5:27               ` Ingo Molnar
2008-07-06  5:37                 ` Linus Torvalds
2008-07-06  5:53                   ` Ingo Molnar
2008-07-06  6:13                     ` Ingo Molnar
2008-07-07  1:14             ` Benjamin Herrenschmidt
2008-07-07  3:26               ` Stephen Rothwell
2008-07-07  3:28                 ` Michael Ellerman
2008-07-07  4:59                   ` Stephen Rothwell
2008-07-07  3:43                 ` Benjamin Herrenschmidt

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=20080704150100.1f7b8a65.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=hpa@zytor.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=matthew@wil.cx \
    --cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).