hi,

> > > > > Yes, for it to happen we have to have
> > > > > wandered into the weeds, but this code methodology is to try and help us
>  
> > > > > get somewhere a bit safer. i.e. not add more error states on top if we 
> > > > > don't have to.
> > > > 
> > > > we have many code which assume that vn_lock with LK_RETRY never fails.
> > > > are you going to add error checks on all of them?
> > > > i think that it just bloats the code without any benefits.
> > > 
> > > The difference here is that you're changing the state of PDIRUNLOCK. Don't 
> > > clear it if you don't know the lock succeeded.
> > 
> > actually, there's no much differences.
> > if vn_lock(LK_RETRY) can fail,
> > all callers should check it and shouldn't unlock the vnode after an error.
> > it's better to make sure that vn_lock(LK_RETRY) won't fail rather than
> > letting all callers check error conditions, IMO.
> 
> Well, then I guess we'll have to disagree. I'm always concerned by what 
> happens if a routine that "can't fail" realizes it is going to fail. It 
> either has to do something like panic, or it has to pretend things are ok. 
> If instead it can fail, it can tell the upper layers that something is 
> wrong. They can choose to not do anything, but they've been told.

there's no problem to having a routine that won't fail.
and there's no problem not to checking its failure.

i can understand your concern, given that vnode lock's implemention is
somewhat complicated unnecessarily.
it's better just to simplify it.

> Also, consider something like a distributed file system with a lock 
> manager (as in a separate server). Say you can't contact the lock manager. 
> Do you really sit there in uninterruptable sleep for ever? Yes, how and 
> when you decide to give up is something the admin decides (as it should 
> be). But the scenario shows a reason to have vn_lock(LK_RETRY) fail.

no.
such a distributed filesystem is broken in design, IMO.
it's a horrible idea to use vnode locks for remote locking.

btw, please answer my first question.
are you going to add checks on every vn_lock(LK_RETRY)?

YAMAMOTO Takashi