Date: Wed, 31 Oct 2012 03:55:34 +0000 (UTC) From: Davide Italiano <davide@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r242387 - head/sys/fs/smbfs Message-ID: <201210310355.q9V3tYu7042357@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: davide Date: Wed Oct 31 03:55:33 2012 New Revision: 242387 URL: http://svn.freebsd.org/changeset/base/242387 Log: - Do not put in the mntqueue half-constructed vnodes. - Change the code so that it relies on vfs_hash rather than on a home-made hashtable. - There's no need to inline fnv_32_buf(). Reviewed by: delphij Tested by: pho Sponsored by: iXsystems inc. Modified: head/sys/fs/smbfs/smbfs.h head/sys/fs/smbfs/smbfs_node.c head/sys/fs/smbfs/smbfs_node.h head/sys/fs/smbfs/smbfs_vfsops.c Modified: head/sys/fs/smbfs/smbfs.h ============================================================================== --- head/sys/fs/smbfs/smbfs.h Wed Oct 31 03:34:07 2012 (r242386) +++ head/sys/fs/smbfs/smbfs.h Wed Oct 31 03:55:33 2012 (r242387) @@ -82,9 +82,6 @@ struct smbmount { /* struct simplelock sm_npslock;*/ struct smbnode * sm_npstack[SMBFS_MAXPATHCOMP]; int sm_caseopt; - struct sx sm_hashlock; - LIST_HEAD(smbnode_hashhead, smbnode) *sm_hash; - u_long sm_hashlen; int sm_didrele; }; Modified: head/sys/fs/smbfs/smbfs_node.c ============================================================================== --- head/sys/fs/smbfs/smbfs_node.c Wed Oct 31 03:34:07 2012 (r242386) +++ head/sys/fs/smbfs/smbfs_node.c Wed Oct 31 03:55:33 2012 (r242387) @@ -27,6 +27,7 @@ */ #include <sys/param.h> #include <sys/systm.h> +#include <sys/fnv_hash.h> #include <sys/kernel.h> #include <sys/lock.h> #include <sys/malloc.h> @@ -52,54 +53,15 @@ #include <fs/smbfs/smbfs_node.h> #include <fs/smbfs/smbfs_subr.h> -#define SMBFS_NOHASH(smp, hval) (&(smp)->sm_hash[(hval) & (smp)->sm_hashlen]) -#define smbfs_hash_lock(smp) sx_xlock(&smp->sm_hashlock) -#define smbfs_hash_unlock(smp) sx_xunlock(&smp->sm_hashlock) - extern struct vop_vector smbfs_vnodeops; /* XXX -> .h file */ static MALLOC_DEFINE(M_SMBNODE, "smbufs_node", "SMBFS vnode private part"); static MALLOC_DEFINE(M_SMBNODENAME, "smbufs_nname", "SMBFS node name"); -int smbfs_hashprint(struct mount *mp); - -#if 0 -#ifdef SYSCTL_DECL -SYSCTL_DECL(_vfs_smbfs); -#endif -SYSCTL_PROC(_vfs_smbfs, OID_AUTO, vnprint, CTLFLAG_WR|CTLTYPE_OPAQUE, - NULL, 0, smbfs_hashprint, "S,vnlist", "vnode hash"); -#endif - -#define FNV_32_PRIME ((u_int32_t) 0x01000193UL) -#define FNV1_32_INIT ((u_int32_t) 33554467UL) - -u_int32_t +u_int32_t __inline smbfs_hash(const u_char *name, int nmlen) { - u_int32_t v; - - for (v = FNV1_32_INIT; nmlen; name++, nmlen--) { - v *= FNV_32_PRIME; - v ^= (u_int32_t)*name; - } - return v; -} - -int -smbfs_hashprint(struct mount *mp) -{ - struct smbmount *smp = VFSTOSMBFS(mp); - struct smbnode_hashhead *nhpp; - struct smbnode *np; - int i; - - for(i = 0; i <= smp->sm_hashlen; i++) { - nhpp = &smp->sm_hash[i]; - LIST_FOREACH(np, nhpp, n_hash) - vprint("", SMBTOV(np)); - } - return 0; + return (fnv_32_buf(name, nmlen, FNV1_32_INIT)); } static char * @@ -149,6 +111,20 @@ smbfs_name_free(u_char *name) #endif } +static int __inline +smbfs_vnode_cmp(struct vnode *vp, void *_sc) +{ + struct smbnode *np; + struct smbcmp *sc; + + np = (struct smbnode *) vp; + sc = (struct smbcmp *) _sc; + if (np->n_parent != sc->n_parent || np->n_nmlen != sc->n_nmlen || + bcmp(sc->n_name, np->n_name, sc->n_nmlen) != 0) + return 1; + return 0; +} + static int smbfs_node_alloc(struct mount *mp, struct vnode *dvp, const char *name, int nmlen, struct smbfattr *fap, struct vnode **vpp) @@ -156,12 +132,14 @@ smbfs_node_alloc(struct mount *mp, struc struct vattr vattr; struct thread *td = curthread; /* XXX */ struct smbmount *smp = VFSTOSMBFS(mp); - struct smbnode_hashhead *nhpp; - struct smbnode *np, *np2, *dnp; - struct vnode *vp; - u_long hashval; + struct smbnode *np, *dnp; + struct vnode *vp, *vp2; + struct smbcmp sc; int error; + sc.n_parent = dvp; + sc.n_nmlen = nmlen; + sc.n_name = name; *vpp = NULL; if (smp->sm_root != NULL && dvp == NULL) { SMBERROR("do not allocate root vnode twice!\n"); @@ -184,38 +162,33 @@ smbfs_node_alloc(struct mount *mp, struc vprint("smbfs_node_alloc: dead parent vnode", dvp); return EINVAL; } - hashval = smbfs_hash(name, nmlen); -retry: - smbfs_hash_lock(smp); -loop: - nhpp = SMBFS_NOHASH(smp, hashval); - LIST_FOREACH(np, nhpp, n_hash) { - vp = SMBTOV(np); - if (np->n_parent != dvp || - np->n_nmlen != nmlen || bcmp(name, np->n_name, nmlen) != 0) - continue; - VI_LOCK(vp); - smbfs_hash_unlock(smp); - if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td) != 0) - goto retry; + *vpp = NULL; + error = vfs_hash_get(mp, smbfs_hash(name, nmlen), LK_EXCLUSIVE, td, + vpp, smbfs_vnode_cmp, &sc); + if (error) + return (error); + if (*vpp) { + np = VTOSMB(*vpp); /* Force cached attributes to be refreshed if stale. */ - (void)VOP_GETATTR(vp, &vattr, td->td_ucred); + (void)VOP_GETATTR(*vpp, &vattr, td->td_ucred); /* * If the file type on the server is inconsistent with * what it was when we created the vnode, kill the * bogus vnode now and fall through to the code below * to create a new one with the right type. */ - if ((vp->v_type == VDIR && (np->n_dosattr & SMB_FA_DIR) == 0) || - (vp->v_type == VREG && (np->n_dosattr & SMB_FA_DIR) != 0)) { - vgone(vp); - vput(vp); - break; + if (((*vpp)->v_type == VDIR && + (np->n_dosattr & SMB_FA_DIR) == 0) || + ((*vpp)->v_type == VREG && + (np->n_dosattr & SMB_FA_DIR) != 0)) { + vgone(*vpp); + vput(*vpp); + } + else { + SMBVDEBUG("vnode taken from the hashtable\n"); + return (0); } - *vpp = vp; - return 0; } - smbfs_hash_unlock(smp); /* * If we don't have node attributes, then it is an explicit lookup * for an existing vnode. @@ -223,15 +196,13 @@ loop: if (fap == NULL) return ENOENT; - error = getnewvnode("smbfs", mp, &smbfs_vnodeops, &vp); - if (error != 0) - return (error); - error = insmntque(vp, mp); /* XXX: Too early for mpsafe fs */ - if (error != 0) + error = getnewvnode("smbfs", mp, &smbfs_vnodeops, vpp); + if (error) return (error); - + vp = *vpp; np = malloc(sizeof *np, M_SMBNODE, M_WAITOK | M_ZERO); - + lockmgr(vp->v_vnlock, LK_EXCLUSIVE, NULL); + /* Vnode initialization */ vp->v_type = fap->fa_attr & SMB_FA_DIR ? VDIR : VREG; vp->v_data = np; np->n_vnode = vp; @@ -239,7 +210,6 @@ loop: np->n_nmlen = nmlen; np->n_name = smbfs_name_alloc(name, nmlen); np->n_ino = fap->fa_ino; - if (dvp) { ASSERT_VOP_LOCKED(dvp, "smbfs_node_alloc"); np->n_parent = dvp; @@ -249,24 +219,18 @@ loop: } } else if (vp->v_type == VREG) SMBERROR("new vnode '%s' born without parent ?\n", np->n_name); - - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); - VN_LOCK_AREC(vp); - - smbfs_hash_lock(smp); - LIST_FOREACH(np2, nhpp, n_hash) { - if (np2->n_parent != dvp || - np2->n_nmlen != nmlen || bcmp(name, np2->n_name, nmlen) != 0) - continue; - vput(vp); -/* smb_name_free(np->n_name); - free(np, M_SMBNODE);*/ - goto loop; + error = insmntque(vp, mp); + if (error) { + free(np, M_SMBNODE); + return (error); } - LIST_INSERT_HEAD(nhpp, np, n_hash); - smbfs_hash_unlock(smp); - *vpp = vp; - return 0; + error = vfs_hash_insert(vp, smbfs_hash(name, nmlen), LK_EXCLUSIVE, + td, &vp2, smbfs_vnode_cmp, &sc); + if (error) + return (error); + if (vp2 != NULL) + *vpp = vp2; + return (0); } int @@ -307,26 +271,21 @@ smbfs_reclaim(ap) KASSERT((np->n_flag & NOPEN) == 0, ("file not closed before reclaim")); - smbfs_hash_lock(smp); /* * Destroy the vm object and flush associated pages. */ vnode_destroy_vobject(vp); - dvp = (np->n_parent && (np->n_flag & NREFPARENT)) ? np->n_parent : NULL; - - if (np->n_hash.le_prev) - LIST_REMOVE(np, n_hash); - if (smp->sm_root == np) { - SMBVDEBUG("root vnode\n"); - smp->sm_root = NULL; - } - vp->v_data = NULL; - smbfs_hash_unlock(smp); + + /* + * Remove the vnode from its hash chain. + */ + vfs_hash_remove(vp); if (np->n_name) smbfs_name_free(np->n_name); free(np, M_SMBNODE); + vp->v_data = NULL; if (dvp != NULL) { vrele(dvp); /* Modified: head/sys/fs/smbfs/smbfs_node.h ============================================================================== --- head/sys/fs/smbfs/smbfs_node.h Wed Oct 31 03:34:07 2012 (r242386) +++ head/sys/fs/smbfs/smbfs_node.h Wed Oct 31 03:55:33 2012 (r242387) @@ -63,6 +63,12 @@ struct smbnode { LIST_ENTRY(smbnode) n_hash; }; +struct smbcmp { + struct vnode * n_parent; + int n_nmlen; + const char * n_name; +}; + #define VTOSMB(vp) ((struct smbnode *)(vp)->v_data) #define SMBTOV(np) ((struct vnode *)(np)->n_vnode) Modified: head/sys/fs/smbfs/smbfs_vfsops.c ============================================================================== --- head/sys/fs/smbfs/smbfs_vfsops.c Wed Oct 31 03:34:07 2012 (r242386) +++ head/sys/fs/smbfs/smbfs_vfsops.c Wed Oct 31 03:55:33 2012 (r242387) @@ -58,8 +58,6 @@ SYSCTL_NODE(_vfs, OID_AUTO, smbfs, CTLFL SYSCTL_INT(_vfs_smbfs, OID_AUTO, version, CTLFLAG_RD, &smbfs_version, 0, ""); SYSCTL_INT(_vfs_smbfs, OID_AUTO, debuglevel, CTLFLAG_RW, &smbfs_debuglevel, 0, ""); -static MALLOC_DEFINE(M_SMBFSHASH, "smbfs_hash", "SMBFS hash table"); - static vfs_init_t smbfs_init; static vfs_uninit_t smbfs_uninit; static vfs_cmount_t smbfs_cmount; @@ -170,10 +168,6 @@ smbfs_mount(struct mount *mp) smp = malloc(sizeof(*smp), M_SMBFSDATA, M_WAITOK | M_ZERO); mp->mnt_data = smp; - smp->sm_hash = hashinit(desiredvnodes, M_SMBFSHASH, &smp->sm_hashlen); - if (smp->sm_hash == NULL) - goto bad; - sx_init(&smp->sm_hashlock, "smbfsh"); smp->sm_share = ssp; smp->sm_root = NULL; if (1 != vfs_scanopt(mp->mnt_optnew, @@ -243,12 +237,6 @@ smbfs_mount(struct mount *mp) smbfs_free_scred(scred); return error; bad: - if (smp) { - if (smp->sm_hash) - free(smp->sm_hash, M_SMBFSHASH); - sx_destroy(&smp->sm_hashlock); - free(smp, M_SMBFSDATA); - } if (ssp) smb_share_put(ssp, scred); smbfs_free_scred(scred); @@ -291,10 +279,6 @@ smbfs_unmount(struct mount *mp, int mntf goto out; smb_share_put(smp->sm_share, scred); mp->mnt_data = NULL; - - if (smp->sm_hash) - free(smp->sm_hash, M_SMBFSHASH); - sx_destroy(&smp->sm_hashlock); free(smp, M_SMBFSDATA); MNT_ILOCK(mp); mp->mnt_flag &= ~MNT_LOCAL;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201210310355.q9V3tYu7042357>