Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Back out previous temporarily - seeing unusual lookup fa...



details:   https://anonhg.NetBSD.org/src/rev/fa2c619e8258
branches:  trunk
changeset: 846798:fa2c619e8258
user:      ad <ad%NetBSD.org@localhost>
date:      Sun Dec 01 18:31:19 2019 +0000

description:
Back out previous temporarily - seeing unusual lookup failures.  Will
come back to it.

diffstat:

 sys/kern/vfs_cache.c |  317 ++++++++++++++++++++++----------------------------
 sys/sys/namei.src    |    9 +-
 2 files changed, 141 insertions(+), 185 deletions(-)

diffs (truncated from 787 to 300 lines):

diff -r a809d78fa209 -r fa2c619e8258 sys/kern/vfs_cache.c
--- a/sys/kern/vfs_cache.c      Sun Dec 01 18:29:26 2019 +0000
+++ b/sys/kern/vfs_cache.c      Sun Dec 01 18:31:19 2019 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: vfs_cache.c,v 1.124 2019/12/01 13:39:53 ad Exp $       */
+/*     $NetBSD: vfs_cache.c,v 1.125 2019/12/01 18:31:19 ad Exp $       */
 
 /*-
- * Copyright (c) 2008, 2019 The NetBSD Foundation, Inc.
+ * Copyright (c) 2008 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.124 2019/12/01 13:39:53 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.125 2019/12/01 18:31:19 ad Exp $");
 
 #define __NAMECACHE_PRIVATE
 #ifdef _KERNEL_OPT
@@ -130,7 +130,7 @@
  * - Invalidate: active--->queued
  *
  *   Done by cache_invalidate.  If not already invalidated, nullify
- *   ncp->nc_dvp and ncp->nc_vp, and add to namecache_gc_queue.  Called,
+ *   ncp->nc_dvp and ncp->nc_vp, and add to cache_gcqueue.  Called,
  *   among various other places, in cache_lookup(dvp, name, namelen,
  *   nameiop, cnflags, &iswht, &vp) when MAKEENTRY is missing from
  *   cnflags.
@@ -145,17 +145,16 @@
  * Locking.
  *
  * L namecache_lock            Global lock for namecache table and queues.
- * G namecache_gc_lock         Global lock for garbage collection.
  * C struct nchcpu::cpu_lock   Per-CPU lock to reduce read contention.
- * N struct namecache::nc_lock Per-entry lock, matching nc_vp->v_interlock.
- *                             If nc_vp==NULL, lock is private / not shared.
+ * N struct namecache::nc_lock Per-entry lock.
+ * V struct vnode::v_interlock Vnode interlock.
  *
- * Lock order: L -> C -> N
+ * Lock order: L -> C -> N -> V
  *
  *     Examples:
  *     . L->C: cache_reclaim
- *     . C->N: cache_lookup
- *     . L->N: cache_purge1, cache_revlookup
+ *     . C->N->V: cache_lookup
+ *     . L->N->V: cache_purge1, cache_revlookup
  *
  * All use serialized by namecache_lock:
  *
@@ -168,9 +167,8 @@
  * - Insertion serialized by namecache_lock,
  * - read protected by per-CPU lock,
  * - insert/read ordering guaranteed by memory barriers, and
- * - deletion allowed only under namecache_lock, with namecache_gc_lock
- *   taken to chop out the garbage collection list, and *all* per-CPU locks
- *   observed as "unowned" at least once:
+ * - deletion allowed only under namecache_lock and *all* per-CPU locks
+ *   in CPU_INFO_FOREACH order:
  *
  *     nchashtbl / struct namecache::nc_hash
  *
@@ -182,13 +180,11 @@
  *
  *     struct namecache::nc_dvp
  *     struct namecache::nc_vp
- *     struct namecache::nc_hittime (*)
- *
- * All use serialized by struct namecache_gc_lock:
+ *     struct namecache::nc_gcqueue (*)
+ *     struct namecache::nc_hittime (**)
  *
- *     struct namecache::nc_gclist
- *
- * (*) cache_prune reads nc_hittime unlocked, since approximate is OK.
+ * (*) Once on the queue, only cache_thread uses this nc_gcqueue, unlocked.
+ * (**) cache_prune reads nc_hittime unlocked, since approximate is OK.
  *
  * Unlocked because stable after initialization:
  *
@@ -261,7 +257,7 @@
  * Structures associated with name cacheing.
  */
 
