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

[Jiri Olsa]

On Tue, Jun 28, 2016 at 11:19:59PM +0200, Rasmus Villemoes wrote:

SNIP

> > -			save_arg(void *);
> > +			if (spec.cpumask) {
> 
> As I hinted in the other mail, I think it's better just to put the
> fmt[1]=='b' here and not change struct printf_spec.
> 
> > +				/*
> > +				 * Store entire cpumask directly to buffer
> > +				 * instead of storing just a pointer.
> > +				 */
> > +				struct cpumask *mask = va_arg(args, void *);
> > +
> > +				str = PTR_ALIGN(str, sizeof(u32));
> > +
> > +				if (str + sizeof(*mask) <= end)
> > +					cpumask_copy((struct cpumask *) str, mask);
> 
> A cpumask is an array of longs. Why is u32-alignment enough for that?
> cpumask_copy may end up compiling to a simple "*dst = *src", and even if
> this is a memcpy(), the same 4-but-possibly-not-8 byte aligned
> pointer is created below in bstr_printf which is then passed on to
> pointer() and then bitmap_* which certainly expects an unsigned long*. 

hum, the binary buffer is copied after to trace buffer which
has 32bit alignment I think, so bigger alignment breaks the
data.. also the save_arg macro does 32bit alignment for 8 bytes

anyway, I haven't checked deeply on this, was just hunting
wrong cpumask, now with Steven's changes I'm ok to drop it ;-)

thanks,
jirka