tech-kern archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Move the vnode lock into file systems
On Mon, Jun 28, 2010 at 09:06:57AM +0000, Andrew Doran wrote:
> On Sat, Jun 26, 2010 at 10:39:27AM +0200, Juergen Hannken-Illjes wrote:
> > The vnode lock operations currently work on a rw lock located inside the
> > vnode. I propose to move this lock into the file system node.
> >
> > This place is more logical as we lock a file system node and not a vnode.
> > This becomes clear if we think of a file system where one file system node
> > is attached to more than one vnode. Ptyfs allowing multiple mounts is such
> > a candidate.
> >
> > A diff implemeting this for ufs, lfs and ext2fs is attached.
> >
> > Comments or objections anyone?
>
> I like this idea. Any reason this cannot live in genfs_node?
>
> Thinking out loud here, but doing this would help us eliminate deadfs and
> make forced unmount safe again. With another field in genfs_node (say
> 'bool gone') to indicate that VOP_REVOKE has taken place, we could return
> failure at the lock point for revoked vnodes. We'd still need to allow
> VOP_INACTIVE and VOP_RECLAIM to take place on the vnode though (possibly
> also VOP_CLOSE??) - haven't thought it through fully.
Note, not suggesting that we do this, it would obviously
require a lot of other work - just thinking out loud.
> Have you given thought to removing the lock function from vget()? That
> is, change it only to acquire references and deal with XLOCK?
>
> Thanks!
>
> > --
> > Juergen Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU
> > Braunschweig (Germany)
> >
> > Index: sys/ufs/ufs/inode.h
> > ===================================================================
> > RCS file: /cvsroot/src/sys/ufs/ufs/inode.h,v
> > retrieving revision 1.56
> > diff -p -u -4 -r1.56 inode.h
> > --- sys/ufs/ufs/inode.h 22 Feb 2009 20:28:07 -0000 1.56
> > +++ sys/ufs/ufs/inode.h 25 Jun 2010 20:43:24 -0000
> > @@ -91,9 +91,9 @@ struct inode {
> > #define i_fs inode_u.fs
> > #define i_lfs inode_u.lfs
> > #define i_e2fs inode_u.e2fs
> >
> > - void *i_unused1; /* Unused. */
> > + krwlock_t i_lock; /* Inode lock. */
> > struct dquot *i_dquot[MAXQUOTAS]; /* Dquot structures. */
> > u_quad_t i_modrev; /* Revision level for NFS lease. */
> > struct lockf *i_lockf;/* Head of byte-level lock list. */
> >
> > Index: sys/ufs/ufs/ufs_extern.h
> > ===================================================================
> > RCS file: /cvsroot/src/sys/ufs/ufs/ufs_extern.h,v
> > retrieving revision 1.62
> > diff -p -u -4 -r1.62 ufs_extern.h
> > --- sys/ufs/ufs/ufs_extern.h 13 Sep 2009 05:17:37 -0000 1.62
> > +++ sys/ufs/ufs/ufs_extern.h 25 Jun 2010 20:43:24 -0000
> > @@ -67,11 +67,11 @@ int ufs_create(void *);
> > int ufs_getattr(void *);
> > int ufs_inactive(void *);
> > #define ufs_fcntl genfs_fcntl
> > #define ufs_ioctl genfs_enoioctl
> > -#define ufs_islocked genfs_islocked
> > +int ufs_islocked(void *);
> > int ufs_link(void *);
> > -#define ufs_lock genfs_lock
> > +int ufs_lock(void *);
> > int ufs_lookup(void *);
> > int ufs_mkdir(void *);
> > int ufs_mknod(void *);
> > #define ufs_mmap genfs_mmap
> > @@ -88,9 +88,9 @@ int ufs_rmdir(void *);
> > #define ufs_poll genfs_poll
> > int ufs_setattr(void *);
> > int ufs_strategy(void *);
> > int ufs_symlink(void *);
> > -#define ufs_unlock genfs_unlock
> > +int ufs_unlock(void *);
> > int ufs_whiteout(void *);
> > int ufsspec_close(void *);
> > int ufsspec_read(void *);
> > int ufsspec_write(void *);
> > Index: sys/ufs/ufs/ufs_ihash.c
> > ===================================================================
> > RCS file: /cvsroot/src/sys/ufs/ufs/ufs_ihash.c,v
> > retrieving revision 1.28
> > diff -p -u -4 -r1.28 ufs_ihash.c
> > --- sys/ufs/ufs/ufs_ihash.c 5 Nov 2009 08:18:02 -0000 1.28
> > +++ sys/ufs/ufs/ufs_ihash.c 25 Jun 2010 20:43:24 -0000
> > @@ -170,9 +170,10 @@ ufs_ihashins(struct inode *ip)
> >
> > KASSERT(mutex_owned(&ufs_hashlock));
> >
> > /* lock the inode, then put it on the appropriate hash list */
> > - vlockmgr(&ip->i_vnode->v_lock, LK_EXCLUSIVE);
> > + rw_init(&ip->i_lock);
> > + rw_enter(&ip->i_lock, RW_WRITER);
> >
> > mutex_enter(&ufs_ihash_lock);
> > ipp = &ihashtbl[INOHASH(ip->i_dev, ip->i_number)];
> > LIST_INSERT_HEAD(ipp, ip, i_hash);
> > @@ -187,5 +188,6 @@ ufs_ihashrem(struct inode *ip)
> > {
> > mutex_enter(&ufs_ihash_lock);
> > LIST_REMOVE(ip, i_hash);
> > mutex_exit(&ufs_ihash_lock);
> > + rw_destroy(&ip->i_lock);
> > }
> > Index: sys/ufs/ufs/ufs_vnops.c
> > ===================================================================
> > RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
> > retrieving revision 1.183
> > diff -p -u -4 -r1.183 ufs_vnops.c
> > --- sys/ufs/ufs/ufs_vnops.c 24 Jun 2010 13:03:20 -0000 1.183
> > +++ sys/ufs/ufs/ufs_vnops.c 25 Jun 2010 20:43:25 -0000
> > @@ -2346,4 +2346,66 @@ ufs_gop_markupdate(struct vnode *vp, int
> >
> > ip->i_flag |= mask;
> > }
> > }
> > +
> > +/*
> > + * Lock the node.
> > + */
> > +int
> > +ufs_lock(void *v)
> > +{
> > + struct vop_lock_args /* {
> > + struct vnode *a_vp;
> > + int a_flags;
> > + } */ *ap = v;
> > + struct inode *ip = VTOI(ap->a_vp);
> > + int flags = ap->a_flags;
> > + krw_t op;
> > +
> > + KASSERT((flags & ~(LK_EXCLUSIVE | LK_SHARED | LK_NOWAIT)) == 0);
> > +
> > + op = ((flags & LK_EXCLUSIVE) != 0 ? RW_WRITER : RW_READER);
> > +
> > + if ((flags & LK_NOWAIT) != 0)
> > + return (rw_tryenter(&ip->i_lock, op) ? 0 : EBUSY);
> > +
> > + rw_enter(&ip->i_lock, op);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Unlock the node.
> > + */
> > +int
> > +ufs_unlock(void *v)
> > +{
> > + struct vop_unlock_args /* {
> > + struct vnode *a_vp;
> > + } */ *ap = v;
> > + struct inode *ip = VTOI(ap->a_vp);
> > +
> > + rw_exit(&ip->i_lock);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Return whether or not the node is locked.
> > + */
> > +int
> > +ufs_islocked(void *v)
> > +{
> > + struct vop_islocked_args /* {
> > + struct vnode *a_vp;
> > + } */ *ap = v;
> > + struct inode *ip = VTOI(ap->a_vp);
> > +
> > + if (rw_write_held(&ip->i_lock))
> > + return LK_EXCLUSIVE;
> > +
> > + if (rw_read_held(&ip->i_lock))
> > + return LK_SHARED;
> > +
> > + return 0;
> > +}
> > Index: sys/ufs/lfs/lfs_syscalls.c
> > ===================================================================
> > RCS file: /cvsroot/src/sys/ufs/lfs/lfs_syscalls.c,v
> > retrieving revision 1.137
> > diff -p -u -4 -r1.137 lfs_syscalls.c
> > --- sys/ufs/lfs/lfs_syscalls.c 24 Jun 2010 13:03:19 -0000 1.137
> > +++ sys/ufs/lfs/lfs_syscalls.c 25 Jun 2010 20:43:24 -0000
> > @@ -1136,12 +1136,12 @@ lfs_fastvget(struct mount *mp, ino_t ino
> > error = copyin(dinp, ip->i_din.ffs1_din, sizeof (struct
> > ufs1_dinode));
> > if (error) {
> > DLOG((DLOG_CLEAN, "lfs_fastvget: dinode copyin failed"
> > " for ino %d\n", ino));
> > + /* Unlock and discard unneeded inode. */
> > + rw_exit(&ip->i_lock);
> > ufs_ihashrem(ip);
> >
> > - /* Unlock and discard unneeded inode. */
> > - vlockmgr(&vp->v_lock, LK_RELEASE);
> > lfs_vunref(vp);
> > *vpp = NULL;
> > return (error);
> > }
> > @@ -1158,13 +1158,13 @@ lfs_fastvget(struct mount *mp, ino_t ino
> > /*
> > * The inode does not contain anything useful, so it
> > * would be misleading to leave it on its hash chain.
> > * Iput() will return it to the free list.
> > + * Unlock and discard unneeded inode.
> > */
> > + rw_exit(&ip->i_lock);
> > ufs_ihashrem(ip);
> >
> > - /* Unlock and discard unneeded inode. */
> > - vlockmgr(&vp->v_lock, LK_RELEASE);
> > lfs_vunref(vp);
> > brelse(bp, 0);
> > *vpp = NULL;
> > return (error);
Home |
Main Index |
Thread Index |
Old Index