Changeset 3362 in kBuild

Timestamp:

Jun 8, 2020 7:28:44 PM (5 years ago)

Author:

bird

Message:

kFsCache: Don't need to lock the whole cache when using kFsCacheObjAddUserData and kFsCacheObjGetUserData. Fixed incorrect debug hack checking that we don't release objects while they are in the path hash structures.

Location:

trunk/src/lib/nt

Files:

• kFsCache.c (modified) (10 diffs)
• kFsCache.h (modified) (5 diffs)

trunk/src/lib/nt/kFsCache.c

@@ -429 +429 @@
                                     ? pCache->auGenerations[       pFsObj->fFlags & KFSOBJ_F_USE_CUSTOM_GEN]
                                     : pCache->auGenerationsMissing[pFsObj->fFlags & KFSOBJ_F_USE_CUSTOM_GEN];
-            pFsObj->abUnused[0] += 1; // for debugging
+            pFsObj->cPathHashRefs += 1; // for debugging
         }
         else
@@ -486 +486 @@
                                     ? pCache->auGenerations[       pFsObj->fFlags & KFSOBJ_F_USE_CUSTOM_GEN]
                                     : pCache->auGenerationsMissing[pFsObj->fFlags & KFSOBJ_F_USE_CUSTOM_GEN];
-            pFsObj->abUnused[0] += 1; // for debugging
+            pFsObj->cPathHashRefs += 1; // for debugging
         }
         else
@@ -593 +593 @@
         pObj->bObjType      = bObjType;
         pObj->fHaveStats    = K_FALSE;
-        pObj->abUnused[0]   = K_FALSE;
-        pObj->abUnused[1]   = K_FALSE;
+        pObj->cPathHashRefs = 0;
+        pObj->idxUserDataLock = KU8_MAX;
         pObj->fFlags        = pParent->Obj.fFlags & KFSOBJ_F_INHERITED_MASK;
         pObj->pParent       = pParent;
@@ -3386 +3386 @@
             else
             {
+                pHashEntry->pFsObj->cPathHashRefs -= 1;
                 kFsCacheObjRelease(pCache, pHashEntry->pFsObj);
                 if (pHashEntry->fAbsolute)
@@ -3447 +3448 @@
             else
             {
+                pHashEntry->pFsObj->cPathHashRefs -= 1;
                 kFsCacheObjRelease(pCache, pHashEntry->pFsObj);
                 if (pHashEntry->fAbsolute)
@@ -3900 +3902 @@
     KFSCACHE_LOG(("Destroying %s/%s, type=%d, pObj=%p, pszWhere=%s\n",
                   pObj->pParent ? pObj->pParent->Obj.pszName : "", pObj->pszName, pObj->bObjType, pObj, pszWhere));
-    if (pObj->abUnused[1] != 0)
+    if (pObj->cPathHashRefs != 0)
     {
         fprintf(stderr, "Destroying %s/%s, type=%d, path hash entries: %d!\n", pObj->pParent ? pObj->pParent->Obj.pszName : "",
-                pObj->pszName, pObj->bObjType, pObj->abUnused[0]);
+                pObj->pszName, pObj->bObjType, pObj->cPathHashRefs);
         fflush(stderr);
         __debugbreak();
@@ -4080 +4082 @@
 {
     kHlpAssert(cbUserData >= sizeof(*pNew));
-    KFSCACHE_LOCK(pCache);
+    KFSCACHE_OBJUSERDATA_LOCK(pCache, pObj);
 
     if (kFsCacheObjGetUserData(pCache, pObj, uKey) == NULL)
@@ -4091 +4093 @@
             pNew->pNext         = pObj->pUserDataHead;
             pObj->pUserDataHead = pNew;
-            KFSCACHE_UNLOCK(pCache);
+            KFSCACHE_OBJUSERDATA_UNLOCK(pCache, pObj);
             return pNew;
         }
     }
 
-    KFSCACHE_UNLOCK(pCache);
+    KFSCACHE_OBJUSERDATA_UNLOCK(pCache, pObj);
     return NULL;
 }
@@ -4115 +4117 @@
     kHlpAssert(pCache->u32Magic == KFSCACHE_MAGIC);
     kHlpAssert(pObj->u32Magic == KFSOBJ_MAGIC);
-    KFSCACHE_LOCK(pCache);
+    KFSCACHE_OBJUSERDATA_LOCK(pCache, pObj);
 
     for (pCur = pObj->pUserDataHead; pCur; pCur = pCur->pNext)
         if (pCur->uKey == uKey)
         {
-            KFSCACHE_UNLOCK(pCache);
+            KFSCACHE_OBJUSERDATA_UNLOCK(pCache, pObj);
             return pCur;
         }
 
-    KFSCACHE_UNLOCK(pCache);
+    KFSCACHE_OBJUSERDATA_UNLOCK(pCache, pObj);
     return NULL;
 }
 
