* Why do we pass in a directory and a dentry to lookup() and rename()
@ 2010-12-26 21:45 Theodore Ts'o
[not found] ` <AANLkTikS1BMu+DyZEVZ9H6daoh5dJM7uGPUB9ugdHuRA@mail.gmail.com>
2010-12-28 9:46 ` Neil Brown
0 siblings, 2 replies; 4+ messages in thread
From: Theodore Ts'o @ 2010-12-26 21:45 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Why do we pass in a directory and a dentry to lookup() and rename()
[not found] ` <AANLkTikS1BMu+DyZEVZ9H6daoh5dJM7uGPUB9ugdHuRA@mail.gmail.com>
@ 2010-12-27 5:50 ` Ted Ts'o
0 siblings, 0 replies; 4+ messages in thread
From: Ted Ts'o @ 2010-12-27 5:50 UTC (permalink / raw)
To: Rajat Sharma; +Cc: Al Viro, linux-fsdevel
On Mon, Dec 27, 2010 at 11:14:05AM +0530, Rajat Sharma wrote:
> Hi Theodore,
>
> In both the operations, inodes refer to parent directory while dentry refers
> to child. Hope this clarifies.
Sure, but we can get to the parent directory's inode from the dentry via:
dentry->d_parent->d_inode
... can we not?
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Why do we pass in a directory and a dentry to lookup() and rename()
2010-12-26 21:45 Why do we pass in a directory and a dentry to lookup() and rename() Theodore Ts'o
[not found] ` <AANLkTikS1BMu+DyZEVZ9H6daoh5dJM7uGPUB9ugdHuRA@mail.gmail.com>
@ 2010-12-28 9:46 ` Neil Brown
2011-01-05 11:26 ` Al Viro
1 sibling, 1 reply; 4+ messages in thread
From: Neil Brown @ 2010-12-28 9:46 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Al Viro, linux-fsdevel
On Sun, 26 Dec 2010 16:45:51 -0500 "Theodore Ts'o" <tytso@mit.edu> wrote:
> 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?
>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Why do we pass in a directory and a dentry to lookup() and rename()
2010-12-28 9:46 ` Neil Brown
@ 2011-01-05 11:26 ` Al Viro
0 siblings, 0 replies; 4+ messages in thread
From: Al Viro @ 2011-01-05 11:26 UTC (permalink / raw)
To: Neil Brown; +Cc: Theodore Ts'o, linux-fsdevel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-01-05 11:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-26 21:45 Why do we pass in a directory and a dentry to lookup() and rename() Theodore Ts'o
[not found] ` <AANLkTikS1BMu+DyZEVZ9H6daoh5dJM7uGPUB9ugdHuRA@mail.gmail.com>
2010-12-27 5:50 ` Ted Ts'o
2010-12-28 9:46 ` Neil Brown
2011-01-05 11:26 ` Al Viro
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.