svn commit: r242387 - head/sys/fs/smbfs
Davide Italiano
davide at freebsd.org
Wed Oct 31 03:57:15 UTC 2012
On Wed, Oct 31, 2012 at 4:55 AM, Davide Italiano <davide at freebsd.org> wrote:
> 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;
Alan,
I committed this because I'm going to commit in a while some changes
which depended on this commit.
If you want still to take a look at the patch to notify if there's
something wrong, I'll be more than happy.
Thanks
Davide
More information about the svn-src-head
mailing list