hi,

> > > While we could accept NFS's attempt to remove this directory (we'd trigger 
> > > silly-rename issues as the mount structure has a reference to the 
> > > directory, so it's not a free vnode), I think that would be sub-optimal. 
> > > So to stay consistent, we have to move a test into nfs_remove to check and 
> > > see if we have a vnode in-core (which we'd need anyway as we have to zap 
> > > said node), and check to see if it's mounted-on.
> > 
> > do you really want to support such a weird situation? :-)
> > if so, i think you want to eliminate f_mntonname and do getcwd-like thing
> > to get the path of the mountpoint.
> 
> I certainly don't want to get bitten by it. :-) I'm concerned it'd be a 
> new way to make the kernel fall over.. So in that way, I want to "support" 
> not dying.
> 
> Also, when exactly would we be doing this getcwd-like thing to get the 
> mountpoint path? How will we know when to (and not to) do the parent 
> directory lookup?

always when getfsstat is called?

> My point is that we are talking about (with the (dvp, component) cache)
> doing mount-point detection using something that is fragile for a remote
> file system, unless we can put some sort of watch (or lock or guard) on
> each path component.  Otherwise either the server or another client can
> come in and do a rename that will invalidate the usefullness of our cache.
> And we wouldn't even know it.

anyway it's fragile to mount filesystem on the remote vnode. :-)

i've been almost concerned that the (dvp, component) cache was not
a good idea.  maybe it's better to do the mountpoint test in
each dirop VOPs as you said.  i'm not sure if it's the best way, though.

> > > Sounds like the best thing to do for now is leave VOP_RENAME() alone.
> > 
> > the current "VOP_LOOKUP for dirop" method is wasteful even for ufs.
> > ie. in-core inode is bloated unnecessarily to store a result of VOP_LOOKUP.
> > i guess it's worse for more complicated directory implementations.
> 
> [sorry I mentioned VOP_RENAME(); we're talking about VOP_REMOVE()]
> 
> Huh?
> 
> For ufs, AFAICT we add two int32_t's to hold slot information. We also 
> use these values for file creation. So it doesn't seem like a waste to me.

it's a waste because the members are only used during dirops.
you can implement an internal version of ufs_lookup, which stores slot info
into kernel stack or somewhere instead of the in-core inode, and
call it from dirop VOPs.

> Also, how exactly do we eliminate the lookup for NFS? Yes, we could do 
> something like NOLEAF to the lookup, and we could push the parent and 
> component into VOP_REMOVE(). But fundamentally, as part of the remove 
> process, we have to know if we have an in-kernel vnode. How do we find 
> that vnode without doing a lookup? "Lookup" after all is the way we map a 
> name to an inode.

it needs lookup, but doesn't need VOP_LOOKUP, which always loads the vnode
as you said below.

> And since we have to do the lookup to find the vnode, what's wrong with
> what we're doing now, other than that we will always load the vnode into
> core even if it's not in-core?

isn't it enough?

YAMAMOTO Takashi