From: Stepan Moskovchenko <stepanm@codeaurora.org>
To: Joe Perches <joe@perches.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
Stepan Moskovchenko <stepanm@codeaurora.org>,
Rob Landley <rob@landley.net>,
Andrew Morton <akpm@linux-foundation.org>,
George Spelvin <linux@horizon.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Stephen Boyd <sboyd@codeaurora.org>,
Andrei Emeltchenko <andrei.emeltchenko@intel.com>,
mingo@kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] lib: vsprintf: Add %pa format specifier for phys_addr_t types
Date: Wed, 23 Jan 2013 16:37:08 -0800 [thread overview]
Message-ID: <51008234.8080702@codeaurora.org> (raw)
In-Reply-To: <1358914468.2107.26.camel@joe-AO722>
> On Tue, 2013-01-22 at 23:26 +0100, Geert Uytterhoeven wrote:
>> On Tue, Jan 22, 2013 at 6:47 AM, Stepan Moskovchenko
>> <stepanm@codeaurora.org> wrote:
>> > Add the %pa format specifier for printing a phys_addr_t
>> > type, since the physical address size on some platforms
>> > can vary based on build options, regardless of the native
>> > integer type.
>>
>> I'm not so sure it's a good idea to start using %p for integer types.
>
> It would also require that some automatic variables
> that could otherwise be used in registers be put on
> the stack.
>
When calling something like printk on a variable, is that really a big
concern in the grand scheme of things? On a system with a 32-bit
phys_addr_t type, how does the overhead of having to put the variable on
the stack compare to the overhead needed to cast to an unsigned long
long and emit the eight extra '0' characters when printing a formatted
physical address? If allocation efficiency is a big concern for a
particular code path, the caller is always free to continue to cast to
unsigned long long and use %llu, if they don't mind extra padding where
phys_addr_t is smaller than a long long. But considering this is
primarily meant to be used by printk during initialization / debug /
error handling paths, there will be plenty of other overhead involved in
producing log output.
As far as type safety is concerned, the %p specifier is already willing
to accept pointers to all sorts of things which it is powerless to
check, and it is up to the caller to do the right thing.
Printing out a physical address is a pretty reasonable thing for a
driver to do in an init or debug path. It is conceivable that the same
driver may be used on a system with 32-bit ore >32-bit physical address
types. With something like ARM LPAE, it is even possible for the same
system to use 32-bit and >32-bit addressing based strictly on build
options, while still using the same drivers, meaning that the drivers
would need to deal with phys_addr_t being of variable size. It also
seems like a potential bit of extra overhead is not worth the ugliness
of casting each phys_addr_t to an unsigned long long and printing it as
a 64-bit type regardless of the actual number of address bits available.
If a driver is really concerned about efficiency of its init paths, it
is free to continue using %llu, or better yet, cut down on the amount of
log spam these paths generate.
Steve
next prev parent reply other threads:[~2013-01-24 0:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-22 5:47 [PATCH] lib: vsprintf: Add %pa format specifier for phys_addr_t types Stepan Moskovchenko
2013-01-22 7:29 ` Andy Shevchenko
2013-01-22 7:52 ` Joe Perches
2013-01-22 21:07 ` Stepan Moskovchenko
2013-01-22 22:26 ` Geert Uytterhoeven
2013-01-23 4:14 ` Joe Perches
2013-01-24 0:37 ` Stepan Moskovchenko [this message]
2013-01-23 0:14 ` [PATCH v2] " Stepan Moskovchenko
2013-01-24 23:07 ` Andrew Morton
2013-02-07 4:39 ` Rob Landley
2013-02-07 6:39 ` Geert Uytterhoeven
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=51008234.8080702@codeaurora.org \
--to=stepanm@codeaurora.org \
--cc=akpm@linux-foundation.org \
--cc=andrei.emeltchenko@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=geert@linux-m68k.org \
--cc=joe@perches.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@horizon.com \
--cc=mingo@kernel.org \
--cc=rob@landley.net \
--cc=sboyd@codeaurora.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