From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161059AbcFOVLQ (ORCPT ); Wed, 15 Jun 2016 17:11:16 -0400 Received: from mail-bl2on0099.outbound.protection.outlook.com ([65.55.169.99]:42300 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933044AbcFOVLN (ORCPT ); Wed, 15 Jun 2016 17:11:13 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Yuri.Norov@caviumnetworks.com; Date: Thu, 16 Jun 2016 00:11:04 +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: <20160615211104.GA6978@yury-N73SV> References: <1465990973-31483-1-git-send-email-maddy@linux.vnet.ibm.com> <20160615195127.GA6039@yury-N73SV> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160615195127.GA6039@yury-N73SV> User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [50.233.148.158] X-ClientProxiedBy: CY1PR14CA0079.namprd14.prod.outlook.com (10.164.65.175) To BN4PR07MB2244.namprd07.prod.outlook.com (10.164.63.150) X-MS-Office365-Filtering-Correlation-Id: 14dee812-ceb9-4435-c841-08d395619385 X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2244;2:qXy9JSU16q4nz12fA2gKZ/oUEL3DfP4843olhVUdJRWsmPu4iczTsJRWg5QWgoMyZ7dfF80Jeq8QMBuAB29lNZYgLfVuSagMY5PuFRfL6xSnHw/QYHeYRI65XaflZDxIH1ZIrPjvnUP7xt1DvHvuubDJqO5e9UnhfHi+49MZJR6MoHTU4UuYIERG7g3SCYxi;3:JudQBtLZFRKlqcSUkExglMbEI3+YDBw0FjV71csAkugyyv3EkQF3nO1i8FEyhJaw2EnssDHYL8pCZVRUlAV5fSWbmwO3UByKjgbdz2bV6Avmd/JU0bolAxy5T06FRtcg;25:T6nHXGzMFD3bnIWlH2+PuqkkqfTS3ZdYC9ogis/IccYPDhG9etiMvEug0Wt1U8n931lfGuWVLTKHENL1h5pNU+F8i4mWuEYvegSh9jq/cEu6Vrijmujh+KUOJigjtnFwWEG51uNaoVxnwtAFfKTl2TrNKWx1i5SaROD5UuCaPgC96fL53pHNNe3vDVTjIyMCAyONHP240RAQYMtyZzshTsYLq4zTpwhEwRPgyhe1jbCX/+POFgZndZZyvl4fs184qvHF06G0/wIH2XzIP2ftxkgmBDJs6b+WY9KqN9JZV8bdojzCeCOsUDJ54EpYaDg2eNCV4VD7S4etozZ8JOn9Xjj9n8nU/WA4yyd+rMryh9T8XtNuIQhvMvjN1fiCeajT572WancRKDfcnbKp35TcOqOUiUN5uo3X7yREmgnVuvE= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN4PR07MB2244; X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2244;20:TG3YUlFVGpDbtP3FYlwHeE9NR2mzvaBM5YOjEZ4E0pM8Cta6O+eaO3hy048ql8vTtFNQt71y4Ja+0qeQRGdPrfJ8vtLpop4b7JnB/Xzsf07G8gyZQt/cTYDh7lsI0a47KlMej+zP9xgJfE25tIYqcXVnTvtcrI0e3gLao0cvEEH3bDNy5NcatKHPiOGNqwrWsreEKHjSV85AjMRWXLAtHTZgoo/9QTXDw8qR6iZjV/+rg7mbKYAcdvmndE8d6J4hGGJwKIZxOX4jU/y01x4lDVvVR7iEAGiTIkj9FIkEMrWG7cnDUVoYWvAEu0DHlj7c/VJ51AYxA3Bxz80kw6+B3w9BqWNarGVTCsPJBvx1466C+aNwN0umlZb/AN7h4W1Oy+NL2W19vTQ2QDWAXejUWB1muJOW/sOdvP8utYufijydW4bNpoySoXE1/28qrwrFY9pBT3BSBSsmxk0PfU6eLCE4N1XBSM0C65Q5YiSAj6Og9FEtvce55ySRxbDrtBJSitJjL3VN8VMP/CrYjZot/bbeZQ/KRzGKxsA38OyGuh6XREAgZ1BH4hWlTwfWPflsdrqkYs/jt2+dTxy+uSPfIX9yhWodlZ2e0Pdiv9drHrE= 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:BN4PR07MB2244;BCL:0;PCL:0;RULEID:;SRVR:BN4PR07MB2244; X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2244;4:2Pulo0WX2r5yc2r+uIZweMUO24g8/lA4gt2X9qklDF2PVdEbXyK4VZ/yEjHhsEILKNJQY6jCV65Wq6RvM7PeYOC9KdJ2FUHs5T70Cnml2sblo1xHrYDvjpgpUtq7e4ENYC1IN923pevA7sRV3ru7ATrVNej8AwRcx2D1igeIwnSTcx1cMYBk8jWDRnwMwiQHzPCofQAIgc3xlC0E9aMOf+LKgJNEs5tP6SFFB9fyDbpN/Y2UPq/UvrboJ/kgsYlzKhRsg88ASK0zOcPZotInqUcAM+ytoAQoKYZ8LQG3wjYdTsK2jxHESVJEjPWr90vgCa1grlou+E+FOXUoUC0TlBXyRpWWmEtq80zD+/qOtbzjhPLQ+yISY5OzH4iUzP5uGbYKESHFjbtlrUgQdDraHeSpchiTCTcPGeVP/DTDELa4LjxE94L/LASrrbLoGWNiCQH6r9x/aOSsbW/ASOFppqOS9AVP0LqRj0+W2Tx8S0A= X-Forefront-PRVS: 09749A275C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(6069001)(7916002)(189002)(24454002)(199003)(92566002)(33716001)(189998001)(97736004)(2906002)(97756001)(9686002)(50466002)(2950100001)(4326007)(1076002)(77096005)(68736007)(4001350100001)(110136002)(5008740100001)(54356999)(3846002)(101416001)(586003)(23726003)(66066001)(19580405001)(50986999)(76176999)(19580395003)(6116002)(46406003)(83506001)(81156014)(42186005)(33656002)(47776003)(106356001)(105586002)(8676002)(76506005)(5004730100002)(81166006);DIR:OUT;SFP:1101;SCL:1;SRVR:BN4PR07MB2244;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;BN4PR07MB2244;23:ExhtpzUhe/0dywF5QFKWv4rrbRPCUGrQB4yxBvxQf?= =?us-ascii?Q?Cz9af1fEXtO9sxc4y6ITdGoE525AgZix80gqyrJYNqVJW4L+iGaCRg8dvPd8?= =?us-ascii?Q?wfvSduKgtYMFAvkh9s9VD8+qwZ/Fr6QkPdWoY4Cl+S0yWFfBFcnTsxlVqaNt?= =?us-ascii?Q?1bTZ+ebOD2FP0fTxbh7Hg4swOOwYLmainYriVHShWzcjsTagUDy4Tmxpsslj?= =?us-ascii?Q?NmKihlTAa+RlP4b9hI/RgmTm4DUSFhZssw1gZfoOM3UuzzZ2XEjzy305Lj+A?= =?us-ascii?Q?5Qq6bk+UpNpfIetT/NOUMvsTPf8xdeXmGsBZS4oFMJl7cGTB1+/Ki5j0lfo+?= =?us-ascii?Q?7Y5miIXW6Jof19Twyolzji/wT2yttsVRXg+JdD4jR9yYhmyXACGzXwxLYxnt?= =?us-ascii?Q?9/g6yZ+PSZJUnxEL/2RBJEcZEoJ9bXRNLNdRTpb78vF2A/1svFUEhXUa9m/O?= =?us-ascii?Q?K1leKa6vkd4mPU0cbk7/rTddf7957rY5YSyGFb39x4K3ll+JQe01RdMyT/T+?= =?us-ascii?Q?8vJ/p4BYXOTIW4Ackp4f3by07rlwYBH9u+InjAeEmSFuSJJJSo/li6Jmzwxk?= =?us-ascii?Q?upsm+7Jgg4tR4RJS8vA8z1aMxKzNaNd9b1a8rAkPqrBzHhvxgzVRUoM5nCWH?= =?us-ascii?Q?xPsOBHn92Bf3qcT4zOF6WVTBvLN9TxBU3GtCXcL6MC1IpqOPLC2djsQiy9FQ?= =?us-ascii?Q?oS+p9kk0kdsZOxYwgMYZZWdzc+ROrMzeehyGQhSXUcryZwlf44XM3kgJpScF?= =?us-ascii?Q?3usbeCeRdu5fghDhQMVfeSafi1wmnQz9ZfA+SWiMRSY6xJ2ENsdCDcYsN/sq?= =?us-ascii?Q?Uw6uvQBfQPLcsZGPnXreJeXl4VHhmU09fZqLmmSmiX8aDX+K6CnEzaUfjew8?= =?us-ascii?Q?tRBtANv3xKi0EssfmGtENCZduwzmUi/q5gOv6JZhTe4huUQa6iWqblEZRlIT?= =?us-ascii?Q?B3kP3Kl+EKdSfaUC+Q+JqivOU8808lZppYHS9x2SFlzh5Rfwvu4IHkGalqXC?= =?us-ascii?Q?CHWeZVRV28wNebt9jKDyl0/fPuBx7vKLWAotrMAQ6JFYZEx/EsXzcJbYQQUd?= =?us-ascii?Q?OMIPgCfclSFkNGiO1ruZeN6DBAcDMUP4Ly5yItAP+lAchKKv9naLK5eaFm8u?= =?us-ascii?Q?ZAQYr10o7iAUkg6mxKhf8e436LP0KeXC6RxQ61Uaw9YraRjtxSwntQJr6qKP?= =?us-ascii?Q?zWQuD7vCvMyIpc=3D?= X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2244;6:EchXlx6TqoYyTLisEhbuLagqDPpWjL1UpgiKrphoAThLG8FBYJrUBnVSCkwTe3RNgxGdSsxE25zw/2t+Dpk9sL+hDddPy4bFmswKRMr+duZIoH0pLrIqZ/1fgKehCuRgH8l1xeX+AG8dQ2UxFkUeWA2SIO63kbo5129lJHYpJFoc1EnuECQ60UHzr0+ReJXBDE8LrocGO/vhyJfQYxqspmgNlmrMt3p+kolI9NrePWHLpWya4no9xr6oidRRZ/WzwjYFb//OTLHGsVEETceniQ==;5:SCD8UpihmWzUigoQISTlZs9fUB1tFZLXyt42ZgP3FaH/Trzmwkkm/inI/1JHAJit/fxWY5pbfDUPIE3PhBZyCrV6t/5Kyg3XJMNb6CWzqAwytQ6UVcBzpvEf5WRKFJodAC6aydu50eGtQl58nZzAZA==;24:mvz9b4l0R1oZZjVMkjEucD5TsHzxuLRUn2Y5ziLZOCR5mkkemD/Gb/8sShoXuGCmHGrRJEMh1m6KsmWDNgTeEwFnEq7coJDZ8cEI6JJ759k=;7:eubvIWWdz1bH9gncH5x2Dw3HnS2ajT5cMckvmgg+A3rjY6IAeIyJKbhJYw+1MOp0ejkAKoXlIGxYbsKEu+K/oDaCZGyw2yaM0v80w4FcuwwfcI9ORu92VsmpWIvNcAEIIznwPhMYxS9uxua0Mz4Nbuab+rn/W5Yk/xW7/FaOF7NFhsbi6saViwuzMKSxVBc1yvSGZzPrgPtiVwJQK0lMjw== SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2016 21:11:10.6794 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN4PR07MB2244 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 15, 2016 at 10:51:27PM +0300, Yury Norov wrote: > 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... yes it is, cpu_to_le64() is what you want > > 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