From: Matthew Dillon Date: Fri, 10 Feb 2012 03:28:11 +0000 (-0800) Subject: hammer2 - cleanup, kldload, mount, iget/ifree and other stuff. X-Git-Tag: v3.4.0rc~1228 X-Git-Url: https://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/54eb943bf11899aee5aa68e71ab7e8911517e72f hammer2 - cleanup, kldload, mount, iget/ifree and other stuff. * Make kldload and kldunload run cleanly * Make mount/umount run cleanly. Properly deallocate the root inode and close the underlying device. * Write iget/ifree and properly deal with deadlocks between reclaim and iget. * Remove the hm_ prefix for the hammer2_mount structure. * Use 'hammer2_xxx_t' instead of 'struct hammer2_xxx' in many cases. * Other general cleanups. --- diff --git a/sys/vfs/hammer2/dotest b/sys/vfs/hammer2/dotest new file mode 100755 index 0000000000..cdbd70a994 --- /dev/null +++ b/sys/vfs/hammer2/dotest @@ -0,0 +1,6 @@ +#!/bin/csh +# + +# ./mkvntest +# kldload /usr/obj/usr/src/sys/vfs/hammer2/hammer2.ko +mount_hammer2 /dev/vn0@ROOTx /mnt diff --git a/sys/vfs/hammer2/hammer2.h b/sys/vfs/hammer2/hammer2.h index b38eda3cd4..0af4541f5f 100644 --- a/sys/vfs/hammer2/hammer2.h +++ b/sys/vfs/hammer2/hammer2.h @@ -73,14 +73,17 @@ struct hammer2_mount; * A hammer2 inode. */ struct hammer2_inode { - struct hammer2_mount *mp; + struct hammer2_mount *hmp; struct lock lk; struct vnode *vp; - hammer2_tid_t inum; + hammer2_inode_data_t data; unsigned char type; int busy; + u_int refs; }; +typedef struct hammer2_inode hammer2_inode_t; + #define HAMMER2_INODE_TYPE_DIR 0x01 #define HAMMER2_INODE_TYPE_FILE 0x02 #define HAMMER2_INODE_TYPE_ROOT 0x10 @@ -90,28 +93,28 @@ struct hammer2_inode { * Governing mount structure for filesystem (aka vp->v_mount) */ struct hammer2_mount { - struct mount *hm_mp; - int hm_ronly; /* block device mounted read-only */ - struct vnode *hm_devvp; /* device vnode */ - struct lock hm_lk; + struct mount *mp; + int ronly; /* block device mounted read-only */ + struct vnode *devvp; /* device vnode */ + struct lock lk; - /* Root inode */ - struct hammer2_inode *hm_iroot; + struct hammer2_inode *iroot; - /* Per-mount inode zone */ - struct malloc_type *hm_inodes; - int hm_ninodes; - int hm_maxinodes; + struct malloc_type *inodes; + int ninodes; + int maxinodes; - struct malloc_type *hm_ipstacks; - int hm_nipstacks; - int hm_maxipstacks; + struct malloc_type *ipstacks; + int nipstacks; + int maxipstacks; - struct hammer2_volume_data hm_sb; + hammer2_volume_data_t voldata; - struct netexport hm_export; + struct netexport export; }; +typedef struct hammer2_mount hammer2_mount_t; + #if defined(_KERNEL) MALLOC_DECLARE(M_HAMMER2); @@ -120,17 +123,17 @@ static __inline struct mount * H2TOMP(struct hammer2_mount *hmp) { - return (struct mount *) hmp->hm_mp; + return (struct mount *) hmp->mp; } -#define VTOI(vp) ((struct hammer2_inode *) (vp)->v_data) +#define VTOI(vp) ((hammer2_inode_t *)(vp)->v_data) #define ITOV(ip) ((ip)->vp) static __inline struct hammer2_mount * MPTOH2(struct mount *mp) { - return (struct hammer2_mount *) mp->mnt_data; + return (hammer2_mount_t *) mp->mnt_data; } extern struct vop_ops hammer2_vnode_vops; @@ -139,19 +142,20 @@ extern struct vop_ops hammer2_fifo_vops; /* hammer2_subr.c */ -void hammer2_inode_lock_sh(struct hammer2_inode *ip); -void hammer2_inode_lock_up(struct hammer2_inode *ip); -void hammer2_inode_lock_ex(struct hammer2_inode *ip); -void hammer2_inode_unlock_ex(struct hammer2_inode *ip); -void hammer2_inode_unlock_up(struct hammer2_inode *ip); -void hammer2_inode_unlock_sh(struct hammer2_inode *ip); +void hammer2_inode_lock_sh(hammer2_inode_t *ip); +void hammer2_inode_lock_up(hammer2_inode_t *ip); +void hammer2_inode_lock_ex(hammer2_inode_t *ip); +void hammer2_inode_unlock_ex(hammer2_inode_t *ip); +void hammer2_inode_unlock_up(hammer2_inode_t *ip); +void hammer2_inode_unlock_sh(hammer2_inode_t *ip); -struct vnode *igetv(struct hammer2_inode *, int *); +void hammer2_mount_exlock(hammer2_mount_t *hmp); +void hammer2_mount_shlock(hammer2_mount_t *hmp); +void hammer2_mount_unlock(hammer2_mount_t *hmp); -void hammer2_mount_exlock(struct hammer2_mount *); -void hammer2_mount_shlock(struct hammer2_mount *); -void hammer2_mount_unlock(struct hammer2_mount *); -struct hammer2_inode *alloci(struct hammer2_mount *hmp); +struct vnode *hammer2_igetv(hammer2_inode_t *ip, int *errorp); +hammer2_inode_t *hammer2_alloci(hammer2_mount_t *hmp); +void hammer2_freei(hammer2_inode_t *ip); #endif /* !_KERNEL */ #endif /* !_VFS_HAMMER2_HAMMER2_H_ */ diff --git a/sys/vfs/hammer2/hammer2_subr.c b/sys/vfs/hammer2/hammer2_subr.c index 020b50feca..27be6342c7 100644 --- a/sys/vfs/hammer2/hammer2_subr.c +++ b/sys/vfs/hammer2/hammer2_subr.c @@ -61,13 +61,13 @@ */ void -hammer2_inode_lock_sh(struct hammer2_inode *ip) +hammer2_inode_lock_sh(hammer2_inode_t *ip) { lockmgr(&ip->lk, LK_SHARED); } void -hammer2_inode_lock_up(struct hammer2_inode *ip) +hammer2_inode_lock_up(hammer2_inode_t *ip) { lockmgr(&ip->lk, LK_EXCLUSIVE); ++ip->busy; @@ -75,19 +75,19 @@ hammer2_inode_lock_up(struct hammer2_inode *ip) } void -hammer2_inode_lock_ex(struct hammer2_inode *ip) +hammer2_inode_lock_ex(hammer2_inode_t *ip) { lockmgr(&ip->lk, LK_EXCLUSIVE); } void -hammer2_inode_unlock_ex(struct hammer2_inode *ip) +hammer2_inode_unlock_ex(hammer2_inode_t *ip) { lockmgr(&ip->lk, LK_RELEASE); } void -hammer2_inode_unlock_up(struct hammer2_inode *ip) +hammer2_inode_unlock_up(hammer2_inode_t *ip) { lockmgr(&ip->lk, LK_UPGRADE); --ip->busy; @@ -95,7 +95,7 @@ hammer2_inode_unlock_up(struct hammer2_inode *ip) } void -hammer2_inode_unlock_sh(struct hammer2_inode *ip) +hammer2_inode_unlock_sh(hammer2_inode_t *ip) { lockmgr(&ip->lk, LK_RELEASE); } @@ -105,21 +105,21 @@ hammer2_inode_unlock_sh(struct hammer2_inode *ip) */ void -hammer2_mount_exlock(struct hammer2_mount *hmp) +hammer2_mount_exlock(hammer2_mount_t *hmp) { - lockmgr(&hmp->hm_lk, LK_EXCLUSIVE); + lockmgr(&hmp->lk, LK_EXCLUSIVE); } void -hammer2_mount_shlock(struct hammer2_mount *hmp) +hammer2_mount_shlock(hammer2_mount_t *hmp) { - lockmgr(&hmp->hm_lk, LK_SHARED); + lockmgr(&hmp->lk, LK_SHARED); } void -hammer2_mount_unlock(struct hammer2_mount *hmp) +hammer2_mount_unlock(hammer2_mount_t *hmp) { - lockmgr(&hmp->hm_lk, LK_RELEASE); + lockmgr(&hmp->lk, LK_RELEASE); } /* @@ -127,45 +127,85 @@ hammer2_mount_unlock(struct hammer2_mount *hmp) */ /* - * igetv: + * Get the vnode associated with the given inode, allocating the vnode if + * necessary. * - * Get a vnode associated with the given inode. If one exists, return it, - * locked and ref-ed. Otherwise, a new vnode is allocated and associated - * with the vnode. + * Great care must be taken to avoid deadlocks and vnode acquisition/reclaim + * races. * - * The lock prevents the inode from being reclaimed, I believe (XXX) + * The vnode will be returned exclusively locked and referenced. The + * reference on the vnode prevents it from being reclaimed. + * + * The inode (ip) must be referenced by the caller and not locked to avoid + * it getting ripped out from under us or deadlocked. */ struct vnode * -igetv(struct hammer2_inode *ip, int *error) +hammer2_igetv(hammer2_inode_t *ip, int *errorp) { struct vnode *vp; - struct hammer2_mount *hmp; - int rc; - - hmp = ip->mp; - rc = 0; - - kprintf("igetv\n"); - tsleep(&igetv, 0, "", hz * 10); - - hammer2_inode_lock_ex(ip); - do { - /* Reuse existing vnode */ + hammer2_mount_t *hmp; + + hmp = ip->hmp; + *errorp = 0; + + for (;;) { + /* + * Attempt to reuse an existing vnode assignment. It is + * possible to race a reclaim so the vget() may fail. The + * inode must be unlocked during the vget() to avoid a + * deadlock against a reclaim. + */ vp = ip->vp; if (vp) { - /* XXX: Is this necessary? */ - vx_lock(vp); + /* + * Lock the inode and check for a reclaim race + */ + hammer2_inode_lock_ex(ip); + if (ip->vp != vp) { + hammer2_inode_unlock_ex(ip); + continue; + } + + /* + * Inode must be unlocked during the vget() to avoid + * possible deadlocks, vnode is held to prevent + * destruction during the vget(). The vget() can + * still fail if we lost a reclaim race on the vnode. + */ + vhold_interlocked(vp); + hammer2_inode_unlock_ex(ip); + if (vget(vp, LK_EXCLUSIVE)) { + vdrop(vp); + continue; + } + vdrop(vp); + /* vp still locked and ref from vget */ + *errorp = 0; break; } - /* Allocate and initialize a new vnode */ - rc = getnewvnode(VT_HAMMER2, H2TOMP(hmp), &vp, - VLKTIMEOUT, LK_CANRECURSE); - if (rc) { + /* + * No vnode exists, allocate a new vnode. Beware of + * allocation races. This function will return an + * exclusively locked and referenced vnode. + */ + *errorp = getnewvnode(VT_HAMMER2, H2TOMP(hmp), &vp, 0, 0); + if (*errorp) { vp = NULL; break; } + /* + * Lock the inode and check for an allocation race. + */ + hammer2_inode_lock_ex(ip); + if (ip->vp != NULL) { + vp->v_type = VBAD; + vx_put(vp); + hammer2_inode_unlock_ex(ip); + continue; + } + kprintf("igetv new\n"); switch (ip->type & HAMMER2_INODE_TYPE_MASK) { case HAMMER2_INODE_TYPE_DIR: @@ -173,9 +213,10 @@ igetv(struct hammer2_inode *ip, int *error) break; case HAMMER2_INODE_TYPE_FILE: vp->v_type = VREG; - /*XXX: Init w/ true file size; 0*/ - vinitvmio(vp, 0, PAGE_SIZE, -1); + vinitvmio(vp, 0, HAMMER2_LBUFSIZE, + (int)ip->data.size & HAMMER2_LBUFMASK); break; + /* XXX FIFO */ default: break; } @@ -185,49 +226,60 @@ igetv(struct hammer2_inode *ip, int *error) vp->v_data = ip; ip->vp = vp; - } while (0); - hammer2_inode_unlock_ex(ip); + hammer2_inode_unlock_ex(ip); + break; + } /* - * XXX: Under what conditions can a vnode be reclaimed? How do we want - * to interlock against vreclaim calls into hammer2? When do we need to? + * Return non-NULL vp and *errorp == 0, or NULL vp and *errorp != 0. */ - - kprintf("igetv exit\n"); - - /* vp is either NULL or a locked, ref-ed vnode referring to inode ip */ - *error = rc; return (vp); } /* - * alloci: + * Allocate a HAMMER2 inode memory structure. * - * Allocate an inode in a HAMMER2 mount. The returned inode is locked - * exclusively. The HAMMER2 mountpoint must be locked on entry. + * The returned inode is locked exclusively and referenced. + * The HAMMER2 mountpoint must be locked on entry. */ -struct hammer2_inode * -alloci(struct hammer2_mount *hmp) +hammer2_inode_t * +hammer2_alloci(hammer2_mount_t *hmp) { - struct hammer2_inode *ip; + hammer2_inode_t *ip; kprintf("alloci\n"); - ip = kmalloc(sizeof(struct hammer2_inode), hmp->hm_inodes, - M_WAITOK | M_ZERO); + ip = kmalloc(sizeof(hammer2_inode_t), hmp->inodes, M_WAITOK | M_ZERO); if (!ip) { /* XXX */ } - ++hmp->hm_ninodes; + ++hmp->ninodes; ip->type = 0; - ip->mp = hmp; + ip->hmp = hmp; lockinit(&ip->lk, "h2inode", 0, 0); ip->vp = NULL; - + ip->refs = 1; hammer2_inode_lock_ex(ip); return (ip); } +/* + * Free a HAMMER2 inode memory structure. + * + * The inode must be locked exclusively with one reference and will + * be destroyed on return. + */ +void +hammer2_freei(hammer2_inode_t *ip) +{ + hammer2_mount_t *hmp = ip->hmp; + + KKASSERT(ip->hmp == NULL); + KKASSERT(ip->vp == NULL); + KKASSERT(ip->refs == 1); + hammer2_inode_unlock_ex(ip); + kfree(ip, hmp->inodes); +} diff --git a/sys/vfs/hammer2/hammer2_vfsops.c b/sys/vfs/hammer2/hammer2_vfsops.c index 1346fd824b..16dad39711 100644 --- a/sys/vfs/hammer2/hammer2_vfsops.c +++ b/sys/vfs/hammer2/hammer2_vfsops.c @@ -137,14 +137,14 @@ hammer2_mount(struct mount *mp, char *path, caddr_t data, struct ucred *cred) { struct hammer2_mount_info info; - struct hammer2_mount *hmp; + hammer2_mount_t *hmp; struct vnode *devvp; struct nlookupdata nd; char devstr[MNAMELEN]; size_t size; size_t done; char *dev, *label; - int ronly; + int ronly = 1; int error; #if 0 int rc; @@ -160,13 +160,11 @@ hammer2_mount(struct mount *mp, char *path, caddr_t data, /* * Root mount */ - return (EOPNOTSUPP); } else { /* * Non-root mount or updating a mount */ - error = copyin(data, &info, sizeof(info)); if (error) return (error); @@ -190,8 +188,9 @@ hammer2_mount(struct mount *mp, char *path, caddr_t data, /* Update mount */ /* HAMMER2 implements NFS export via mountctl */ hmp = MPTOH2(mp); - devvp = hmp->hm_devvp; - return hammer2_remount(mp, path, devvp, cred); + devvp = hmp->devvp; + error = hammer2_remount(mp, path, devvp, cred); + return error; } } @@ -200,76 +199,53 @@ hammer2_mount(struct mount *mp, char *path, caddr_t data, */ /* Lookup name and verify it refers to a block device */ error = nlookup_init(&nd, dev, UIO_SYSSPACE, NLC_FOLLOW); - if (error) - return (error); - error = nlookup(&nd); - if (error) - return (error); - error = cache_vref(&nd.nl_nch, nd.nl_cred, &devvp); - if (error) - return (error); + if (error == 0) + error = nlookup(&nd); + if (error == 0) + error = cache_vref(&nd.nl_nch, nd.nl_cred, &devvp); nlookup_done(&nd); - if (!vn_isdisk(devvp, &error)) { - vrele(devvp); - return (error); + if (error == 0) { + if (vn_isdisk(devvp, &error)) + error = vfs_mountedon(devvp); } + if (error == 0 && vcount(devvp) > 0) + error = EBUSY; /* - * Common path for new root/non-root mounts; - * devvp is a ref-ed by not locked vnode referring to the fs device + * Now open the device */ - - error = vfs_mountedon(devvp); - if (error) { - vrele(devvp); - return (error); + if (error == 0) { + ronly = ((mp->mnt_flag & MNT_RDONLY) != 0); + vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); + error = vinvalbuf(devvp, V_SAVE, 0, 0); + if (error == 0) { + error = VOP_OPEN(devvp, ronly ? FREAD : FREAD | FWRITE, + FSCRED, NULL); + } + vn_unlock(devvp); } - - if (vcount(devvp) > 0) { + if (error && devvp) { vrele(devvp); - return (EBUSY); + devvp = NULL; } + if (error) + return error; /* - * Open the fs device + * Block device opened successfully, finish initializing the + * mount structure. + * + * From this point on we have to call hammer2_unmount() on failure. */ - ronly = (mp->mnt_flag & MNT_RDONLY) != 0; - vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - error = vinvalbuf(devvp, V_SAVE, 0, 0); - if (error) { - vn_unlock(devvp); - vrele(devvp); - return (error); - } - /* This is correct; however due to an NFS quirk of my setup, FREAD - * is required... */ - /* - error = VOP_OPEN(devvp, ronly ? FREAD : FREAD | FWRITE, FSCRED, NULL); - */ - - error = VOP_OPEN(devvp, FREAD, FSCRED, NULL); - vn_unlock(devvp); - if (error) { - vrele(devvp); - return (error); - } - -#ifdef notyet - /* VOP_IOCTL(EXTENDED_DISK_INFO, devvp); */ - /* if vn device, never use bdwrite(); */ - /* check if device supports BUF_CMD_READALL; */ - /* check if device supports BUF_CMD_WRITEALL; */ -#endif - hmp = kmalloc(sizeof(*hmp), M_HAMMER2, M_WAITOK | M_ZERO); - mp->mnt_data = (qaddr_t) hmp; - hmp->hm_mp = mp; - hmp->hm_ronly = ronly; - hmp->hm_devvp = devvp; - lockinit(&hmp->hm_lk, "h2mp", 0, 0); - kmalloc_create(&hmp->hm_inodes, "HAMMER2-inodes"); - kmalloc_create(&hmp->hm_ipstacks, "HAMMER2-ipstacks"); + mp->mnt_data = (qaddr_t)hmp; + hmp->mp = mp; + hmp->ronly = ronly; + hmp->devvp = devvp; + lockinit(&hmp->lk, "h2mp", 0, 0); + kmalloc_create(&hmp->inodes, "HAMMER2-inodes"); + kmalloc_create(&hmp->ipstacks, "HAMMER2-ipstacks"); mp->mnt_flag = MNT_LOCAL; @@ -279,9 +255,9 @@ hammer2_mount(struct mount *mp, char *path, caddr_t data, mp->mnt_kern_flag |= MNTK_ALL_MPSAFE; /* Setup root inode */ - hmp->hm_iroot = alloci(hmp); - hmp->hm_iroot->type = HAMMER2_INODE_TYPE_DIR | HAMMER2_INODE_TYPE_ROOT; - hmp->hm_iroot->inum = 1; + hmp->iroot = hammer2_alloci(hmp); + hmp->iroot->type = HAMMER2_INODE_TYPE_DIR | HAMMER2_INODE_TYPE_ROOT; + hmp->iroot->data.inum = 1; /* currently rely on tmpfs routines */ vfs_getnewfsid(mp); @@ -298,7 +274,7 @@ hammer2_mount(struct mount *mp, char *path, caddr_t data, hammer2_statfs(mp, &mp->mnt_stat, cred); - hammer2_inode_unlock_ex(hmp->hm_iroot); + hammer2_inode_unlock_ex(hmp->iroot); return 0; } @@ -315,9 +291,11 @@ static int hammer2_unmount(struct mount *mp, int mntflags) { - struct hammer2_mount *hmp; + hammer2_mount_t *hmp; int flags; int error; + int ronly = ((mp->mnt_flag & MNT_RDONLY) != 0); + struct vnode *devvp; kprintf("hammer2_unmount\n"); @@ -339,14 +317,27 @@ hammer2_unmount(struct mount *mp, int mntflags) * 4) Free the mount point */ - kmalloc_destroy(&hmp->hm_inodes); - kmalloc_destroy(&hmp->hm_ipstacks); + if (hmp->iroot) { + hammer2_inode_lock_ex(hmp->iroot); + hammer2_freei(hmp->iroot); + hmp->iroot = NULL; + } + if ((devvp = hmp->devvp) != NULL) { + vinvalbuf(devvp, (ronly ? 0 : V_SAVE), 0, 0); + hmp->devvp = NULL; + VOP_CLOSE(devvp, (ronly ? FREAD : FREAD|FWRITE)); + vrele(devvp); + devvp = NULL; + } - hammer2_mount_unlock(hmp); + kmalloc_destroy(&hmp->inodes); + kmalloc_destroy(&hmp->ipstacks); - // Tmpfs does this - //kfree(hmp, M_HAMMER2); + hammer2_mount_unlock(hmp); + mp->mnt_data = NULL; + hmp->mp = NULL; + kfree(hmp, M_HAMMER2); return (error); } @@ -364,7 +355,7 @@ static int hammer2_root(struct mount *mp, struct vnode **vpp) { - struct hammer2_mount *hmp; + hammer2_mount_t *hmp; int error; struct vnode *vp; @@ -372,11 +363,11 @@ hammer2_root(struct mount *mp, struct vnode **vpp) hmp = MPTOH2(mp); hammer2_mount_exlock(hmp); - if (hmp->hm_iroot == NULL) { + if (hmp->iroot == NULL) { *vpp = NULL; error = EINVAL; } else { - vp = igetv(hmp->hm_iroot, &error); + vp = hammer2_igetv(hmp->iroot, &error); *vpp = vp; if (vp == NULL) kprintf("vnodefail\n"); @@ -390,7 +381,7 @@ static int hammer2_statfs(struct mount *mp, struct statfs *sbp, struct ucred *cred) { - struct hammer2_mount *hmp; + hammer2_mount_t *hmp; kprintf("hammer2_statfs\n"); @@ -439,8 +430,8 @@ int hammer2_sync(struct mount *mp, int waitfor) { #if 0 - struct hammer2_mount *hmp; - struct hammer2_inode *ip; + hammer2_mount_t *hmp; + hammer2_inode_t *ip; #endif kprintf("hammer2_sync \n"); diff --git a/sys/vfs/hammer2/hammer2_vnops.c b/sys/vfs/hammer2/hammer2_vnops.c index 8737f770fc..01cffbcc9d 100644 --- a/sys/vfs/hammer2/hammer2_vnops.c +++ b/sys/vfs/hammer2/hammer2_vnops.c @@ -78,16 +78,20 @@ hammer2_vop_reclaim(struct vop_reclaim_args *ap) struct hammer2_inode *ip; struct hammer2_mount *hmp; - kprintf("hammer2_reclaim\n"); - - /* Is the vnode locked? Must it be on exit? */ - vp = ap->a_vp; ip = VTOI(vp); - hmp = ip->mp; + hmp = ip->hmp; + hammer2_inode_lock_ex(ip); vp->v_data = NULL; ip->vp = NULL; + hammer2_inode_unlock_ex(ip); + + /* + * XXX handle background sync when ip dirty, kernel will no longer + * notify us regarding this inode because there is no longer a + * vnode attached to it. + */ return (0); } diff --git a/sys/vfs/hammer2/mkvntest b/sys/vfs/hammer2/mkvntest new file mode 100755 index 0000000000..fad5923c7e --- /dev/null +++ b/sys/vfs/hammer2/mkvntest @@ -0,0 +1,10 @@ +#!/bin/csh +# + +vnconfig -u vn0 >& /dev/null +rm -f /usr/obj/hammer2.img +truncate -s 1G /usr/obj/hammer2.img +newfs_hammer2 -L ROOT /usr/obj/hammer2.img +vnconfig -c vn0 /usr/obj/hammer2.img + +echo "hammer2.img on /dev/vn0"