From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Laight Date: Wed, 04 Jun 2014 13:34:08 +0000 Subject: RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit Message-Id: <063D6719AE5E284EB5DD2968C1650D6D172571DB@AcuExch.aculab.com> List-Id: References: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr> <063D6719AE5E284EB5DD2968C1650D6D1725705F@AcuExch.aculab.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: 'Julia Lawall' , Geert Uytterhoeven Cc: linux-rdma , "kernel-janitors@vger.kernel.org" , Linux Fbdev development list , Linux-sh list , "linux-kernel@vger.kernel.org" , "ath10k@lists.infradead.org" , linux-wireless , "netdev@vger.kernel.org" , driverdevel , "iss_storagedev@hp.com" , scsi , linux-s390 , "adi-buildroot-devel@lists.sourceforge.net" , Arnd Bergmann , "sebott@linux.vnet.ibm.com" From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > > OK, thanks. I was only looking at the C code. > > > > > > But the C code contains a loop that is followed by: > > > > > > if (!size) > > > return result; > > > tmp = *p; > > > > > > found_first: > > > tmp |= ~0UL << size; > > > if (tmp = ~0UL) /* Are any bits zero? */ > > > return result + size; /* Nope. */ > > > > > > In the first return, it would seem that result = size. Could the second > > > one be changed to just return size? It should not hurt performance. > > > > "size" may have been changed between function entry and this line. > > So you have to store it in a temporary. > > Sorry, after reflection it seems that indeed size + result is always the > original size, so it is actually all of the code that uses >= that is > doing something unnecessary. = for the failure test is fine. There is nothing wrong with defensive coding. The 'tmp |= ~0UL << size' ensures that the return value is 'correct' when there are no bits set. The function could have been defined so that this wasn't needed. If you assume that the 'no zero bits' is unlikely, then checking the return value from ffz() could well be slightly faster. Not that anything is likely to notice. David From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx0.aculab.com ([213.249.233.131]:58179 "HELO mx0.aculab.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753254AbaFDNec convert rfc822-to-8bit (ORCPT ); Wed, 4 Jun 2014 09:34:32 -0400 Received: from mx0.aculab.com ([127.0.0.1]) by localhost (mx0.aculab.com [127.0.0.1]) (amavisd-new, port 10024) with SMTP id 06368-08 for ; Wed, 4 Jun 2014 14:34:29 +0100 (BST) From: David Laight To: 'Julia Lawall' , Geert Uytterhoeven CC: linux-rdma , "kernel-janitors@vger.kernel.org" , Linux Fbdev development list , Linux-sh list , "linux-kernel@vger.kernel.org" , "ath10k@lists.infradead.org" , linux-wireless , "netdev@vger.kernel.org" , driverdevel , "iss_storagedev@hp.com" , scsi , linux-s390 , "adi-buildroot-devel@lists.sourceforge.net" , Arnd Bergmann , "sebott@linux.vnet.ibm.com" Subject: RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit Date: Wed, 4 Jun 2014 13:34:08 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6D172571DB@AcuExch.aculab.com> (sfid-20140604_153455_401557_9CF03577) References: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr> <063D6719AE5E284EB5DD2968C1650D6D1725705F@AcuExch.aculab.com> In-Reply-To: Content-Type: text/plain; charset="Windows-1252" MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > > OK, thanks. I was only looking at the C code. > > > > > > But the C code contains a loop that is followed by: > > > > > > if (!size) > > > return result; > > > tmp = *p; > > > > > > found_first: > > > tmp |= ~0UL << size; > > > if (tmp == ~0UL) /* Are any bits zero? */ > > > return result + size; /* Nope. */ > > > > > > In the first return, it would seem that result == size. Could the second > > > one be changed to just return size? It should not hurt performance. > > > > "size" may have been changed between function entry and this line. > > So you have to store it in a temporary. > > Sorry, after reflection it seems that indeed size + result is always the > original size, so it is actually all of the code that uses >= that is > doing something unnecessary. == for the failure test is fine. There is nothing wrong with defensive coding. The 'tmp |= ~0UL << size' ensures that the return value is 'correct' when there are no bits set. The function could have been defined so that this wasn't needed. If you assume that the 'no zero bits' is unlikely, then checking the return value from ffz() could well be slightly faster. Not that anything is likely to notice. David From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753509AbaFDNej (ORCPT ); Wed, 4 Jun 2014 09:34:39 -0400 Received: from mx0.aculab.com ([213.249.233.131]:58216 "HELO mx0.aculab.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753481AbaFDNeg convert rfc822-to-8bit (ORCPT ); Wed, 4 Jun 2014 09:34:36 -0400 From: David Laight To: "'Julia Lawall'" , Geert Uytterhoeven CC: linux-rdma , "kernel-janitors@vger.kernel.org" , Linux Fbdev development list , Linux-sh list , "linux-kernel@vger.kernel.org" , "ath10k@lists.infradead.org" , linux-wireless , "netdev@vger.kernel.org" , driverdevel , "iss_storagedev@hp.com" , scsi , linux-s390 , "adi-buildroot-devel@lists.sourceforge.net" , Arnd Bergmann , "sebott@linux.vnet.ibm.com" Subject: RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit Thread-Topic: [PATCH 0/10] use safer test on the result of find_first_zero_bit Thread-Index: AQHPf9jD3P2Wp/I7I0O0KpvEWFb8KZtgszAA///xwwCAABGWgIAAAUyAgAACFQCAACLfgIAAFbFw Date: Wed, 4 Jun 2014 13:34:08 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6D172571DB@AcuExch.aculab.com> References: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr> <063D6719AE5E284EB5DD2968C1650D6D1725705F@AcuExch.aculab.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.202.99.200] Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > > OK, thanks. I was only looking at the C code. > > > > > > But the C code contains a loop that is followed by: > > > > > > if (!size) > > > return result; > > > tmp = *p; > > > > > > found_first: > > > tmp |= ~0UL << size; > > > if (tmp == ~0UL) /* Are any bits zero? */ > > > return result + size; /* Nope. */ > > > > > > In the first return, it would seem that result == size. Could the second > > > one be changed to just return size? It should not hurt performance. > > > > "size" may have been changed between function entry and this line. > > So you have to store it in a temporary. > > Sorry, after reflection it seems that indeed size + result is always the > original size, so it is actually all of the code that uses >= that is > doing something unnecessary. == for the failure test is fine. There is nothing wrong with defensive coding. The 'tmp |= ~0UL << size' ensures that the return value is 'correct' when there are no bits set. The function could have been defined so that this wasn't needed. If you assume that the 'no zero bits' is unlikely, then checking the return value from ffz() could well be slightly faster. Not that anything is likely to notice. David From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: From: David Laight Subject: RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit Date: Wed, 4 Jun 2014 13:34:08 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6D172571DB@AcuExch.aculab.com> References: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr> <063D6719AE5E284EB5DD2968C1650D6D1725705F@AcuExch.aculab.com> In-Reply-To: Content-Language: en-US MIME-Version: 1.0 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org List-Archive: To: 'Julia Lawall' , Geert Uytterhoeven Cc: driverdevel , linux-s390 , Linux Fbdev development list , scsi , "iss_storagedev@hp.com" , Linux-sh list , linux-rdma , linux-wireless , "kernel-janitors@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "ath10k@lists.infradead.org" , "adi-buildroot-devel@lists.sourceforge.net" , Arnd Bergmann , "netdev@vger.kernel.org" , "sebott@linux.vnet.ibm.com" List-ID: From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > > OK, thanks. I was only looking at the C code. > > > > > > But the C code contains a loop that is followed by: > > > > > > if (!size) > > > return result; > > > tmp = *p; > > > > > > found_first: > > > tmp |= ~0UL << size; > > > if (tmp == ~0UL) /* Are any bits zero? */ > > > return result + size; /* Nope. */ > > > > > > In the first return, it would seem that result == size. Could the second > > > one be changed to just return size? It should not hurt performance. > > > > "size" may have been changed between function entry and this line. > > So you have to store it in a temporary. > > Sorry, after reflection it seems that indeed size + result is always the > original size, so it is actually all of the code that uses >= that is > doing something unnecessary. == for the failure test is fine. There is nothing wrong with defensive coding. The 'tmp |= ~0UL << size' ensures that the return value is 'correct' when there are no bits set. The function could have been defined so that this wasn't needed. If you assume that the 'no zero bits' is unlikely, then checking the return value from ffz() could well be slightly faster. Not that anything is likely to notice. David _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Laight Date: Wed, 04 Jun 2014 13:34:08 +0000 Subject: RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit Message-Id: <063D6719AE5E284EB5DD2968C1650D6D172571DB@AcuExch.aculab.com> List-Id: References: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr> <063D6719AE5E284EB5DD2968C1650D6D1725705F@AcuExch.aculab.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: 'Julia Lawall' , Geert Uytterhoeven Cc: driverdevel , linux-s390 , Linux Fbdev development list , scsi , "iss_storagedev@hp.com" , Linux-sh list , linux-rdma , linux-wireless , "kernel-janitors@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "ath10k@lists.infradead.org" , "adi-buildroot-devel@lists.sourceforge.net" , Arnd Bergmann , "netdev@vger.kernel.org" , "sebott@linux.vnet.ibm.com" From: Julia Lawall > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote: > > > Hi Julia, > > > > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote: > > > OK, thanks. I was only looking at the C code. > > > > > > But the C code contains a loop that is followed by: > > > > > > if (!size) > > > return result; > > > tmp = *p; > > > > > > found_first: > > > tmp |= ~0UL << size; > > > if (tmp = ~0UL) /* Are any bits zero? */ > > > return result + size; /* Nope. */ > > > > > > In the first return, it would seem that result = size. Could the second > > > one be changed to just return size? It should not hurt performance. > > > > "size" may have been changed between function entry and this line. > > So you have to store it in a temporary. > > Sorry, after reflection it seems that indeed size + result is always the > original size, so it is actually all of the code that uses >= that is > doing something unnecessary. = for the failure test is fine. There is nothing wrong with defensive coding. The 'tmp |= ~0UL << size' ensures that the return value is 'correct' when there are no bits set. The function could have been defined so that this wasn't needed. If you assume that the 'no zero bits' is unlikely, then checking the return value from ffz() could well be slightly faster. Not that anything is likely to notice. David