Subject: Re: Incorrect locking of uobj in nfs_getpages()?
To: None <jdolecek@NetBSD.org>
From: Bill Sommerfeld <sommerfeld@netbsd.org>
List: tech-kern
Date: 12/20/2001 10:02:05
> However, nfs_getpages() does explicit simple_lock(&uobj->vmobjlock)
> and simple_unlock(&uobj->vmobjlock) when manipulating the pg->flags.
> This seems to be WRONG. 

There's something more subtle going on here.

First, I can't find a simple statement of the VOP_GETPAGES locking
protocol in either of the places which I'd expect to find it.
(kern/vnode_if.src; miscfs/genfs/genfs_vnops.c)

BTW, the best way to document locking assumptions in code is with
higher-level assertions:

	LOCK_ASSERT(simple_lock_held(&uobj->vmobjlock));

	LOCK_ASSERT(!simple_lock_held(&uobj->vmobjlock));

It appears from current usage that VOP_GETPAGES() implementations are
supposed to be entered with the vnode's uobj locked, and it's supposed
to return with the uobj unlocked *unless* the PGO_LOCKED flag is
passed.

nfs_getpages calls into genfs_getpages to do the dirty work, so
genfs_getpages may either leave it locked or unlock it.  This is an
excellent example of why flag usage of this form is so error-prone --
it would have perhaps have been more sensible to have VOP_GETPAGES()
and VOP_GETLOCKEDPAGES() and pager->pgo_get() and
pager->pgo_getlocked() ops, each with a straightforward locking
interface.

					- Bill