-static kmutex_t namecache_lock __cacheline_aligned;
+static kmutex_t *namecache_lock __read_mostly;
 static pool_cache_t namecache_cache __read_mostly;
 static TAILQ_HEAD(, namecache) nclruhead __cacheline_aligned;
 
@@ -280,9 +276,8 @@
 static long    numcache __cacheline_aligned;
 
 /* Garbage collection queue and number of entries pending in it. */
-static kmutex_t namecache_gc_lock __cacheline_aligned;
-static SLIST_HEAD(namecache_gc_queue, namecache) namecache_gc_queue;
-static u_int   namecache_gc_pend;
+static void    *cache_gcqueue;
+static u_int   cache_gcpend;
 
 /* Cache effectiveness statistics.  This holds total from per-cpu stats */
 struct nchstats        nchstats __cacheline_aligned;
@@ -292,6 +287,8 @@
  * values and add current per-cpu increments to the subsystem total
  * last collected by cache_reclaim().
  */
+#define        CACHE_STATS_CURRENT     /* nothing */
+
 #define        COUNT(cpup, f)  ((cpup)->cpu_stats.f++)
 
 #define        UPDATE(cpup, f) do { \
@@ -301,10 +298,15 @@
        Xcpup->cpu_stats_last.f = Xcnt; \
 } while (/* CONSTCOND */ 0)
 
+#define        ADD(stats, cpup, f) do { \
+       struct nchcpu *Xcpup = (cpup); \
+       stats.f += Xcpup->cpu_stats.f - Xcpup->cpu_stats_last.f; \
+} while (/* CONSTCOND */ 0)
+
 /* Do unlocked stats the same way. Use a different name to allow mind changes */
 #define        COUNT_UNL(cpup, f)      COUNT((cpup), f)
 
-static const int cache_lowat = 97;
+static const int cache_lowat = 95;
 static const int cache_hiwat = 98;
 static const int cache_hottime = 5;    /* number of seconds */
 static int doingcache = 1;             /* 1 => enable the cache */