+
+/**
+ * Determins the idxUserDataLock value.
+ *
+ * Called by KFSCACHE_OBJUSERDATA_LOCK when idxUserDataLock is set to KU8_MAX.
+ *
+ * @returns The proper idxUserDataLock value.
+ * @param   pCache              The cache.
+ * @param   pObj                The object.
+ */
+KU8 kFsCacheObjGetUserDataLockIndex(PKFSCACHE pCache, PKFSOBJ pObj)
+{
+    KU8 idxUserDataLock = pObj->idxUserDataLock;
+    if (idxUserDataLock == KU8_MAX)
+    {
+        KFSCACHE_LOCK(pCache);
+        idxUserDataLock = pObj->idxUserDataLock;
+        if (idxUserDataLock == KU8_MAX)
+        {
+            idxUserDataLock = pCache->idxUserDataNext++;
+            idxUserDataLock %= K_ELEMENTS(pCache->auUserDataLocks);
+            pObj->idxUserDataLock = idxUserDataLock;
+        }
+        KFSCACHE_UNLOCK(pCache);
+    }
+    return idxUserDataLock;
+}
 
 /**
@@ -4766 +4795 @@
 
 #ifdef KFSCACHE_CFG_LOCKING
-            InitializeCriticalSection(&pCache->u.CritSect);
+            {
+                KSIZE idx = K_ELEMENTS(pCache->auUserDataLocks);
+                while (idx-- > 0)
+                    InitializeCriticalSection(&pCache->auUserDataLocks[idx].CritSect);
+                InitializeCriticalSection(&pCache->u.CritSect);
+            }
 #endif
             return pCache;

trunk/src/lib/nt/kFsCache.h

@@ -161 +161 @@
     /** Set if the Stats member is valid, clear if not. */
     KBOOL               fHaveStats;
-    /** Unused flags. */
-    KBOOL               abUnused[2];
+    /** Internal debug field for counting path hash references.
+     * @internal  */
+    KU8                 cPathHashRefs;
+    /** Index into KFSCACHE::auUserData. */
+    KU8                 idxUserDataLock;
     /** Flags, KFSOBJ_F_XXX. */
     KU32                fFlags;
@@ -381 +384 @@
 /** @def KFSCACHE_UNLOCK
  *  Counterpart to KFSCACHE_LOCK. */
+/** @def KFSCACHE_OBJUSERDATA_LOCK
+ *  Locks the user data list of an object exclusively. */
+/** @def KFSCACHE_OBJUSERDATA_UNLOCK
+ *  Counterpart to KFSCACHE_OBJUSERDATA_LOCK. */
 #ifdef KFSCACHE_CFG_LOCKING
 # define KFSCACHE_LOCK(a_pCache)        EnterCriticalSection(&(a_pCache)->u.CritSect)
 # define KFSCACHE_UNLOCK(a_pCache)      LeaveCriticalSection(&(a_pCache)->u.CritSect)
+# define KFSCACHE_OBJUSERDATA_LOCK(a_pCache, a_pObj) do { \
+        KU8 idxUserDataLock = (a_pObj)->idxUserDataLock; \
+        if (idxUserDataLock != KU8_MAX) \
+        { /* likely */ } \
+        else \
+            idxUserDataLock = kFsCacheObjGetUserDataLockIndex(a_pCache, a_pObj); \
+        idxUserDataLock &= (KU8)(K_ELEMENTS((a_pCache)->auUserDataLocks) - 1); \
+        EnterCriticalSection(&(a_pCache)->auUserDataLocks[idxUserDataLock].CritSect); \
+    } while (0)
+# define KFSCACHE_OBJUSERDATA_UNLOCK(a_pCache, a_pObj) \
+    LeaveCriticalSection(&(a_pCache)->auUserDataLocks[(a_pObj)->idxUserDataLock & (K_ELEMENTS((a_pCache)->auUserDataLocks) - 1)].CritSect)
 #else
 # define KFSCACHE_LOCK(a_pCache)        do { } while (0)
 # define KFSCACHE_UNLOCK(a_pCache)      do { } while (0)
+# define KFSCACHE_OBJUSERDATA_LOCK(a_pCache, a_pObj)    do { } while (0)
+# define KFSCACHE_OBJUSERDATA_UNLOCK(a_pCache, a_pObj)  do { } while (0)
 #endif
 
@@ -456 +476 @@
 
 #ifdef KFSCACHE_CFG_LOCKING
-    /** Critical section protecting the cache. */
     union
     {
@@ -463 +482 @@
 # endif
         KU64                abPadding[2 * 4 + 4 * sizeof(void *)];
-    } u;
+    }
+    /** Critical section protecting the cache. */
+                        u,
+    /** Critical section protecting the pUserDataHead of objects.
+     * @note Array size must be a power of two. */
+                        auUserDataLocks[8];
+    /** The next auUserData index. */
+    KU8                 idxUserDataNext;
 #endif
 
@@ -544 +570 @@
 PKFSUSERDATA kFsCacheObjAddUserData(PKFSCACHE pCache, PKFSOBJ pObj, KUPTR uKey, KSIZE cbUserData);
 PKFSUSERDATA kFsCacheObjGetUserData(PKFSCACHE pCache, PKFSOBJ pObj, KUPTR uKey);
+KU8         kFsCacheObjGetUserDataLockIndex(PKFSCACHE pCache, PKFSOBJ pObj);
 KBOOL       kFsCacheObjGetFullPathA(PKFSOBJ pObj, char *pszPath, KSIZE cbPath, char chSlash);
 KBOOL       kFsCacheObjGetFullPathW(PKFSOBJ pObj, wchar_t *pwszPath, KSIZE cwcPath, wchar_t wcSlash);