Subject: Re: ufs-ism in lookup(9)
To: None <wrstuden@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 03/23/2004 09:04:52
hi,

> > > Well, given that you have patches to every other file system, it does not 
> > > seem at ALL ufs-centric. Thus "ufs-ism" is not appropriate.
> > 
> > i guess that all of them are inherited from ufs.
> 
> The "guess" aspect of this thread is what's irritating me. And what makes 
> me nervous about your changes. If you're "guessing" about the code, you 
> really shouldn't be changing the lookup path. At all.

i'm guessing about the history.  not about the code itself.

> > ufs needs this because ufs_remove relies upon the info stored in
> > in-core inode by ufs_lookup is valid.  eg. i_offset.
> 
> Indeed. I had forgotten that part. However given your gentle reminder,
> don't you see why this behavior is correct for ANY local file system that
> supports hard links to files (given our name cache setup), and may be true
> for other local file systems? It's a fundamental behavior, given our name 
> cache setup.

ok, if you don't like the word "ufs-ism", i'll stop using it.
it isn't my point at all.  my point is, it's better to move
the assumptions for a particular class of filesystem implementations
to their fs-dependent code.

> > currently, when you do "rm somefileonnfs",
> > our nfs client always issues a LOOKUP rpc before a REMOVE rpc even if
> > the "somefileonnfs" is on the namecache.  it's what i'd like to change.
> > 
> > as i said in the first mail, it could be achieved in nfs code.
> > however, i think that it's cleaner to push what i called ufs-ism into
> > ufs-like filesystems themselves.
> 
> You missed my point. You have skipped from "step A" to "step E" of your
> thought process. I wanted you to work through the steps. Mainly since I
> _think_ that at "step C" we can do things differently, and make both you
> and me happy. However since you aren't stepping through things, it's hard
> to tell.

i don't see what's unclear, sorry.
i described the problem and the code is there.

> The part of your change I object to the most is your change to 
> vfs_lookup.c. From reading the comments, the code to disable caching in 
> the delete case is exactly correct, and the not-CREATE case seems ok 
> (though perhaps a bit questionable). docache's definition is, after all, 
> /* == 0 do not cache last component */.

> Thoughts?

you believe the comment while i believe the code when they disagree.
i'd say the comments are wrong, however, ok, i have no particular problem
with your suggested way.  (ie. change the MAKEENTRY's behaviour)

YAMAMOTO Takashi