Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask

[Steven Rostedt]

On Tue, 28 Jun 2016 21:27:29 +0200
Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote:


> > I probably should make a trace_printk() that doesn't default to the
> > binary print, to handle things like this.
> >
> >   trace_printk_ptr()?
> >
> > Or even just see if I can find a way that detects this in the fmt
> > string. Hmm, that probably can't be done at compile time :-/  
> 
> Well, not with gcc itself, but it wouldn't be too hard to make smatch
> complain loudly if trace_printk is used on a format string with any %p
> extension (directing people to use trace_printk_ptr()) - the format
> parsing (and type checking) is already there.

But people don't usually run "make smatch" on debug kernels.
trace_printk() currently is not allowed to be used in the kernel. When
it is used, a big ugly banner is posted on boot up saying that the
kernel is in "debug mode" and is "unstable" (even though it isn't) just
to scare people enough to never compile with a trace_printk() in their
code. If they need a permanent trace_printk() then they need to use
tracepoints.

> 
> >> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> >> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> >> ---
> >>  lib/vsprintf.c | 41 ++++++++++++++++++++++++++++++++++++-----
> >>  1 file changed, 36 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> >> index 0967771d8f7f..f21d68e1b5fc 100644
> >> --- a/lib/vsprintf.c
> >> +++ b/lib/vsprintf.c
> >> @@ -388,7 +388,8 @@ enum format_type {
> >>  struct printf_spec {
> >>  	unsigned int	type:8;		/* format_type enum */
> >>  	signed int	field_width:24;	/* width of output field */
> >> -	unsigned int	flags:8;	/* flags to number() */
> >> +	unsigned int	flags:7;	/* flags to number() */
> >> +	unsigned int	cpumask:1;	/* pointer to cpumask flag */  
> >
> > Why not just add this as another flag? There's one left. I'm not sure
> > gcc does nice things with bit fields not a multiple of 8.  
> 
> I really don't think we should pollute the common printf code with this
> stuff, partly because of the code generation issues, but also: what
> should we do the next time someone decides to handle a %p extension more
> correctly in vbin_printf?


Perhaps we should add an WARN_ON is these are used in vbin_printf().

I'll go and make a trace_printk_ptr() patch.

-- Steve