Subject: Re: vn_lock(LK_RETRY) (was: Re: CVS commit: src/sys/miscfs)
To: None <email@example.com>
From: YAMAMOTO Takashi <firstname.lastname@example.org>
Date: 06/19/2004 14:19:57
> > > > > 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.
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)?