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
