hi,

in the other file systems, I've done something similar to this
(protecting the bmap with a new lock instead of the vnode lock),
there it's called the getpages lock or "glock".  using the same
lock to protect the LFS bmap in general might make it easier to
convert LFS to use the page cache instead of the buffer cache eventually,
you might want to investigate that a bit.

ignoring the above for the rest of this mail:

you should try to enhance the lockmgr() locks to do what you want
rather than implementing your own custom locks.  hiding the lock
implementation behind the LFS_*_META() macros like you've got them
is good, however.

you've got this bit:

 @@ -1283,6 +1295,8 @@ lfs_fakebuf(struct lfs *fs, struct vnode
        bp = lfs_newbuf(VTOI(vp)->i_lfs, vp, lbn, size);
        error = copyin(uaddr, bp->b_data, size);
        if (error) {
+               panic("lfs_newbuf: copyin failed: uaddr=%p, error=%d\n",
+                   (void *)uaddr, error);
                lfs_freebuf(bp);
                return NULL;
        }


do we really need to panic here?  there ought to be some way to recover
from a copyin() failure.

I didn't notice anything else from a brief look over your patch,
but I'm hardly an LFS expert so I can't comment on the LFS-specific
aspects of this.  you seem to be the LFS maintainer at this point,
so we'll just trust you.  :-)

-Chuck


On Sun, Dec 15, 2002 at 10:46:25PM +0900, YAMAMOTO Takashi wrote:
> hi.
> 
> currently, since lfs_markv have to acquire vnode locks,
> lfs_reserve have to release vnode locks before wait for lfs_avail.
> however, releasing locks in lfs_reserve isn't safe because
> we can't keep the vnodes/inodees from modifying without holding the locks.
> 
> i'd like to separate metadata (ie. block pointers) lock from vnode lock
> so that lfs_reserve no longer need to release vnode locks as attached patch.
> is it ok?
> 
> thanks.
> 
> YAMAMOTO Takashi