From 59eb00662b48f700a961ebe4a6fb572d8e4225f0 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 19 Aug 2017 12:10:09 -0700 Subject: [PATCH] hammer2 - Fix snapshots and multi-label mounts * Allow the same /dev/blah@DIFFERENTLABEL to be specified in a mount command, so multiple labels from the same device can be mounted. * Devfs can throw different vnodes for the same device. When matching up hammer2_dev, check devvp->v_rdev for a match as well. * Fix a number of bugs in the snapshot code that left a hammer2_inode structure hanging and caused a panic. * Fix races in admin thread flag messaging that could lead to 60-second delays during umount. --- sys/vfs/hammer2/hammer2_chain.c | 18 +++++++++++++----- sys/vfs/hammer2/hammer2_ioctl.c | 31 +++++++++++++++++++++---------- sys/vfs/hammer2/hammer2_synchro.c | 13 ++++--------- sys/vfs/hammer2/hammer2_vfsops.c | 26 ++++++++++++++++++++++---- 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/sys/vfs/hammer2/hammer2_chain.c b/sys/vfs/hammer2/hammer2_chain.c index 2c5104d124..73ddb98e86 100644 --- a/sys/vfs/hammer2/hammer2_chain.c +++ b/sys/vfs/hammer2/hammer2_chain.c @@ -5175,11 +5175,11 @@ hammer2_chain_bulkdrop(hammer2_chain_t *copy) } /* - * Create a snapshot of the specified {parent, ochain} with the specified - * label. The originating hammer2_inode must be exclusively locked for - * safety. - * - * The ioctl code has already synced the filesystem. + * Create a snapshot of the specified (chain) with the specified label. + * The originating hammer2_inode must be exclusively locked for + * safety. The device's bulklk should be held by the caller. The caller + * is responsible for synchronizing the filesystem to storage before + * taking the snapshot. */ int hammer2_chain_snapshot(hammer2_chain_t *chain, hammer2_ioc_pfs_t *pmp, @@ -5227,11 +5227,13 @@ hammer2_chain_snapshot(hammer2_chain_t *chain, hammer2_ioc_pfs_t *pmp, VATTR_NULL(&vat); vat.va_type = VDIR; vat.va_mode = 0755; + hammer2_chain_unlock(chain); nip = hammer2_inode_create(hmp->spmp->iroot, hmp->spmp->iroot, &vat, proc0.p_ucred, pmp->name, name_len, 0, 1, 0, 0, HAMMER2_INSERT_PFSROOT, &error); + hammer2_chain_lock(chain, HAMMER2_RESOLVE_ALWAYS); if (nip) { hammer2_inode_modify(nip); @@ -5262,10 +5264,16 @@ hammer2_chain_snapshot(hammer2_chain_t *chain, hammer2_ioc_pfs_t *pmp, /* XXX doesn't work with real cluster */ wipdata->meta = nip->meta; wipdata->u.blockset = ripdata->u.blockset; + hammer2_flush(nchain, 1); + KKASSERT(wipdata == &nchain->data->ipdata); + hammer2_pfsalloc(nchain, wipdata, nchain->bref.modify_tid, 0); + hammer2_chain_unlock(nchain); hammer2_chain_drop(nchain); + hammer2_inode_chain_sync(nip); hammer2_inode_unlock(nip); + hammer2_inode_run_sideq(hmp->spmp); } return (error); } diff --git a/sys/vfs/hammer2/hammer2_ioctl.c b/sys/vfs/hammer2/hammer2_ioctl.c index 4f4b8c9a18..010761582a 100644 --- a/sys/vfs/hammer2/hammer2_ioctl.c +++ b/sys/vfs/hammer2/hammer2_ioctl.c @@ -180,21 +180,28 @@ static int hammer2_ioctl_recluster(hammer2_inode_t *ip, void *data) { hammer2_ioc_recluster_t *recl = data; + struct vnode *vproot; struct file *fp; hammer2_cluster_t *cluster; int error; fp = holdfp(curproc->p_fd, recl->fd, -1); if (fp) { - kprintf("reconnect to cluster: XXX "); - cluster = &ip->pmp->iroot->cluster; - if (cluster->nchains != 1 || cluster->focus == NULL) { - kprintf("not a local device mount\n"); - error = EINVAL; - } else { - hammer2_cluster_reconnect(cluster->focus->hmp, fp); - kprintf("ok\n"); - error = 0; + error = VFS_ROOT(ip->pmp->mp, &vproot); + if (error == 0) { + cluster = &ip->pmp->iroot->cluster; + kprintf("reconnect to cluster: nc=%d focus=%p\n", + cluster->nchains, cluster->focus); + if (cluster->nchains != 1 || cluster->focus == NULL) { + kprintf("not a local device mount\n"); + error = EINVAL; + } else { + hammer2_cluster_reconnect(cluster->focus->hmp, + fp); + kprintf("ok\n"); + error = 0; + } + vput(vproot); } } else { error = EINVAL; @@ -414,7 +421,7 @@ hammer2_ioctl_pfs_get(hammer2_inode_t *ip, void *data) if (save_key == (hammer2_key_t)-1) { hammer2_inode_lock(ip->pmp->iroot, 0); parent = NULL; - chain = hammer2_inode_chain(hmp->spmp->iroot, 0, + chain = hammer2_inode_chain(ip->pmp->iroot, 0, HAMMER2_RESOLVE_ALWAYS | HAMMER2_RESOLVE_SHARED); } else { @@ -789,6 +796,8 @@ hammer2_ioctl_pfs_snapshot(hammer2_inode_t *ip, void *data) if (hmp == NULL) return (EINVAL); + lockmgr(&hmp->bulklk, LK_EXCLUSIVE); + hammer2_vfs_sync(pmp->mp, MNT_WAIT); hammer2_trans_init(pmp, HAMMER2_TRANS_ISFLUSH); @@ -806,6 +815,8 @@ hammer2_ioctl_pfs_snapshot(hammer2_inode_t *ip, void *data) hammer2_inode_unlock(ip); hammer2_trans_done(pmp); + lockmgr(&hmp->bulklk, LK_RELEASE); + return (error); } diff --git a/sys/vfs/hammer2/hammer2_synchro.c b/sys/vfs/hammer2/hammer2_synchro.c index c7f9079c65..384be9f4e3 100644 --- a/sys/vfs/hammer2/hammer2_synchro.c +++ b/sys/vfs/hammer2/hammer2_synchro.c @@ -131,8 +131,7 @@ hammer2_primary_sync_thread(void *arg) continue; if (flags & HAMMER2_THREAD_WAITING) wakeup(&thr->flags); - flags = nflags; - /* fall through */ + continue; } if (flags & HAMMER2_THREAD_UNFREEZE) { @@ -143,8 +142,7 @@ hammer2_primary_sync_thread(void *arg) continue; if (flags & HAMMER2_THREAD_WAITING) wakeup(&thr->flags); - flags = nflags; - /* fall through */ + continue; } /* @@ -152,12 +150,10 @@ hammer2_primary_sync_thread(void *arg) */ if (flags & HAMMER2_THREAD_FROZEN) { nflags = flags | HAMMER2_THREAD_WAITING; + tsleep_interlock(&thr->flags, 0); - if (atomic_cmpset_int(&thr->flags, flags, nflags)) { + if (atomic_cmpset_int(&thr->flags, flags, nflags)) tsleep(&thr->flags, PINTERLOCKED, "frozen", 0); - atomic_clear_int(&thr->flags, - HAMMER2_THREAD_WAITING); - } continue; } @@ -251,7 +247,6 @@ hammer2_primary_sync_thread(void *arg) tsleep_interlock(&thr->flags, 0); if (atomic_cmpset_int(&thr->flags, flags, nflags)) { tsleep(&thr->flags, 0, "h2idle", hz * 5); - atomic_clear_int(&thr->flags, HAMMER2_THREAD_WAITING); } } thr->td = NULL; diff --git a/sys/vfs/hammer2/hammer2_vfsops.c b/sys/vfs/hammer2/hammer2_vfsops.c index 477389a873..c0a834c4dd 100644 --- a/sys/vfs/hammer2/hammer2_vfsops.c +++ b/sys/vfs/hammer2/hammer2_vfsops.c @@ -983,9 +983,12 @@ hammer2_vfs_mount(struct mount *mp, char *path, caddr_t data, error = 0; } + /* + * Make sure its a block device. Do not check to see if it is + * already mounted until we determine that its a fresh H2 device. + */ if (error == 0 && devvp) { - if (vn_isdisk(devvp, &error)) - error = vfs_mountedon(devvp); + vn_isdisk(devvp, &error); } /* @@ -996,12 +999,25 @@ hammer2_vfs_mount(struct mount *mp, char *path, caddr_t data, lockmgr(&hammer2_mntlk, LK_EXCLUSIVE); if (devvp) { /* - * Match the device + * Match the device. Due to the way devfs works, + * we may not be able to directly match the vnode pointer, + * so also check to see if the underlying device matches. */ TAILQ_FOREACH(hmp, &hammer2_mntlist, mntentry) { if (hmp->devvp == devvp) break; + if (devvp->v_rdev && + hmp->devvp->v_rdev == devvp->v_rdev) { + break; + } } + + /* + * If no match this may be a fresh H2 mount, make sure + * the device is not mounted on anything else. + */ + if (hmp == NULL) + error = vfs_mountedon(devvp); } else if (error == 0) { /* * Match the label to a pmp already probed. @@ -1029,8 +1045,10 @@ hammer2_vfs_mount(struct mount *mp, char *path, caddr_t data, hammer2_chain_t *schain; hammer2_xid_t xid; - if (error == 0 && vcount(devvp) > 0) + if (error == 0 && vcount(devvp) > 0) { + kprintf("Primary device already has references\n"); error = EBUSY; + } /* * Now open the device -- 2.11.4.GIT