From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755044AbcFQJNz (ORCPT ); Fri, 17 Jun 2016 05:13:55 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:10401 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754824AbcFQJNu (ORCPT ); Fri, 17 Jun 2016 05:13:50 -0400 X-IBM-Helo: d28dlp01.in.ibm.com X-IBM-MailFrom: maddy@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Subject: Re: [PATCH] tools/perf: Fix the mask in regs_dump__printf To: Jiri Olsa References: <1466140958-1928-1-git-send-email-maddy@linux.vnet.ibm.com> <20160617063707.GA31475@krava> Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Yury Norov , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Kan Liang , Wang Nan , Michael Ellerman From: Madhavan Srinivasan Date: Fri, 17 Jun 2016 14:43:31 +0530 User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160617063707.GA31475@krava> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16061709-0016-0000-0000-000002A79083 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16061709-0017-0000-0000-0000245B649C Message-Id: <7abb0f9a-dea0-de86-ba0b-cebce0f11744@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-06-17_05:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1606170113 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 17 June 2016 12:07 PM, Jiri Olsa wrote: > On Fri, Jun 17, 2016 at 10:52:38AM +0530, Madhavan Srinivasan wrote: >> When decoding the perf_regs mask in regs_dump__printf(), >> we loop through the mask using find_first_bit and find_next_bit functions. >> And mask is of type "u64". But "u64" is send as a "unsigned long *" to >> lib functions along with sizeof(). >> >> While the exisitng code works fine in most of the case, when using a >> 32bit perf on a 64bit kernel (Big Endian), we end up reading the wrong word >> in the u64 mask. Patch to fix the mask in regs_dump__printf(). >> >> Suggested-by: Yury Norov >> Cc: Yury Norov >> Cc: Peter Zijlstra >> Cc: Ingo Molnar >> Cc: Arnaldo Carvalho de Melo >> Cc: Alexander Shishkin >> Cc: Jiri Olsa >> Cc: Adrian Hunter >> Cc: Kan Liang >> Cc: Wang Nan >> Cc: Michael Ellerman >> Signed-off-by: Madhavan Srinivasan >> --- >> tools/perf/util/session.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c >> index 5214974e841a..2eaa42a4832a 100644 >> --- a/tools/perf/util/session.c >> +++ b/tools/perf/util/session.c >> @@ -940,8 +940,13 @@ static void branch_stack__printf(struct perf_sample *sample) >> static void regs_dump__printf(u64 mask, u64 *regs) >> { >> unsigned rid, i = 0; >> + unsigned long _mask[sizeof(mask)/sizeof(unsigned long)]; >> >> - for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) { >> + _mask[0] = mask & ULONG_MAX; >> + if (sizeof(mask) > sizeof(unsigned long)) >> + _mask[1] = mask >> 32; >> + > I think we should do this earlier when reading the mask, > not at the moment we just print it, like we do for other > types.. maybe we could do this as an extra bit for Ok. I tried something like this. Currently mask is read in perf_evsel__parse_sample(), so added for both sample type (PERF_SAMPLE_REGS_USER and PERF_SAMPLE_REGS_INTR) if (data->user_regs.abi) { - u64 mask = evsel->attr.sample_regs_user; + u.val64 = evsel->attr.sample_regs_user; - sz = hweight_long(mask) * sizeof(u64); + if (sizeof(u64) > sizeof(unsigned long)) { + u64 mask = u.val64; + u.val32[1] = mask >> 32; + u.val32[0] = mask & ULONG_MAX; + } + + sz = hweight_long(u.val64) * sizeof(u64); OVERFLOW_CHECK(array, sz, max_size); - data->user_regs.mask = mask; + data->user_regs.mask = u.val64; data->user_regs.regs = (u64 *)array; Issue I see is when printing the mask value in a 32bit perf on a 64bit kernel (big endian). 442044948492 0xdc0 [0x188]: PERF_RECORD_SAMPLE(IP, 0x1): 7299/7299: 0xc000000000059200 period: 12599 addr: 0 ... intr regs: mask 0xffffffff000007ff ABI 32-bit ^^^ shld have been 0x7ffffffffff I agree it is better to fix this when reading, but we need to swap again when printing? > perf_event__swap_ops[PERF_RECORD_SAMPLE] function > > also there's print_sample_iregs in builtin-script.c that's > most likely affected as well My bad. Should have seen this too. Maddy > > thanks, > jirka > >> + for_each_set_bit(rid, _mask, sizeof(mask) * 8) { >> u64 val = regs[i++]; >> >> printf(".... %-5s 0x%" PRIx64 "\n", >> -- >> 1.9.1 >>