From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752650AbcF1VUE (ORCPT ); Tue, 28 Jun 2016 17:20:04 -0400 Received: from mail-wm0-f44.google.com ([74.125.82.44]:37050 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752186AbcF1VUC (ORCPT ); Tue, 28 Jun 2016 17:20:02 -0400 From: Rasmus Villemoes To: Jiri Olsa Cc: Lai Jiangshan , Steven Rostedt , lkml , Frederic Weisbecker Subject: Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask Organization: D03 References: <1467128075-10841-1-git-send-email-jolsa@kernel.org> X-Hashcash: 1:20:160628:jolsa@kernel.org::Id+sV/0bEqRFWGOg:010aP X-Hashcash: 1:20:160628:rostedt@goodmis.org::dOk6/ELM4/Rcoq5R:0000000000000000000000000000000000000000001ADX X-Hashcash: 1:20:160628:laijs@cn.fujitsu.com::fkqQSqQ+sKVXKTsH:000000000000000000000000000000000000000002U+K X-Hashcash: 1:20:160628:fweisbec@gmail.com::T+5JIk/me5IzMzSU:00000000000000000000000000000000000000000002OZ/ X-Hashcash: 1:20:160628:linux-kernel@vger.kernel.org::s9RHAbcKawnglsbV:0000000000000000000000000000000006TrY Date: Tue, 28 Jun 2016 23:19:59 +0200 In-Reply-To: <1467128075-10841-1-git-send-email-jolsa@kernel.org> (Jiri Olsa's message of "Tue, 28 Jun 2016 17:34:34 +0200") Message-ID: <87r3bhgge8.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 28 2016, Jiri Olsa wrote: > When using trace_printk for cpumask I've got wrong results, > some bitmaps were completely different from what I expected. > > Currently you get wrong results when using trace_printk > on local cpumask, like: > > void test(void) > { > struct cpumask mask; > ... > trace_printk("mask '%*pbl'\n", cpumask_pr_args(&mask)); > } > > The reason is that trace_printk stores the data into binary > buffer (pointer for cpumask), which is read after via read > handler of trace/trace_pipe files. At that time pointer for > local cpumask is no longer valid and you get wrong data. > > Fixing this by storing complete cpumask into tracing buffer. > > Cc: Steven Rostedt > Signed-off-by: Jiri Olsa > --- > 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 */ > unsigned int base:8; /* number base, 8, 10 or 16 only */ > signed int precision:16; /* # of digits/chars */ > } __packed; > @@ -1864,6 +1865,7 @@ qualifier: > > case 'p': > spec->type = FORMAT_TYPE_PTR; > + spec->cpumask = fmt[1] == 'b'; > return ++fmt - start; > > case '%': > @@ -2338,7 +2340,23 @@ do { \ > } > > case FORMAT_TYPE_PTR: > - 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*. Rasmus