Why do we pass in a directory and a dentry to lookup() and rename()

In the inode operations functions lookup() and rename():

struct dentry * (*lookup)(struct inode *inode,
struct dentry *dentry, struct nameidata *nd);

int (*rename)(struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_entry);

... is the fact that we pass in the inode superfluous?

i.e., it looks like in all cases one can obtain the inode by looking at dentry->d_parent. Or am I missing something?

Thanks,

- Ted

Sure, but we can get to the parent directory's inode from the dentry via:

      dentry->d_parent->d_inode

... can we not?

- Ted

I'd guess that it is "historical reasons".

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.

NeilBrown

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.