? ufs/ffs/snapshot.diff Index: ufs/ffs/ffs_snapshot.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_snapshot.c,v retrieving revision 1.141 diff -u -p -r1.141 ffs_snapshot.c --- ufs/ffs/ffs_snapshot.c 19 Mar 2008 06:19:01 -0000 1.141 +++ ufs/ffs/ffs_snapshot.c 30 Mar 2008 03:21:15 -0000 @@ -127,12 +127,17 @@ ffs_copyonwrite(devvp, bp) TAILQ_HEAD(snaphead, inode); struct snapdata { + LIST_ENTRY(snapdata) sn_link; struct snaphead sn_head; daddr_t sn_listsize; daddr_t *sn_blklist; struct lock sn_lock; }; +LIST_HEAD(, snapdata) snapfree; +static struct mtx snapfree_lock; +MTX_SYSINIT(ffs_snapfree, &snapfree_lock, "snapdata free list", MTX_DEF); + static int cgaccount(int, struct vnode *, struct buf *, int); static int expunge_ufs1(struct vnode *, struct inode *, struct fs *, int (*)(struct vnode *, ufs1_daddr_t *, ufs1_daddr_t *, struct fs *, @@ -162,7 +167,8 @@ static int mapacct_ufs2(struct vnode *, struct fs *, ufs_lbn_t, int); static int readblock(struct vnode *vp, struct buf *, ufs2_daddr_t); static void process_deferred_inactive(struct mount *); -static void try_free_snapdata(struct vnode *devvp, struct thread *td); +static void try_free_snapdata(struct vnode *devvp); +static struct snapdata *ffs_snapdata_acquire(struct vnode *devvp); static int ffs_bp_snapblk(struct vnode *, struct buf *); /* @@ -603,34 +609,12 @@ loop: } MNT_IUNLOCK(mp); /* - * If there already exist snapshots on this filesystem, grab a - * reference to their shared lock. If this is the first snapshot - * on this filesystem, we need to allocate a lock for the snapshots - * to share. In either case, acquire the snapshot lock and give - * up our original private lock. + * Acquire a lock on the snapdata structure, creating it if necessary. */ - VI_LOCK(devvp); - sn = devvp->v_rdev->si_snapdata; - if (sn != NULL) { - xp = TAILQ_FIRST(&sn->sn_head); - VI_UNLOCK(devvp); - VI_LOCK(vp); - vp->v_vnlock = &sn->sn_lock; - } else { - VI_UNLOCK(devvp); - sn = malloc(sizeof *sn, M_UFSMNT, M_WAITOK | M_ZERO); - TAILQ_INIT(&sn->sn_head); - lockinit(&sn->sn_lock, PVFS, "snaplk", VLKTIMEOUT, - LK_CANRECURSE | LK_NOSHARE); - VI_LOCK(vp); - vp->v_vnlock = &sn->sn_lock; - mp_fixme("si_snapdata setting is racey."); - devvp->v_rdev->si_snapdata = sn; - xp = NULL; - } - lockmgr(vp->v_vnlock, LK_INTERLOCK | LK_EXCLUSIVE | LK_RETRY, - VI_MTX(vp)); + sn = ffs_snapdata_acquire(devvp); + vp->v_vnlock = &sn->sn_lock; lockmgr(&vp->v_lock, LK_RELEASE, NULL); + xp = TAILQ_FIRST(&sn->sn_head); /* * If this is the first snapshot on this filesystem, then we need * to allocate the space for the list of preallocated snapshot blocks. @@ -1566,7 +1550,6 @@ ffs_snapremove(vp) struct vnode *devvp; struct buf *ibp; struct fs *fs; - struct thread *td = curthread; ufs2_daddr_t numblks, blkno, dblk; int error, loc, last; struct snapdata *sn; @@ -1588,14 +1571,12 @@ ffs_snapremove(vp) ip->i_nextsnap.tqe_prev = 0; VI_UNLOCK(devvp); lockmgr(&vp->v_lock, LK_EXCLUSIVE, NULL); - VI_LOCK(vp); KASSERT(vp->v_vnlock == &sn->sn_lock, ("ffs_snapremove: lost lock mutation")); vp->v_vnlock = &vp->v_lock; - VI_UNLOCK(vp); VI_LOCK(devvp); lockmgr(&sn->sn_lock, LK_RELEASE, NULL); - try_free_snapdata(devvp, td); + try_free_snapdata(devvp); } else VI_UNLOCK(devvp); /* @@ -1904,7 +1885,7 @@ ffs_snapshot_mount(mp) */ vp = NULL; lastvp = NULL; - sn = devvp->v_rdev->si_snapdata; + sn = NULL; for (snaploc = 0; snaploc < FSMAXSNAP; snaploc++) { if (fs->fs_snapinum[snaploc] == 0) break; @@ -1937,30 +1918,11 @@ ffs_snapshot_mount(mp) continue; } /* - * If there already exist snapshots on this filesystem, grab a - * reference to their shared lock. If this is the first snapshot - * on this filesystem, we need to allocate a lock for the - * snapshots to share. In either case, acquire the snapshot - * lock and give up our original private lock. + * Acquire a lock on the snapdata structure, creating it if + * necessary. */ - VI_LOCK(devvp); - if (sn != NULL) { - - VI_UNLOCK(devvp); - VI_LOCK(vp); - vp->v_vnlock = &sn->sn_lock; - } else { - VI_UNLOCK(devvp); - sn = malloc(sizeof *sn, M_UFSMNT, M_WAITOK | M_ZERO); - TAILQ_INIT(&sn->sn_head); - lockinit(&sn->sn_lock, PVFS, "snaplk", VLKTIMEOUT, - LK_CANRECURSE | LK_NOSHARE); - VI_LOCK(vp); - vp->v_vnlock = &sn->sn_lock; - devvp->v_rdev->si_snapdata = sn; - } - lockmgr(vp->v_vnlock, LK_INTERLOCK | LK_EXCLUSIVE | LK_RETRY, - VI_MTX(vp)); + sn = ffs_snapdata_acquire(devvp); + vp->v_vnlock = &sn->sn_lock; lockmgr(&vp->v_lock, LK_RELEASE, NULL); /* * Link it onto the active snapshot list. @@ -1980,7 +1942,7 @@ ffs_snapshot_mount(mp) /* * No usable snapshots found. */ - if (vp == NULL) + if (sn == NULL || vp == NULL) return; /* * Allocate the space for the block hints list. We always want to @@ -2035,7 +1997,6 @@ ffs_snapshot_unmount(mp) struct snapdata *sn; struct inode *xp; struct vnode *vp; - struct thread *td = curthread; VI_LOCK(devvp); sn = devvp->v_rdev->si_snapdata; @@ -2045,13 +2006,10 @@ ffs_snapshot_unmount(mp) xp->i_nextsnap.tqe_prev = 0; lockmgr(&sn->sn_lock, LK_INTERLOCK | LK_EXCLUSIVE, VI_MTX(devvp)); - VI_LOCK(vp); - lockmgr(&vp->v_lock, LK_INTERLOCK | LK_EXCLUSIVE, VI_MTX(vp)); - VI_LOCK(vp); + lockmgr(&vp->v_lock, LK_EXCLUSIVE, NULL); KASSERT(vp->v_vnlock == &sn->sn_lock, ("ffs_snapshot_unmount: lost lock mutation")); vp->v_vnlock = &vp->v_lock; - VI_UNLOCK(vp); lockmgr(&vp->v_lock, LK_RELEASE, NULL); lockmgr(&sn->sn_lock, LK_RELEASE, NULL); if (xp->i_effnlink > 0) @@ -2059,7 +2017,7 @@ ffs_snapshot_unmount(mp) VI_LOCK(devvp); sn = devvp->v_rdev->si_snapdata; } - try_free_snapdata(devvp, td); + try_free_snapdata(devvp); ASSERT_VOP_LOCKED(devvp, "ffs_snapshot_unmount"); } @@ -2486,14 +2444,52 @@ process_deferred_inactive(struct mount * vn_finished_secondary_write(mp); } +static struct snapdata * +ffs_snapdata_alloc(void) +{ + struct snapdata *sn; + + /* + * Fetch a snapdata from the free list if there is one available. + */ + mtx_lock(&snapfree_lock); + sn = LIST_FIRST(&snapfree); + if (sn) + LIST_REMOVE(sn, sn_link); + mtx_unlock(&snapfree_lock); + if (sn) + return (sn); + /* + * If there were no free snapdatas allocate one. + */ + sn = malloc(sizeof *sn, M_UFSMNT, M_WAITOK | M_ZERO); + TAILQ_INIT(&sn->sn_head); + lockinit(&sn->sn_lock, PVFS, "snaplk", VLKTIMEOUT, + LK_CANRECURSE | LK_NOSHARE); + return (sn); +} + +/* + * The snapdata is never freed because we can not be certain that + * there are no threads sleeping on the snap lock. Persisting + * them permanently avoids costly synchronization in ffs_lock(). + */ +static void +ffs_snapdata_free(struct snapdata *sn) +{ + mtx_lock(&snapfree_lock); + LIST_INSERT_HEAD(&snapfree, sn, sn_link); + mtx_unlock(&snapfree_lock); +} + /* Try to free snapdata associated with devvp */ static void -try_free_snapdata(struct vnode *devvp, - struct thread *td) +try_free_snapdata(struct vnode *devvp) { struct snapdata *sn; ufs2_daddr_t *snapblklist; + ASSERT_VI_LOCKED(devvp, "try_free_snapdata"); sn = devvp->v_rdev->si_snapdata; if (sn == NULL || TAILQ_FIRST(&sn->sn_head) != NULL || @@ -2504,14 +2500,52 @@ try_free_snapdata(struct vnode *devvp, devvp->v_rdev->si_snapdata = NULL; devvp->v_vflag &= ~VV_COPYONWRITE; + lockmgr(&sn->sn_lock, LK_DRAIN|LK_INTERLOCK, VI_MTX(devvp)); snapblklist = sn->sn_blklist; sn->sn_blklist = NULL; sn->sn_listsize = 0; - lockmgr(&sn->sn_lock, LK_DRAIN|LK_INTERLOCK, VI_MTX(devvp)); lockmgr(&sn->sn_lock, LK_RELEASE, NULL); - lockdestroy(&sn->sn_lock); - free(sn, M_UFSMNT); if (snapblklist != NULL) FREE(snapblklist, M_UFSMNT); + ffs_snapdata_free(sn); +} + +static struct snapdata * +ffs_snapdata_acquire(struct vnode *devvp) +{ + struct snapdata *nsn; + struct snapdata *sn; + + /* + * Allocate a free snapdata. This is done before acquiring the + * devvp lock to avoid allocation while the devvp interlock is + * held. + */ + nsn = ffs_snapdata_alloc(); + /* + * If there snapshots already exist on this filesystem grab a + * reference to the shared lock. Otherwise this is the first + * snapshot on this filesystem and we need to use our + * pre-allocated snapdata. + */ + VI_LOCK(devvp); + if (devvp->v_rdev->si_snapdata == NULL) { + devvp->v_rdev->si_snapdata = nsn; + nsn = NULL; + } + sn = devvp->v_rdev->si_snapdata; + /* + * Acquire the snapshot lock. + */ + lockmgr(&sn->sn_lock, + LK_INTERLOCK | LK_EXCLUSIVE | LK_RETRY, VI_MTX(devvp)); + /* + * Free any unused snapdata. + */ + if (nsn) + ffs_snapdata_free(nsn); + + return (sn); } + #endif Index: ufs/ffs/ffs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_vnops.c,v retrieving revision 1.181 diff -u -p -r1.181 ffs_vnops.c --- ufs/ffs/ffs_vnops.c 22 Mar 2008 09:15:16 -0000 1.181 +++ ufs/ffs/ffs_vnops.c 30 Mar 2008 03:21:15 -0000 @@ -361,16 +361,6 @@ ffs_lock(ap) vp = ap->a_vp; flags = ap->a_flags; for (;;) { - /* - * vnode interlock must be held to ensure that - * the possibly external lock isn't freed, - * e.g. when mutating from snapshot file vnode - * to regular file vnode. - */ - if ((flags & LK_INTERLOCK) == 0) { - VI_LOCK(vp); - flags |= LK_INTERLOCK; - } lkp = vp->v_vnlock; result = _lockmgr_args(lkp, flags, VI_MTX(vp), LK_WMESG_DEFAULT, LK_PRIO_DEFAULT, LK_TIMO_DEFAULT, @@ -385,9 +375,12 @@ ffs_lock(ap) * right lock. Release it, and try to get the * new lock. */ - (void) _lockmgr_args(lkp, LK_RELEASE, VI_MTX(vp), + (void) _lockmgr_args(lkp, LK_RELEASE, NULL, LK_WMESG_DEFAULT, LK_PRIO_DEFAULT, LK_TIMO_DEFAULT, ap->a_file, ap->a_line); + if ((flags & (LK_INTERLOCK | LK_NOWAIT)) == + (LK_INTERLOCK | LK_NOWAIT)) + return (EBUSY); if ((flags & LK_TYPE_MASK) == LK_UPGRADE) flags = (flags & ~LK_TYPE_MASK) | LK_EXCLUSIVE; flags &= ~LK_INTERLOCK;