From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752513AbcF1Q0S (ORCPT ); Tue, 28 Jun 2016 12:26:18 -0400 Received: from smtprelay0010.hostedemail.com ([216.40.44.10]:44238 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751947AbcF1Q0Q (ORCPT ); Tue, 28 Jun 2016 12:26:16 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::,RULES_HIT:41:355:379:541:599:800:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1730:1747:1777:1792:2198:2199:2393:2553:2559:2562:3138:3139:3140:3141:3142:3165:3622:3865:3866:3867:3868:3870:3871:3873:3874:4321:4605:5007:6119:6261:7875:7903:8603:9010:10004:10400:10848:10967:11026:11232:11473:11658:11914:12043:12291:12296:12438:12517:12519:12555:12663:12683:12740:13019:13138:13161:13229:13231:13439:14096:14097:14181:14659:14721:21080:21324:21434:30034:30054:30062:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:1,LUA_SUMMARY:none X-HE-Tag: light62_18a852811db2c X-Filterd-Recvd-Size: 4636 Date: Tue, 28 Jun 2016 12:26:08 -0400 From: Steven Rostedt To: Jiri Olsa Cc: Lai Jiangshan , lkml , Rasmus Villemoes , Frederic Weisbecker Subject: Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask Message-ID: <20160628122608.0ea291b2@gandalf.local.home> In-Reply-To: <1467128075-10841-1-git-send-email-jolsa@kernel.org> References: <1467128075-10841-1-git-send-email-jolsa@kernel.org> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 28 Jun 2016 17:34:34 +0200 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. The thing is, this is basically true with all pointer derivatives (just look at the list of options under pointer()). 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 :-/ > > 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 */ 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. -- Steve > 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) { > + /* > + * 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); > + > + str += sizeof(*mask); > + } else { > + save_arg(void *); > + } > + > /* skip all alphanumeric pointer suffixes */ > while (isalnum(*fmt)) > fmt++; > @@ -2490,12 +2508,25 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf) > break; > } > > - case FORMAT_TYPE_PTR: > - str = pointer(fmt, str, end, get_arg(void *), spec); > + case FORMAT_TYPE_PTR: { > + void *ptr; > + > + if (spec.cpumask) { > + /* > + * Load cpumask directly from buffer. > + */ > + args = PTR_ALIGN(args, sizeof(u32)); > + ptr = (void *) args; > + args += sizeof(struct cpumask); > + } else { > + ptr = get_arg(void *); > + } > + > + str = pointer(fmt, str, end, ptr, spec); > while (isalnum(*fmt)) > fmt++; > break; > - > + } > case FORMAT_TYPE_PERCENT_CHAR: > if (str < end) > *str = '%';