From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: Why do we pass in a directory and a dentry to lookup() and rename() Date: Wed, 5 Jan 2011 11:26:05 +0000 Message-ID: <20110105112605.GM19804@ZenIV.linux.org.uk> References: <20101228204619.4f9e1722@notabene.brown> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Theodore Ts'o , linux-fsdevel@vger.kernel.org To: Neil Brown Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:54454 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751569Ab1AEL0K (ORCPT ); Wed, 5 Jan 2011 06:26:10 -0500 Content-Disposition: inline In-Reply-To: <20101228204619.4f9e1722@notabene.brown> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Dec 28, 2010 at 08:46:19PM +1100, Neil Brown wrote: > Some times it isn't safe to de-reference ->d_parent without extra locking > (via dget_parent) so there could be cases where passing an > already-ref-counted pointer might be cleaner, but during lookup and rename > there must be sufficient locking so that ->d_parent cannot change. > So all you could gain by passing the pointer separately is avoiding a single > memory reference. As d_parent is almost certainly in cache when these are > called, that would not be a big gain. > > It might be worth checking that the code doesn't get bigger if you drop the > arg and use ->d_parent instead, but assuming it doesn't (which seems likely), > I would agree that passing the parent inode explicitly is unnecessary. In fact, all directory operations have relevant ->d_parent stable; a lot of instances rely on that, actually. So locking isn't a problem. We could switch, but I'm not sure if it's worth the code churn. Do we really win anything there? On targets where everything is pushed on stack - probably yes, but I suspect that for majority of those methods it'll be a loss on just about every target. _Maybe_ it's not true for ->rename() (more arguments than for everything else), but... Hell knows; I suspect that it might be worth playing with if/when we go for explicit passing of creds to the methods. Extra argument plus the need to go through the instances anyway might make it worth bothering.