Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S261694AbVC0Oay (ORCPT ); Sun, 27 Mar 2005 09:30:54 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S261693AbVC0Oay (ORCPT ); Sun, 27 Mar 2005 09:30:54 -0500 Received: from pentafluge.infradead.org ([213.146.154.40]:14545 "EHLO pentafluge.infradead.org") by vger.kernel.org with ESMTP id S261685AbVC0Oab (ORCPT ); Sun, 27 Mar 2005 09:30:31 -0500 Subject: Re: [PATCH] remove redundant NULL pointer checks prior to calling kfree() in fs/nfsd/ From: Arjan van de Ven To: Denis Vlasenko Cc: linux-os@analogic.com, Jesper Juhl , Neil Brown , nfs@lists.sourceforge.net, Trond Myklebust , linux-kernel@vger.kernel.org In-Reply-To: <200503271545.28335.vda@ilport.com.ua> References: <1111826041.6293.31.camel@laptopd505.fenrus.org> <200503271545.28335.vda@ilport.com.ua> Content-Type: text/plain Date: Sun, 27 Mar 2005 16:30:19 +0200 Message-Id: <1111933819.6297.45.camel@laptopd505.fenrus.org> Mime-Version: 1.0 X-Mailer: Evolution 2.0.4 (2.0.4-2) Content-Transfer-Encoding: 7bit X-Spam-Score: 3.7 (+++) X-Spam-Report: SpamAssassin version 2.63 on pentafluge.infradead.org summary: Content analysis details: (3.7 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 1.1 RCVD_IN_DSBL RBL: Received via a relay in list.dsbl.org [] 2.5 RCVD_IN_DYNABLOCK RBL: Sent directly from dynamic IP address [80.57.133.107 listed in dnsbl.sorbs.net] 0.1 RCVD_IN_SORBS RBL: SORBS: sender is listed in SORBS [80.57.133.107 listed in dnsbl.sorbs.net] X-SRS-Rewrite: SMTP reverse-path rewritten from by pentafluge.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1753 Lines: 45 On Sun, 2005-03-27 at 15:45 +0300, Denis Vlasenko wrote: > On Saturday 26 March 2005 10:34, Arjan van de Ven wrote: > > On Fri, 2005-03-25 at 17:34 -0500, linux-os wrote: > > > On Fri, 25 Mar 2005, Jesper Juhl wrote: > > > > > > > (please keep me on CC) > > > > > > > > > > > > checking for NULL before calling kfree() is redundant and needlessly > > > > enlarges the kernel image, let's get rid of those checks. > > > > > > > > > > Hardly. ORing a value with itself and jumping on condition is > > > real cheap compared with pushing a value into the stack > > > > which century are you from? > > "jumping on condition" can easily be 100+ cycles, depending on how > > effective the branch predictor is. Pushing a value onto the stack otoh > > is half a cycle. > > linux-os is right because kfree does NULL check with exactly > the same code sequence, test and branch: I know it does. The thing is that you have *two* chances to get a branch mispredict now. Now if kfree did NOT do the if but move it always to the caller, then you have somewhat different dynamics (since you then always if the conditional jump once no matter) but that is not the case. > I conclude that if(p) kfree(p) makes sense only if: > a) p is more often NULL than not, and > b) it's in the hot path (you don't want to save on code size) > > Since (a) is not typical, I think Jesper's cleanups are ok. note that to gain from teh branch predictor "more often than not" probably needs to be in the 20:1 ratio to actually gain. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/