Date: Tue, 22 Sep 2015 08:55:38 +0200 From: Oliver Pinter <oliver.pinter@hardenedbsd.org> To: Kirk McKusick <mckusick@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: Re: svn commit: r288079 - in stable/10/sys: kern sys Message-ID: <CAPQ4ffs8qWwC4LHAYNno7ddo818ub7ijf5wnowHgqmS=iEknWA@mail.gmail.com> In-Reply-To: <CAPQ4ffuJ-RFEHpBvEQnjJRO=AKJK0wyyrgB1aR5T7MJpptQrJQ@mail.gmail.com> References: <201509220043.t8M0h6jQ034824@repo.freebsd.org> <CAPQ4ffuJ-RFEHpBvEQnjJRO=AKJK0wyyrgB1aR5T7MJpptQrJQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 9/22/15, Oliver Pinter <oliver.pinter@hardenedbsd.org> wrote: > On 9/22/15, Kirk McKusick <mckusick@freebsd.org> wrote: >> Author: mckusick >> Date: Tue Sep 22 00:43:05 2015 >> New Revision: 288079 >> URL: https://svnweb.freebsd.org/changeset/base/288079 >> >> Log: >> MFC of 281677: >> >> More accurately collect name-cache statistics in sysctl functions >> sysctl_debug_hashstat_nchash() and sysctl_debug_hashstat_rawnchash(). >> These changes are in preparation for allowing changes in the size >> of the vnode hash tables driven by increases and decreases in the >> maximum number of vnodes in the system. >> >> Reviewed by: kib@ >> Phabric: D2265 >> >> MFC of 287497: >> >> Track changes to kern.maxvnodes and appropriately increase or decrease >> the size of the name cache hash table (mapping file names to vnodes) >> and the vnode hash table (mapping mount point and inode number to >> vnode). >> An appropriate locking strategy is the key to changing hash table sizes >> while they are in active use. >> >> Reviewed by: kib >> Tested by: Peter Holm >> Differential Revision: https://reviews.freebsd.org/D2265 >> >> Modified: >> stable/10/sys/kern/vfs_cache.c >> stable/10/sys/kern/vfs_hash.c >> stable/10/sys/kern/vfs_subr.c >> stable/10/sys/sys/vnode.h >> Directory Properties: >> stable/10/ (props changed) >> >> Modified: stable/10/sys/kern/vfs_cache.c >> ============================================================================== >> --- stable/10/sys/kern/vfs_cache.c Mon Sep 21 22:34:16 2015 (r288078) >> +++ stable/10/sys/kern/vfs_cache.c Tue Sep 22 00:43:05 2015 (r288079) >> @@ -330,23 +330,27 @@ sysctl_debug_hashstat_rawnchash(SYSCTL_H >> int n_nchash; >> int count; >> >> +retry: >> n_nchash = nchash + 1; /* nchash is max index, not count */ >> if (!req->oldptr) >> return SYSCTL_OUT(req, 0, n_nchash * sizeof(int)); >> - >> - /* Scan hash tables for applicable entries */ >> - for (ncpp = nchashtbl; n_nchash > 0; n_nchash--, ncpp++) { >> - CACHE_RLOCK(); >> - count = 0; >> - LIST_FOREACH(ncp, ncpp, nc_hash) { >> - count++; >> - } >> + cntbuf = malloc(n_nchash * sizeof(int), M_TEMP, M_ZERO | M_WAITOK); >> + CACHE_RLOCK(); >> + if (n_nchash != nchash + 1) { >> CACHE_RUNLOCK(); >> - error = SYSCTL_OUT(req, &count, sizeof(count)); >> - if (error) >> - return (error); >> + free(cntbuf, M_TEMP); >> + goto retry; >> } >> - return (0); >> + /* Scan hash tables counting entries */ >> + for (ncpp = nchashtbl, i = 0; i < n_nchash; ncpp++, i++) >> + LIST_FOREACH(ncp, ncpp, nc_hash) >> + cntbuf[i]++; >> + CACHE_RUNLOCK(); >> + for (error = 0, i = 0; i < n_nchash; i++) >> + if ((error = SYSCTL_OUT(req, &cntbuf[i], sizeof(int))) != 0) >> + break; >> + free(cntbuf, M_TEMP); >> + return (error); >> } >> SYSCTL_PROC(_debug_hashstat, OID_AUTO, rawnchash, >> CTLTYPE_INT|CTLFLAG_RD| >> CTLFLAG_MPSAFE, 0, 0, sysctl_debug_hashstat_rawnchash, "S,int", >> @@ -935,6 +939,44 @@ nchinit(void *dummy __unused) >> } >> SYSINIT(vfs, SI_SUB_VFS, SI_ORDER_SECOND, nchinit, NULL); >> >> +void >> +cache_changesize(int newmaxvnodes) >> +{ >> + struct nchashhead *new_nchashtbl, *old_nchashtbl; >> + u_long new_nchash, old_nchash; >> + struct namecache *ncp; >> + uint32_t hash; >> + int i; >> + >> + new_nchashtbl = hashinit(newmaxvnodes * 2, M_VFSCACHE, &new_nchash); >> + /* If same hash table size, nothing to do */ >> + if (nchash == new_nchash) { >> + free(new_nchashtbl, M_VFSCACHE); >> + return; >> + } >> + /* >> + * Move everything from the old hash table to the new table. >> + * None of the namecache entries in the table can be removed >> + * because to do so, they have to be removed from the hash table. >> + */ >> + CACHE_WLOCK(); >> + old_nchashtbl = nchashtbl; >> + old_nchash = nchash; >> + nchashtbl = new_nchashtbl; >> + nchash = new_nchash; >> + for (i = 0; i <= old_nchash; i++) { >> + while ((ncp = LIST_FIRST(&old_nchashtbl[i])) != NULL) { >> + hash = fnv_32_buf(nc_get_name(ncp), ncp->nc_nlen, >> + FNV1_32_INIT); >> + hash = fnv_32_buf(&ncp->nc_dvp, sizeof(ncp->nc_dvp), >> + hash); >> + LIST_REMOVE(ncp, nc_hash); >> + LIST_INSERT_HEAD(NCHHASH(hash), ncp, nc_hash); >> + } >> + } >> + CACHE_WUNLOCK(); >> + free(old_nchashtbl, M_VFSCACHE); >> +} >> >> /* >> * Invalidate all entries to a particular vnode. >> >> Modified: stable/10/sys/kern/vfs_hash.c >> ============================================================================== >> --- stable/10/sys/kern/vfs_hash.c Mon Sep 21 22:34:16 2015 (r288078) >> +++ stable/10/sys/kern/vfs_hash.c Tue Sep 22 00:43:05 2015 (r288079) >> @@ -160,3 +160,40 @@ vfs_hash_rehash(struct vnode *vp, u_int >> vp->v_hash = hash; >> mtx_unlock(&vfs_hash_mtx); >> } >> + >> +void >> +vfs_hash_changesize(int newmaxvnodes) >> +{ >> + struct vfs_hash_head *vfs_hash_newtbl, *vfs_hash_oldtbl; >> + u_long vfs_hash_newmask, vfs_hash_oldmask; >> + struct vnode *vp; >> + int i; >> + >> + vfs_hash_newtbl = hashinit(newmaxvnodes, M_VFS_HASH, >> + &vfs_hash_newmask); >> + /* If same hash table size, nothing to do */ >> + if (vfs_hash_mask == vfs_hash_newmask) { >> + free(vfs_hash_newtbl, M_VFS_HASH); >> + return; >> + } >> + /* >> + * Move everything from the old hash table to the new table. >> + * None of the vnodes in the table can be recycled because to >> + * do so, they have to be removed from the hash table. >> + */ >> + rw_wlock(&vfs_hash_lock); >> + vfs_hash_oldtbl = vfs_hash_tbl; >> + vfs_hash_oldmask = vfs_hash_mask; >> + vfs_hash_tbl = vfs_hash_newtbl; >> + vfs_hash_mask = vfs_hash_newmask; >> + for (i = 0; i <= vfs_hash_oldmask; i++) { >> + while ((vp = LIST_FIRST(&vfs_hash_oldtbl[i])) != NULL) { >> + LIST_REMOVE(vp, v_hashlist); >> + LIST_INSERT_HEAD( >> + vfs_hash_bucket(vp->v_mount, vp->v_hash), >> + vp, v_hashlist); >> + } >> + } >> + rw_wunlock(&vfs_hash_lock); >> + free(vfs_hash_oldtbl, M_VFS_HASH); >> +} >> >> Modified: stable/10/sys/kern/vfs_subr.c >> ============================================================================== >> --- stable/10/sys/kern/vfs_subr.c Mon Sep 21 22:34:16 2015 (r288078) >> +++ stable/10/sys/kern/vfs_subr.c Tue Sep 22 00:43:05 2015 (r288079) >> @@ -280,8 +280,25 @@ static enum { SYNCER_RUNNING, SYNCER_SHU >> * XXX desiredvnodes is historical cruft and should not exist. >> */ >> int desiredvnodes; >> -SYSCTL_INT(_kern, KERN_MAXVNODES, maxvnodes, CTLFLAG_RW, >> - &desiredvnodes, 0, "Maximum number of vnodes"); >> + >> +static int >> +sysctl_update_desiredvnodes(SYSCTL_HANDLER_ARGS) >> +{ >> + int error, old_desiredvnodes; >> + >> + old_desiredvnodes = desiredvnodes; >> + if ((error = sysctl_handle_int(oidp, arg1, arg2, req)) != 0) >> + return (error); >> + if (old_desiredvnodes != desiredvnodes) { >> + vfs_hash_changesize(desiredvnodes); >> + cache_changesize(desiredvnodes); >> + } >> + return (0); >> +} >> + >> +SYSCTL_PROC(_kern, KERN_MAXVNODES, maxvnodes, >> + CTLTYPE_INT | CTLFLAG_MPSAFE | CTLFLAG_RW, &desiredvnodes, 0, >> + sysctl_update_desiredvnodes, "I", "Maximum number of vnodes"); >> SYSCTL_ULONG(_kern, OID_AUTO, minvnodes, CTLFLAG_RW, >> &wantfreevnodes, 0, "Minimum number of vnodes (legacy)"); >> static int vnlru_nowhere; >> >> Modified: stable/10/sys/sys/vnode.h >> ============================================================================== >> --- stable/10/sys/sys/vnode.h Mon Sep 21 22:34:16 2015 (r288078) >> +++ stable/10/sys/sys/vnode.h Tue Sep 22 00:43:05 2015 (r288079) >> @@ -600,6 +600,7 @@ struct vnode; >> typedef int (*vn_get_ino_t)(struct mount *, void *, int, struct vnode >> **); >> >> /* cache_* may belong in namei.h. */ >> +void cache_changesize(int newhashsize); >> #define cache_enter(dvp, vp, cnp) \ >> cache_enter_time(dvp, vp, cnp, NULL, NULL) >> void cache_enter_time(struct vnode *dvp, struct vnode *vp, >> @@ -836,6 +837,7 @@ int fifo_printinfo(struct vnode *); >> /* vfs_hash.c */ >> typedef int vfs_hash_cmp_t(struct vnode *vp, void *arg); >> >> +void vfs_hash_changesize(int newhashsize); >> int vfs_hash_get(const struct mount *mp, u_int hash, int flags, struct >> thread *td, struct vnode **vpp, vfs_hash_cmp_t *fn, void *arg); >> u_int vfs_hash_index(struct vnode *vp); >> int vfs_hash_insert(struct vnode *vp, u_int hash, int flags, struct >> thread >> *td, struct vnode **vpp, vfs_hash_cmp_t *fn, void *arg); > > Seems like this commit breaks the build: > > --- vfs_hash.o --- > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:184:2: > error: implicit declaration of function 'rw_wlock' is invalid in C99 > [-Werror,-Wimplicit-function-declaration] > rw_wlock(&vfs_hash_lock); > ^ > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:184:12: > error: use of undeclared identifier 'vfs_hash_lock'; did you mean > 'vfs_hash_mask'? > rw_wlock(&vfs_hash_lock); > ^~~~~~~~~~~~~ > vfs_hash_mask > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:42:18: > note: 'vfs_hash_mask' declared here > static u_long vfs_hash_mask; > ^ > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:197:2: > error: implicit declaration of function 'rw_wunlock' is invalid in C99 > [-Werror,-Wimplicit-function-declaration] > rw_wunlock(&vfs_hash_lock); > ^ > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:197:2: > note: did you mean 'rw_wlock'? > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:184:2: > note: 'rw_wlock' declared here > rw_wlock(&vfs_hash_lock); > ^ > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:197:14: > error: use of undeclared identifier 'vfs_hash_lock'; did you mean > 'vfs_hash_mask'? > rw_wunlock(&vfs_hash_lock); > ^~~~~~~~~~~~~ > vfs_hash_mask > /jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/kern/vfs_hash.c:42:18: > note: 'vfs_hash_mask' declared here > static u_long vfs_hash_mask; > ^ > 4 errors generated. > *** [vfs_hash.o] Error code 1 > > make[2]: stopped in > /usr/obj/jenkins/workspace/HardenedBSD-10-STABLE-amd64/sys/HARDENEDBSD > --- vfs_export.o --- > ctfconvert -L VERSION -g vfs_export.o > > http://jenkins.hardenedbsd.org:8180/jenkins/job/HardenedBSD-10-STABLE-amd64/99/console The following commit is missing from MFC: commit 09f7a46ec71137e29d82d2f0811a7f81b6142266 Author: mjg <mjg@FreeBSD.org> Date: Tue Dec 30 21:40:45 2014 +0000 Convert vfs hash lock from a mutex to an rwlock. > >> _______________________________________________ >> svn-src-stable-10@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/svn-src-stable-10 >> To unsubscribe, send any mail to >> "svn-src-stable-10-unsubscribe@freebsd.org" >> >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffs8qWwC4LHAYNno7ddo818ub7ijf5wnowHgqmS=iEknWA>