Subject: Re: LK_SHARED for VFS_VGET/FHTOVP
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 05/26/2004 16:37:09
--HWvPVVuAAfuRc6SZ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, May 27, 2004 at 04:43:15AM +0900, YAMAMOTO Takashi wrote:
> hi,
>=20
> > Specifically with using exclusive locks, I think that the vnode system =
was=20
> > fine-grained. Perhaps not the best locking (we can get better paralleli=
sm,=20
> > perhaps), but correct. By changing things so we need big-lock, we take =
a=20
> > step back.
>=20
> what's "the vnode system"?
> the vnode interface itsself, or eachfilesystem implementation,
> or something else?

Both the interface and our file system implementations.

> > Over the years the vnode interface has gotten a reputation for being
> > complicated and troublesome. When I was at NASA, I worked on cleaning l=
ots
> > 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=20
> > documenting what should happen and auditing all the code to match. To g=
o=20
> > back to a comment you made about me believing documentation, I actually=
=20
> > tried to make sure the code and documentation matched (what I think=20
> > "should" happen). :-)
> >=20
> > By encouraging us to break what documentation we have, we open the door=
 to
> > getting back into an untrusted, confusing mess. For instance, new=20
> > developers have to just "know" that it's "ok" to use shared locks for=
=20
> > reads, but not anything else.
>=20
> documented where?

sys/kern/vnode_if.src. Yes, it does not explicitly state that locked state=
=20
=3D=3D exclusive, however until Chuck's change nothing used shared locks. S=
o=20
before that it'd have seemed like stating the obvious.

> > 2) We don't know how bad things are. Sure, ffs_read() (and lfs_read()) =
=20
> > only call ufs_update(), which is "ok" as long as we have big-lock. But =
we
> > don't _know_ what the other file systems do, and what problems they hav=
e; =20
> > we haven't audited the other code. This uncertainty is one of my big
> > concerns about things.
>=20
> have you audited all of our filesystems at some point before Chuck changed
> the code to use LK_SHARED?

No. And AFAICT Chuck didn't perform the audit either. That's one of my=20
concerns. We have no idea what assumptions have been broken.

> > Oh, as an aside, I think in your patch you unlock at one point and then=
=20
> > re-lock with LK_SHARED. I think all you need to do is call VOP_LOCK() w=
ith=20
> > LK_DOWNGRADE or something like it - you can downgrade the exclusive loc=
k=20
> > into a shared one. Also, since you have the exclusive lock, you can jus=
t=20
> > use VOP_LOCK() as no one will have initiated reclaiming in the mean tim=
e.
>=20
> 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.

Take care,

Bill

--HWvPVVuAAfuRc6SZ
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)

iD8DBQFAtSolWz+3JHUci9cRAmIZAJwPbzlSHlpm/SgstXPziqI02bKjJACfQ5OB
fshMySfMmgfholfmjI3vlUo=
=CojY
-----END PGP SIGNATURE-----

--HWvPVVuAAfuRc6SZ--