Subject: Re: CVS commit: src/sys
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Chuck Silvers <chuq@chuq.com>
List: source-changes
Date: 12/28/2005 08:31:57
On Wed, Dec 28, 2005 at 02:13:56PM +0900, YAMAMOTO Takashi wrote:
> > > > One thing I noted in this change is that vget() will now call 
> > > > VOP_INACTIVE() on vnodes that were never activated. Will this cause issues 
> > > > with other file systems?
> > > 
> > > what kind of issues?
> > > afaik, vn_lock fails only when the vnode is being reclaimed.
> > > in that case, calling vrele doesn't hurt.
> > 
> > I'm also suspicious of this change.  this change doesn't seem related
> > to the purpose given in the check-in comment, is it?  if you really want
> > to reduce the amount of duplicate code, why not just split it into
> > multiple functions?
> 
> well, it might not be strictly related to the rest of the commit, right.
> 
> i think VOP_INACTIVE should be called here,
> at least when LK_SLEEPFAIL is involved.  (checkalias)
> (i don't think using LK_SLEEPFAIL is a good idea,
> but it's a separate topic.)

I don't like LK_SLEEPFAIL either.  or half the other crud in lockmgr().


> > also, do we really another flag that means the same thing as VXLOCK?
> > why not just use VXLOCK?  this would mean rearranging vgonel(), vclean(),
> > etc, a bit, but the result would be much nicer.
> 
> do you mean eg. introducing another flag to make vclean exclusive?

are you talking about the check that VXLOCK isn't already set at the top
of vclean()?  if people think it's important to have such a flag purely
for debugging, I suppose that would be ok.  it seems that there should be
better ways to check for that kind of thing though.  or did you mean
something else?

there are other problems with VXLOCK too, since vclean() isn't the only
path that sets it.  but I guess that won't matter for long.


> i agree it would be nicer.  however, as i have a plan to create
> a branch to separate vgone from vnode lock, and it can change this code,
> i don't want to work so hard to make this nicer at this point.

ok, it wasn't at all clear that this was a temporary measure.
I'll stop complaining about the current situation then.

could you post a summary of your new design when you're ready?
it would seem useful to discuss the design before starting with branches.

-Chuck