Re: svn commit: r306224 - head/sys/kern
This causes the AR9331 / Carambola2 kernel to just plainly not start. :( -adrian On 22 September 2016 at 21:45, Mateusz Guzik wrote: > Author: mjg > Date: Fri Sep 23 04:45:11 2016 > New Revision: 306224 > URL: https://svnweb.freebsd.org/changeset/base/306224 > > Log: > cache: get rid of the global lock > > Add a table of vnode locks and use them along with bucketlocks to provide > concurrent modification support. The approach taken is to preserve the > current behaviour of the namecache and just lock all relevant parts before > any changes are made. > > Lookups still require the relevant bucket to be locked. > > Discussed with: kib > Tested by:pho > > Modified: > head/sys/kern/subr_witness.c > head/sys/kern/vfs_cache.c > > Modified: head/sys/kern/subr_witness.c > == > --- head/sys/kern/subr_witness.cFri Sep 23 03:21:40 2016 > (r306223) > +++ head/sys/kern/subr_witness.cFri Sep 23 04:45:11 2016 > (r306224) > @@ -625,7 +625,7 @@ static struct witness_order_list_entry o > /* > * VFS namecache > */ > - { "ncglobal", &lock_class_rw }, > + { "ncvn", &lock_class_mtx_sleep }, > { "ncbuc", &lock_class_rw }, > { "vnode interlock", &lock_class_mtx_sleep }, > { "ncneg", &lock_class_mtx_sleep }, > > Modified: head/sys/kern/vfs_cache.c > == > --- head/sys/kern/vfs_cache.c Fri Sep 23 03:21:40 2016(r306223) > +++ head/sys/kern/vfs_cache.c Fri Sep 23 04:45:11 2016(r306224) > @@ -151,21 +151,35 @@ structnamecache_ts { > * name is located in the cache, it will be dropped. > * > * These locks are used (in the order in which they can be taken): > - * NAME TYPEROLE > - * cache_lock rwlock global, needed for all modifications > - * bucketlock rwlock for access to given hash bucket > - * ncneg_mtxmtx negative entry LRU management > + * NAMETYPEROLE > + * vnodelock mtx vnode lists and v_cache_dd field protection > + * bucketlock rwlock for access to given set of hash buckets > + * ncneg_mtx mtx negative entry LRU management > * > - * A name -> vnode lookup can be safely performed by either locking > cache_lock > - * or the relevant hash bucket. > + * Additionally, ncneg_shrink_lock mtx is used to have at most one thread > + * shrinking the LRU list. > * > - * ".." and vnode -> name lookups require cache_lock. > + * It is legal to take multiple vnodelock and bucketlock locks. The locking > + * order is lower address first. Both are recursive. > * > - * Modifications require both cache_lock and relevant bucketlock taken for > - * writing. > + * "." lookups are lockless. > * > - * Negative entry LRU management requires ncneg_mtx taken on top of either > - * cache_lock or bucketlock. > + * ".." and vnode -> name lookups require vnodelock. > + * > + * name -> vnode lookup requires the relevant bucketlock to be held for > reading. > + * > + * Insertions and removals of entries require involved vnodes and bucketlocks > + * to be write-locked to prevent other threads from seeing the entry. > + * > + * Some lookups result in removal of the found entry (e.g. getting rid of a > + * negative entry with the intent to create a positive one), which poses a > + * problem when multiple threads reach the state. Similarly, two different > + * threads can purge two different vnodes and try to remove the same name. > + * > + * If the already held vnode lock is lower than the second required lock, we > + * can just take the other lock. However, in the opposite case, this could > + * deadlock. As such, this is resolved by trylocking and if that fails > unlocking > + * the first node, locking everything in order and revalidating the state. > */ > > /* > @@ -196,15 +210,9 @@ SYSCTL_UINT(_vfs, OID_AUTO, ncsizefactor > > struct nchstatsnchstats; /* cache effectiveness > statistics */ > > -static struct rwlock cache_lock; > -RW_SYSINIT(vfscache, &cache_lock, "ncglobal"); > - > -#defineCACHE_TRY_WLOCK() rw_try_wlock(&cache_lock) > -#defineCACHE_UPGRADE_LOCK()rw_try_upgrade(&cache_lock) > -#defineCACHE_RLOCK() rw_rlock(&cache_lock) > -#defineCACHE_RUNLOCK() rw_runlock(&cache_lock) > -#defineCACHE_WLOCK() rw_wlock(&cache_lock) > -#defineCACHE_WUNLOCK() rw_wunlock(&cache_lock) > +static struct mtx ncneg_shrink_lock; > +MTX_SYSINIT(vfscache_shrink_neg, &ncneg_shrink_lock, "Name Cache shrink neg", > +MTX_DEF); > > static struct mtx_padalign ncneg_mtx; > MTX_SYSINIT(vfscache_neg, &ncneg_mtx, "ncneg", MTX_DEF); > @@ -214,6 +222,19 @@ static struct rwlock_padalign *bucketlo > #defineHASH2BUCKETLOCK(hash) \ > ((struct rwlock *)(&bucketl
svn commit: r306224 - head/sys/kern
Author: mjg Date: Fri Sep 23 04:45:11 2016 New Revision: 306224 URL: https://svnweb.freebsd.org/changeset/base/306224 Log: cache: get rid of the global lock Add a table of vnode locks and use them along with bucketlocks to provide concurrent modification support. The approach taken is to preserve the current behaviour of the namecache and just lock all relevant parts before any changes are made. Lookups still require the relevant bucket to be locked. Discussed with: kib Tested by:pho Modified: head/sys/kern/subr_witness.c head/sys/kern/vfs_cache.c Modified: head/sys/kern/subr_witness.c == --- head/sys/kern/subr_witness.cFri Sep 23 03:21:40 2016 (r306223) +++ head/sys/kern/subr_witness.cFri Sep 23 04:45:11 2016 (r306224) @@ -625,7 +625,7 @@ static struct witness_order_list_entry o /* * VFS namecache */ - { "ncglobal", &lock_class_rw }, + { "ncvn", &lock_class_mtx_sleep }, { "ncbuc", &lock_class_rw }, { "vnode interlock", &lock_class_mtx_sleep }, { "ncneg", &lock_class_mtx_sleep }, Modified: head/sys/kern/vfs_cache.c == --- head/sys/kern/vfs_cache.c Fri Sep 23 03:21:40 2016(r306223) +++ head/sys/kern/vfs_cache.c Fri Sep 23 04:45:11 2016(r306224) @@ -151,21 +151,35 @@ structnamecache_ts { * name is located in the cache, it will be dropped. * * These locks are used (in the order in which they can be taken): - * NAME TYPEROLE - * cache_lock rwlock global, needed for all modifications - * bucketlock rwlock for access to given hash bucket - * ncneg_mtxmtx negative entry LRU management + * NAMETYPEROLE + * vnodelock mtx vnode lists and v_cache_dd field protection + * bucketlock rwlock for access to given set of hash buckets + * ncneg_mtx mtx negative entry LRU management * - * A name -> vnode lookup can be safely performed by either locking cache_lock - * or the relevant hash bucket. + * Additionally, ncneg_shrink_lock mtx is used to have at most one thread + * shrinking the LRU list. * - * ".." and vnode -> name lookups require cache_lock. + * It is legal to take multiple vnodelock and bucketlock locks. The locking + * order is lower address first. Both are recursive. * - * Modifications require both cache_lock and relevant bucketlock taken for - * writing. + * "." lookups are lockless. * - * Negative entry LRU management requires ncneg_mtx taken on top of either - * cache_lock or bucketlock. + * ".." and vnode -> name lookups require vnodelock. + * + * name -> vnode lookup requires the relevant bucketlock to be held for reading. + * + * Insertions and removals of entries require involved vnodes and bucketlocks + * to be write-locked to prevent other threads from seeing the entry. + * + * Some lookups result in removal of the found entry (e.g. getting rid of a + * negative entry with the intent to create a positive one), which poses a + * problem when multiple threads reach the state. Similarly, two different + * threads can purge two different vnodes and try to remove the same name. + * + * If the already held vnode lock is lower than the second required lock, we + * can just take the other lock. However, in the opposite case, this could + * deadlock. As such, this is resolved by trylocking and if that fails unlocking + * the first node, locking everything in order and revalidating the state. */ /* @@ -196,15 +210,9 @@ SYSCTL_UINT(_vfs, OID_AUTO, ncsizefactor struct nchstatsnchstats; /* cache effectiveness statistics */ -static struct rwlock cache_lock; -RW_SYSINIT(vfscache, &cache_lock, "ncglobal"); - -#defineCACHE_TRY_WLOCK() rw_try_wlock(&cache_lock) -#defineCACHE_UPGRADE_LOCK()rw_try_upgrade(&cache_lock) -#defineCACHE_RLOCK() rw_rlock(&cache_lock) -#defineCACHE_RUNLOCK() rw_runlock(&cache_lock) -#defineCACHE_WLOCK() rw_wlock(&cache_lock) -#defineCACHE_WUNLOCK() rw_wunlock(&cache_lock) +static struct mtx ncneg_shrink_lock; +MTX_SYSINIT(vfscache_shrink_neg, &ncneg_shrink_lock, "Name Cache shrink neg", +MTX_DEF); static struct mtx_padalign ncneg_mtx; MTX_SYSINIT(vfscache_neg, &ncneg_mtx, "ncneg", MTX_DEF); @@ -214,6 +222,19 @@ static struct rwlock_padalign *bucketlo #defineHASH2BUCKETLOCK(hash) \ ((struct rwlock *)(&bucketlocks[((hash) % numbucketlocks)])) +static u_int numvnodelocks; +static struct mtx *vnodelocks; +static inline struct mtx * +VP2VNODELOCK(struct vnode *vp) +{ + struct mtx *vlp; + + if (vp == NULL) + return (NULL); + vlp = &vnodelocks[(((uintptr_t)(vp) >> 8) % numvnodelocks)]; + return (vlp); +} + /* * U