Skip site navigation (1)Skip section navigation (2)
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>