@@ -367,12 +369,15 @@
 
 /*
  * Invalidate a cache entry and enqueue it for garbage collection.
+ * The caller needs to hold namecache_lock or a per-cpu lock to hold
+ * off cache_reclaim().
  */
 static void
 cache_invalidate(struct namecache *ncp)
 {
+       void *head;
 
-       KASSERT(mutex_owned(ncp->nc_lock));
+       KASSERT(mutex_owned(&ncp->nc_lock));
 
        if (ncp->nc_dvp != NULL) {
                SDT_PROBE(vfs, namecache, invalidate, done, ncp->nc_dvp,
@@ -380,10 +385,11 @@
 
                ncp->nc_vp = NULL;
                ncp->nc_dvp = NULL;
-               mutex_enter(&namecache_gc_lock);
-               SLIST_INSERT_HEAD(&namecache_gc_queue, ncp, nc_gclist);
-               namecache_gc_pend++;
-               mutex_exit(&namecache_gc_lock);
+               do {
+                       head = cache_gcqueue;
+                       ncp->nc_gcqueue = head;
+               } while (atomic_cas_ptr(&cache_gcqueue, head, ncp) != head);
+               atomic_inc_uint(&cache_gcpend);
        }
 }
 
@@ -395,7 +401,7 @@
 cache_disassociate(struct namecache *ncp)
 {
 
-       KASSERT(mutex_owned(&namecache_lock));
+       KASSERT(mutex_owned(namecache_lock));
        KASSERT(ncp->nc_dvp == NULL);
 
        if (ncp->nc_lru.tqe_prev != NULL) {
@@ -418,8 +424,7 @@
 
 /*
  * Lock all CPUs to prevent any cache lookup activity.  Conceptually,
- * this locks out all "readers".  This is a very heavyweight operation
- * that we only use for nchreinit().
+ * this locks out all "readers".
  */
 static void
 cache_lock_cpus(void)
@@ -428,9 +433,6 @@
        struct cpu_info *ci;
        struct nchcpu *cpup;
 
-       /* Not necessary but don't want more than one LWP trying this. */
-       KASSERT(mutex_owned(&namecache_lock));
-
        /*
         * Lock out all CPUs first, then harvest per-cpu stats.  This
         * is probably not quite as cache-efficient as doing the lock
@@ -483,7 +485,6 @@
        struct nchashhead *ncpp;
        struct namecache *ncp;
        nchash_t hash;
-       int ticks;
 
        KASSERT(dvp != NULL);
        hash = cache_hash(name, namelen);
@@ -495,22 +496,15 @@
                    ncp->nc_nlen != namelen ||
                    memcmp(ncp->nc_name, name, (u_int)ncp->nc_nlen))
                        continue;
-               mutex_enter(ncp->nc_lock);
+               mutex_enter(&ncp->nc_lock);
                if (__predict_true(ncp->nc_dvp == dvp)) {
-                       ticks = hardclock_ticks;
-                       if (ncp->nc_hittime != ticks) {
-                               /*
-                                * Avoid false sharing on MP: do not store
-                                * to *ncp unless the value changed.
-                                */
-                               ncp->nc_hittime = ticks;
-                       }
+                       ncp->nc_hittime = hardclock_ticks;
                        SDT_PROBE(vfs, namecache, lookup, hit, dvp,
                            name, namelen, 0, 0);
                        return ncp;
                }
                /* Raced: entry has been nullified. */
-               mutex_exit(ncp->nc_lock);
+               mutex_exit(&ncp->nc_lock);
        }
 
        SDT_PROBE(vfs, namecache, lookup, miss, dvp,
@@ -579,6 +573,7 @@
        int error;
        bool hit;
 
+
        /* Establish default result values */
        if (iswht_ret != NULL) {
                *iswht_ret = 0;
@@ -615,13 +610,12 @@
                 * want cache entry to exist.
                 */
                cache_invalidate(ncp);
-               mutex_exit(ncp->nc_lock);
+               mutex_exit(&ncp->nc_lock);
                mutex_exit(&cpup->cpu_lock);
                /* found nothing */
                return false;
        }
-       vp = ncp->nc_vp;
-       if (__predict_false(vp == NULL)) {
+       if (ncp->nc_vp == NULL) {
                if (iswht_ret != NULL) {
                        /*
                         * Restore the ISWHITEOUT flag saved earlier.
@@ -648,11 +642,14 @@
                        /* found nothing */
                        hit = false;
                }
-               mutex_exit(ncp->nc_lock);
+               mutex_exit(&ncp->nc_lock);
                mutex_exit(&cpup->cpu_lock);
                return hit;
        }
-       KASSERT(vp->v_interlock == ncp->nc_lock);
+
+       vp = ncp->nc_vp;
+       mutex_enter(vp->v_interlock);
+       mutex_exit(&ncp->nc_lock);
        mutex_exit(&cpup->cpu_lock);
 
        /*
@@ -727,12 +724,13 @@
                        *iswht_ret = (ncp->nc_flags & ISWHITEOUT) != 0;
                }
                COUNT(cpup, ncs_neghits);
-               mutex_exit(ncp->nc_lock);
+               mutex_exit(&ncp->nc_lock);
                mutex_exit(&cpup->cpu_lock);
                /* found negative entry; vn is already null from above */
                return true;
        }
-       KASSERT(vp->v_interlock == ncp->nc_lock);
+       mutex_enter(vp->v_interlock);
+       mutex_exit(&ncp->nc_lock);
        mutex_exit(&cpup->cpu_lock);
 
        /*
@@ -777,7 +775,6 @@
        struct nchcpu *cpup;
        char *bp;
        int error, nlen;
-       bool locked, again;
 
        if (!doingcache)
                goto out;
@@ -790,12 +787,10 @@
         * is the only place these counters are incremented so no one
         * will be racing with us to increment them.
         */
-       again = false;
- retry:
        cpup = curcpu()->ci_data.cpu_nch;
-       mutex_enter(&namecache_lock);



Home | Main Index | Thread Index | Old Index