From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933154AbcFOTvg (ORCPT ); Wed, 15 Jun 2016 15:51:36 -0400 Received: from mail-bn1on0072.outbound.protection.outlook.com ([157.56.110.72]:9296 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752726AbcFOTve (ORCPT ); Wed, 15 Jun 2016 15:51:34 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Yuri.Norov@caviumnetworks.com; Date: Wed, 15 Jun 2016 22:51:27 +0300 From: Yury Norov To: Madhavan Srinivasan CC: , , Arnaldo Carvalho de Melo , Adrian Hunter , Borislav Petkov , David Ahern , George Spelvin , Jiri Olsa , Namhyung Kim , Rasmus Villemoes , Wang Nan , Yury Norov , Michael Ellerman Subject: Re: [PATCH] tools/perf: fix the word selected in find_*_bit Message-ID: <20160615195127.GA6039@yury-N73SV> References: <1465990973-31483-1-git-send-email-maddy@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1465990973-31483-1-git-send-email-maddy@linux.vnet.ibm.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [50.233.148.158] X-ClientProxiedBy: BN3PR16CA0002.namprd16.prod.outlook.com (10.165.112.140) To SN1PR07MB2256.namprd07.prod.outlook.com (10.164.47.150) X-MS-Office365-Filtering-Correlation-Id: 6bcc7418-6800-4654-deca-08d3955672de X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2256;2:s29PCb7VJM9OoOGQoN1l0SQ0ItZq39D2PYlpnkkJRyPcysK/dUZstSGVA5yZJR1Y7uZ6P0PzhZDR97eima+rdaQwG3UQ1H7Zz7KpBTj005RqLLzs6hRi0d/GSDgG/qHdsjdeXWIFlDWp+OnsoPNOggUJRklDOPoUAGmFDh8A1d5wCLutMJ6d0mzcc9FUPxl0;3:HBFStcInCKiqGCswe/qLX6wF6kjTLhXfTul2E6Jk5QG1bBqyVSN+KvjdWFeHOCZHQkuYFvbNaq0M1Wbtv5OzSHU3mRAV5XXWhjSsM15Zk3ucRFLPfYsfN8WRRwZ4gv2i;25:j3VwqHNpfJpytMOk5Fcebnrtp631lAfG6ogezUsC8SXkdd3Ql6CN2ILCWLXHj0q2ADvms37mMcflqTqOnJbMuvalk7JivIAGezxVI52YaH84AvO+c//pTxdANzoGa16rao9oW7jMdlMIDhEJ+c366A6Nf5MRYh1VTgPnG3sAfv8D9BzqEAKZbGmqWVDumLZmZ2EtH9Fi9gk4/dUwXBM/44tctM1tcrwtqclF+hPp7eDboBLkPA4TqYJ/U2cesoZSmo+nB1BF7FFGZgsagiDcChbkKd5yo0LL+UTE76k1dz99nLbIxXz8JtwSKdK0a5GGZjtRx+ZjIWAZirW6Eel7GIBkAVJIXw7G1ybMAH3L+9wPGEUcYuWxKa58hljfPaldPJYb4p7GWoCHn7fc9mf1a/hOKBDPXqQAgXugfQag0pI= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR07MB2256; X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2256;20:sSP28M/uz0dvMA2O9EYzEP3xFBIH8yPwDJlc90Ag//FXHjsenxXp3ImM5bqj5fCLLfNDG9yxOT+JbHeT5rTlf4TmUkyJf1mDDW1EYMMH9pHF5LUeNZLntn9qiFUjloCI0Be0gp5ZsM1n1osMZvaVWpmbpOYxgoFcwRZV24VDP0KWygelT9pkLiz7e1hx4yI8572rFvX9w9Co90fEFlS4b+GU35haCCeOIoyNfMyTEJfqUpliTfcWeiG1QTrqi655XpEce3ku7VlvFqciKVEcLD//GlaUAlfXNNAsbT5MFiqEVfaQT2RllXZ7AaHkHnfd9Yq+qubdlmgfOGdT/JkGdcGT1EKz7DoLii64C9wLlGxMyEi8I2qX51h43m9ACKT5d5lf1lxPlzNz87yQtwSubBK9PlhZ4PCuWbe5KDcB7VYyMk3DXIysCCDH+asv42RcQI1s4kXWxVgYopLcpWRD3ZWINER3MgGq9fKqUPNqmtwl9a2yk/Wmmb7b6g9myQF+zM83/6V91Hq/0wUxh/onh/T58NdVpY8EWFc3qpUCR5n+QmgQX9IQZcanORPdy6BP1HR6MuM/NNKezMNI/lIcuSMgC0W8ZtTkCDPcwjCFeFc= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(50582790962513)(104084551191319)(228905959029699); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:SN1PR07MB2256;BCL:0;PCL:0;RULEID:;SRVR:SN1PR07MB2256; X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2256;4:aptjPqB8JvUgFEvTw5GTup6YnshouWhE9Azm1CeOpf7Ja7X8teE/OzcjA4aTd7GqdabTvyHG7sGrOnt0/oLcp2Z8iW3BFBNIsWmM0s42UlcgwogLLSm0F+LKMgh+G/Q69bAfVgxQoy3yGo0TvTMGWw+t8vV0tSKXvWh1MxTkoBaboVfUbuHGMr43obh3IqWafK0tZhEIrWdlHuFIVrdfeoS8OZ2/1XYofF1FjBDnJQAcAdAzRBTdVK+uwVyT3M/XHwG2kgT7TQCH84L0jpi7Ol1UXowNTZnYkYqS0u+20tE4U/K9OFbcc1AlVTf4F4w6YAzkAlaTFaMgSaErz+uxz6OUfKv1sbybUlB9P0RfglVeyQ+PYqNV/kH3nTg2tK2s4kFgDM1Xv7twVQ+oQ08C1LlYu/7+ulNoKYIphDBcGvyBJUl8u/MMuzCINyJuhKICw7VUlLcP5Wf9aOnEIjQscrx6420GXw1CfYBlLRml1m8= X-Forefront-PRVS: 09749A275C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(6069001)(7916002)(189002)(199003)(24454002)(101416001)(97756001)(2950100001)(33716001)(50466002)(2906002)(1076002)(5004730100002)(8676002)(81166006)(4326007)(77096005)(76506005)(5008740100001)(33656002)(68736007)(3846002)(23726003)(81156014)(46406003)(97736004)(4001350100001)(47776003)(19580405001)(19580395003)(106356001)(54356999)(76176999)(50986999)(189998001)(83506001)(42186005)(105586002)(6116002)(586003)(9686002)(92566002)(110136002)(66066001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR07MB2256;H:localhost;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;CAT:NONE;LANG:en;CAT:NONE; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;SN1PR07MB2256;23:nWutD8ZKQEzDdhF0bB7cZVAFjONkhGCpAyiYwGoW8?= =?us-ascii?Q?LqZRNDVG5BTmCIAtiFA1alatpYcSqlDgBgRM/TVHLHAykeb4HeO38ezhF5gb?= =?us-ascii?Q?JPrRuSCEmajhhBj3Yuz+oDh6cwVOmvLgs3e0u5Dx6TkjTUhtREQw20WU4Amd?= =?us-ascii?Q?05dkw07at/sPvxfA/beG0vPXHx90uZv5hrSO1UQ7l7ygGjgbcDipN51EeX3b?= =?us-ascii?Q?NfN0rG+jlO0gcP60kLDlSexx+4hjdK6srYg5Udr0EK383cUrqaWXA3jtGqHN?= =?us-ascii?Q?XGInoos649yA87sDY+wbqqoRNXGwLcD8icBzoPErRrNO/yg8ZA4ArDI7fdsV?= =?us-ascii?Q?bzF55/3vPb0LHYzxSk+jdxSYLyJiAaBm46GH5M6i4OIIU7ZJXg2OSqIQXGrh?= =?us-ascii?Q?XdteXgCY1rgPPslHLNtfspJF8ptEm148vgxe1CfNLTiJ0imi8fFD3RP8aq7l?= =?us-ascii?Q?+YyUGbJLniVC/sKTOuW9X7cml50x9W7VQFE9ADsyVzflPuexfkKjWz0vnRCo?= =?us-ascii?Q?G2KCxzUWw9mIcNPsAWD/FNpYvESigZCioKW5CgIxPj0YUQsFW6Zp9PPwUEcM?= =?us-ascii?Q?YUoadziqV+6guSae1kpZ87sG3eWveHggtW/3C9RoLpSjn/9W/pjzPerHIGId?= =?us-ascii?Q?1kcqkD7rVGStJy7xGGvzTP/RaJ63dMDeEMBHx0377yM7iDJmNhdsfYrsZ+KI?= =?us-ascii?Q?kLKnq1yef04NK7hqWGp8DUbcAojgUDeZ4zNihmznehJvcUwOL4Z3kb8cfb48?= =?us-ascii?Q?RjmdESRXj3X5plYNVaxFk76xFv1FzKRrfDOsAkyb0H0DokNQxHSq/xBVTL6/?= =?us-ascii?Q?agcecwuUILLInz+bATEuSB6mbgVps9uqBkcy0tIur4HOzsNSUvtUYXNm0IZd?= =?us-ascii?Q?HHAIsmmYt9nlqZUdoe3C38D1aOTdJyO4uKsX9SC2GEYzzldhT4zdjcRYlDC5?= =?us-ascii?Q?feF5kImNHuqrm0g4wbLsOXqVNmZ6B/62uDw8nBz4KPuoWSEzgyPKg0MaYZOA?= =?us-ascii?Q?5LkCfixa7jLhf1RMRBciADM5GObtjJWI0GWWd60U63IMg6jxyj4Kr9ahKgRe?= =?us-ascii?Q?2cEahLPJywM78QG8o8Z7/5SPzJ1zPupSCxnk+dv7TH/a6UBib6qQx8tEXzNf?= =?us-ascii?Q?96bLvHZnZRCGv1sQFaAd6VJ0VzNR/VeaIZete/mlODxw19mUtB5YjCeMt1TE?= =?us-ascii?Q?fWo/EBPnqV7Ks0=3D?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2256;6:4lLJhTb+Zz1FTtuFzGI6IHBJGzzKFN0JOuk0M1hoqEuA48LwuTY1663Vdsjekpm0ZDD80/XwJkCkrlMiAruql+sU7+mXBFlq0Api6P07ZxvqFikoLPqzZwqGk75lPMSc825BGDdSsb1Svq3oJDnq+bcLZL734oNQJ1l0wGFyP5hGr5PYQrHRKShTldO7m4VGnXoHE5qoZ3f9n76+08ol/205O4kyaw58dHMQdXGgyfi9YGbMQuAwxB8FInsyW4r5Hua4gClIUOrO86NOexzAjg==;5:iIGGaUKtal/PmmWlL/osk9oRVTv1ah5kPcIJRP93kzzTpeB1c33321v+6cqnW6ulkaA+hVjp5qZk9UlLjRZOENln0peaxaBzbsU04RDYvlgOrqawPot3Wedviw3QUxGPmBjNQL+eEx9UcCbVsroz8Q==;24:qj+LuemRo32SoEW/kWj/pcb53mwOBPltKAblheAO63JOgw0enWOkBSJAjIzpVO1vAv8Chx/11sd4Szd3EFSew6qgtRopmN65Cn0d4oGlz0o=;7:lEID/XDvkOcA3SO4td5HKkaqbMtF4hYPVM9x7d6f99VgdiSmtPCCtNpHMXiNkzb7d9c6HBr/272HltTHIvNfVTLEMcfph/fdv7k3qEdwXaLNObja49oStaBORddKA+uIRBWBDo/tmEE0vpacis8Ew4QXkAnC/2knqXm7qlSW796v+s22lzw/4Z3MDKCp5qOVeylKgzrbhn9JbILaYDdbgg== SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2016 19:51:31.3181 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR07MB2256 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Madhavan, On Wed, Jun 15, 2016 at 05:12:53PM +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 reading the wrong word. In find_first_bit(), > one word at a time (based on BITS_PER_LONG) is loaded and > checked for any bit set. In 32bit BE userspace, > BITS_PER_LONG turns out to be 32, and for a mask value of > "0x00000000000000ff", find_first_bit will return 32, instead of 0. > Reason for this is that, value in the word0 is all zeros and value > in word1 is 0xff. Ideally, second word in the mask should be loaded > and searched. Patch swaps the word to look incase of 32bit BE. I think this is not a problem of find_bit() at all. You have wrong typecast as the source of problem (tools/perf/util/session.c"): 940 static void regs_dump__printf(u64 mask, u64 *regs) 941 { 942 unsigned rid, i = 0; 943 944 for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) { ^^^^ Here ^^^^ 945 u64 val = regs[i++]; 946 947 printf(".... %-5s 0x%" PRIx64 "\n", 948 perf_reg_name(rid), val); 949 } 950 } But for some reason you change correct find_bit()... Though proper fix is like this for me: static void regs_dump__printf(u64 mask, u64 *regs) { unsigned rid, i = 0; unsigned long _mask[sizeof(mask)/sizeof(unsigned long)]; _mask[0] = mask & ULONG_MAX; if (sizeof(mask) > sizeof(unsigned long)) _mask[1] = mask >> BITS_PER_LONG; for_each_set_bit(rid, _mask, sizeof(mask) * BITS_PER_BYTE) { u64 val = regs[i++]; printf(".... %-5s 0x%" PRIx64 "\n", perf_reg_name(rid), val); } } Maybe there already is some macro doing the conversion for you... Yury. > Cc: Arnaldo Carvalho de Melo > Cc: Adrian Hunter > Cc: Borislav Petkov > Cc: David Ahern > Cc: George Spelvin > Cc: Jiri Olsa > Cc: Namhyung Kim > Cc: Rasmus Villemoes > Cc: Wang Nan > Cc: Yury Norov > Cc: Michael Ellerman > Signed-off-by: Madhavan Srinivasan > --- > tools/lib/find_bit.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/tools/lib/find_bit.c b/tools/lib/find_bit.c > index 9122a9e80046..996b3e04324f 100644 > --- a/tools/lib/find_bit.c > +++ b/tools/lib/find_bit.c > @@ -37,7 +37,12 @@ static unsigned long _find_next_bit(const unsigned long *addr, > if (!nbits || start >= nbits) > return nbits; > > +#if (__BYTE_ORDER == __BIG_ENDIAN) && (BITS_PER_LONG != 64) > + tmp = addr[(((nbits - 1)/BITS_PER_LONG) - (start / BITS_PER_LONG))] > + ^ invert; > +#else > tmp = addr[start / BITS_PER_LONG] ^ invert; > +#endif > > /* Handle 1st word. */ > tmp &= BITMAP_FIRST_WORD_MASK(start); > @@ -48,7 +53,12 @@ static unsigned long _find_next_bit(const unsigned long *addr, > if (start >= nbits) > return nbits; > > +#if (__BYTE_ORDER == __BIG_ENDIAN) && (BITS_PER_LONG != 64) > + tmp = addr[(((nbits - 1)/BITS_PER_LONG) - (start / BITS_PER_LONG))] > + ^ invert; > +#else > tmp = addr[start / BITS_PER_LONG] ^ invert; > +#endif > } > > return min(start + __ffs(tmp), nbits); > @@ -75,8 +85,15 @@ unsigned long find_first_bit(const unsigned long *addr, unsigned long size) > unsigned long idx; > > for (idx = 0; idx * BITS_PER_LONG < size; idx++) { > +#if (__BYTE_ORDER == __BIG_ENDIAN) && (BITS_PER_LONG != 64) > + if (addr[(((size-1)/BITS_PER_LONG) - idx)]) > + return min(idx * BITS_PER_LONG + > + __ffs(addr[(((size-1)/BITS_PER_LONG) - idx)]), > + size); > +#else > if (addr[idx]) > return min(idx * BITS_PER_LONG + __ffs(addr[idx]), size); > +#endif > } > > return size; > -- > 1.9.1