Subject: Re: LK_SHARED for VFS_VGET/FHTOVP
To: None <wrstuden@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 05/27/2004 11:02:30
hi,

> > > Specifically with using exclusive locks, I think that the vnode system was 
> > > fine-grained. Perhaps not the best locking (we can get better parallelism, 
> > > perhaps), but correct. By changing things so we need big-lock, we take a 
> > > step back.
> > 
> > what's "the vnode system"?
> > the vnode interface itsself, or eachfilesystem implementation,
> > or something else?
> 
> Both the interface and our file system implementations.

if you think that our filesystem implementations are mpsafe without biglock
because of LK_EXCLUSIVE, i have to say that you're too optimistic. :-)

> > > Over the years the vnode interface has gotten a reputation for being
> > > complicated and troublesome. When I was at NASA, I worked on cleaning lots
> > > of it up, and among other things we got production-ready layered file
> > > systems. Part of getting things to work was being an ass about locking,
> > > and making sure we did locking right in all places. "Right" meant 
> > > documenting what should happen and auditing all the code to match. To go 
> > > back to a comment you made about me believing documentation, I actually 
> > > tried to make sure the code and documentation matched (what I think 
> > > "should" happen). :-)
> > > 
> > > By encouraging us to break what documentation we have, we open the door to
> > > getting back into an untrusted, confusing mess. For instance, new 
> > > developers have to just "know" that it's "ok" to use shared locks for 
> > > reads, but not anything else.
> > 
> > documented where?
> 
> sys/kern/vnode_if.src. Yes, it does not explicitly state that locked state 
> == exclusive, however until Chuck's change nothing used shared locks. So 
> before that it'd have seemed like stating the obvious.

it says that VOP_GETATTR doesn't require the vnode locked.
(i think it's fine.)
so updating atime in VOP_READ should be protected by some
filesystem internal locks anyway if you want to see a consistent view of
timestamps without biglock. (a spinlock is enough)
afaik, there's no problem which was introduced by LK_SHARED for VOP_READ.

> > > Oh, as an aside, I think in your patch you unlock at one point and then 
> > > re-lock with LK_SHARED. I think all you need to do is call VOP_LOCK() with 
> > > LK_DOWNGRADE or something like it - you can downgrade the exclusive lock 
> > > into a shared one. Also, since you have the exclusive lock, you can just 
> > > use VOP_LOCK() as no one will have initiated reclaiming in the mean time.
> > 
> > i think it's bad idea to allow to use arbitrary fancy lockmgr flags
> > for VOP_LOCK.
> 
> Why? At one point I'd have agreed with you. At this point, however, I
> think we "know" that we have lockmgr locks. We might as well use lockmgr
> to our benefit. Hmm... need to look at unionfs to see if it's happy with
> shared locks and downgrade.

because VOP_LOCK/UNLOCK != lockmgr.
or, do you want to use VOP_LOCK(vp, LK_RELEASE)? :-)

YAMAMOTO Takashi