Windows: Permit cm_scache rwlock to be dropped when "Stablized"

The cm_buf_opts_t cm_BufStabilize() function was implemented
such that holding the cm_scache_t.rw lock had to be exclusively
held until cm_BufUnstablize() was called.  Unfortunately, this
prevents using Stabilize/Unstabilize to protect the cm_scache_t
during Flush operations as the cm_scache_t.rw lock must be
acquired after the cm_buf_t mutex and not before it.

This patchset reimplements the synchronization logic using
the new CM_SCACHEFLAG_SIZESETTING flag and cm_SyncOp().

LICENSE MIT

Change-Id: Iaada83f7f3b75bb3b213b33b2399e900e48a2fbc
Reviewed-on: http://gerrit.openafs.org/2661
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>

diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c
index ba311d5eda4d814b93469a40404b825c1a6257ec..2821e48030931d9d59cf2cc796a31fffe073a029 100644 (file)
--- a/src/WINNT/afsd/cm_dcache.c
+++ b/src/WINNT/afsd/cm_dcache.c
@@ -463,8 +463,16 @@ long cm_BufRead(cm_buf_t *bufp, long nbytes, long *bytesReadp, cm_user_t *userp)
     return 0;
 }
 
-/* stabilize scache entry, and return with it locked so 
- * it stays stable.
+/*
+ * stabilize scache entry with CM_SCACHESYNC_SETSIZE.  This prevents any new
+ * data buffers to be allocated, new data to be fetched from the file server,
+ * and writes to be accepted from the application but permits dirty buffers
+ * to be written to the file server.
+ *
+ * Stabilize uses cm_SyncOp to maintain the cm_scache_t in this stable state
+ * instead of holding the rwlock exclusively.  This permits background stores
+ * to be performed in parallel and in particular allow FlushFile to be
+ * implemented without violating the locking hierarchy.
  */
 long cm_BufStabilize(void *vscp, cm_user_t *userp, cm_req_t *reqp)
 {
@@ -474,12 +482,9 @@ long cm_BufStabilize(void *vscp, cm_user_t *userp, cm_req_t *reqp)
     lock_ObtainWrite(&scp->rw);
     code = cm_SyncOp(scp, NULL, userp, reqp, 0, 
                      CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS | CM_SCACHESYNC_SETSIZE);
-    if (code) {
-        lock_ReleaseWrite(&scp->rw);
-        return code;
-    }
-        
-    return 0;
+    lock_ReleaseWrite(&scp->rw);
+
+    return code;
 }
 
 /* undoes the work that cm_BufStabilize does: releases lock so things can change again */
@@ -487,6 +492,7 @@ long cm_BufUnstabilize(void *vscp, cm_user_t *userp)
 {
     cm_scache_t *scp = vscp;
         
+    lock_ObtainWrite(&scp->rw);
     cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS | CM_SCACHESYNC_SETSIZE);
 
     lock_ReleaseWrite(&scp->rw);

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index 75ba858250a450d6e2b48f038dd3828c3857ec71..b2ac1222d20f3d13779ddd1795ebbbcfeb7361b6 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -1110,9 +1110,9 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
              * operation ran first, or even which order a read and
              * a write occurred in.
              */
-            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING
-                               | CM_SCACHEFLAG_SIZESTORING | CM_SCACHEFLAG_GETCALLBACK)) {
-                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESTORING|GETCALLBACK want FETCHSTATUS", scp);
+            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING | CM_SCACHEFLAG_SIZESETTING |
+                              CM_SCACHEFLAG_SIZESTORING | CM_SCACHEFLAG_GETCALLBACK)) {
+                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESETTING|SIZESTORING|GETCALLBACK want FETCHSTATUS", scp);
                 goto sleep;
             }
         }
@@ -1121,9 +1121,9 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
             /* if we're going to make an RPC to change the status, make sure
              * that no one is bringing in or sending out the status.
              */
-            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING |
+            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING | CM_SCACHEFLAG_SIZESETTING |
                               CM_SCACHEFLAG_SIZESTORING | CM_SCACHEFLAG_GETCALLBACK)) {
-                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESTORING|GETCALLBACK want STORESIZE|STORESTATUS|SETSIZE|GETCALLBACK", scp);
+                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESETTING|SIZESTORING|GETCALLBACK want STORESIZE|STORESTATUS|SETSIZE|GETCALLBACK", scp);
                 goto sleep;
             }
             if ((!bufp || bufp && scp->fileType == CM_SCACHETYPE_FILE) &&
@@ -1137,9 +1137,9 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
              * nothing is happening to that chunk, and that we aren't
              * changing the basic file status info, either.
              */
-            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING
-                               | CM_SCACHEFLAG_SIZESTORING | CM_SCACHEFLAG_GETCALLBACK)) {
-                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESTORING|GETCALLBACK want FETCHDATA", scp);
+            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING | CM_SCACHEFLAG_SIZESETTING |
+                              CM_SCACHEFLAG_SIZESTORING | CM_SCACHEFLAG_GETCALLBACK)) {
+                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESETTING|SIZESTORING|GETCALLBACK want FETCHDATA", scp);
                 goto sleep;
             }
             if (bufp && (bufp->cmFlags & (CM_BUF_CMFETCHING | CM_BUF_CMSTORING | CM_BUF_CMWRITING))) {
@@ -1203,8 +1203,9 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
              * operations don't change any of the data that we're
              * changing here.
              */
-            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING | CM_SCACHEFLAG_SIZESTORING)) {
-                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESTORING want SETSTATUS", scp);
+            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING |
+                              CM_SCACHEFLAG_SIZESETTING | CM_SCACHEFLAG_SIZESTORING)) {
+                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESETTING|SIZESTORING want SETSTATUS", scp);
                 goto sleep;
             }
         }
@@ -1230,9 +1231,9 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
             /* don't write unless the status is stable and the chunk
              * is stable.
              */
-            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING
-                               | CM_SCACHEFLAG_SIZESTORING)) {
-                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESTORING want WRITE", scp);
+            if (scp->flags & (CM_SCACHEFLAG_FETCHING | CM_SCACHEFLAG_STORING | CM_SCACHEFLAG_SIZESETTING |
+                              CM_SCACHEFLAG_SIZESTORING)) {
+                osi_Log1(afsd_logp, "CM SyncOp scp 0x%p is FETCHING|STORING|SIZESETTING|SIZESTORING want WRITE", scp);
                 goto sleep;
             }
             if (bufp && (bufp->cmFlags & (CM_BUF_CMFETCHING |
@@ -1349,6 +1350,8 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
         scp->flags |= CM_SCACHEFLAG_FETCHING;
     if (flags & CM_SCACHESYNC_STORESTATUS)
         scp->flags |= CM_SCACHEFLAG_STORING;
+    if (flags & CM_SCACHESYNC_SETSIZE)
+        scp->flags |= CM_SCACHEFLAG_SIZESETTING;
     if (flags & CM_SCACHESYNC_STORESIZE)
         scp->flags |= CM_SCACHEFLAG_SIZESTORING;
     if (flags & CM_SCACHESYNC_GETCALLBACK)
@@ -1418,6 +1421,8 @@ void cm_SyncOpDone(cm_scache_t *scp, cm_buf_t *bufp, afs_uint32 flags)
         scp->flags &= ~CM_SCACHEFLAG_FETCHING;
     if (flags & CM_SCACHESYNC_STORESTATUS)
         scp->flags &= ~CM_SCACHEFLAG_STORING;
+    if (flags & CM_SCACHESYNC_SETSIZE)
+        scp->flags &= ~CM_SCACHEFLAG_SIZESETTING;
     if (flags & CM_SCACHESYNC_STORESIZE)
         scp->flags &= ~CM_SCACHEFLAG_SIZESTORING;
     if (flags & CM_SCACHESYNC_GETCALLBACK)

diff --git a/src/WINNT/afsd/cm_scache.h b/src/WINNT/afsd/cm_scache.h
index b54c2f9cbbb231dba02243db9cee2275f1ac4156..5bfc6fa1b92f664957e42d992d958c9e5b883555 100644 (file)
--- a/src/WINNT/afsd/cm_scache.h
+++ b/src/WINNT/afsd/cm_scache.h
@@ -253,6 +253,7 @@ typedef struct cm_scache {
                                                 * this is a truncate op. */
 #define CM_SCACHEFLAG_INHASH           0x40    /* in the hash table */
 #define CM_SCACHEFLAG_BULKSTATTING     0x80    /* doing a bulk stat */
+#define CM_SCACHEFLAG_SIZESETTING       0x100   /* Stabilized; Truncate */
 #define CM_SCACHEFLAG_WAITING          0x200   /* waiting for fetch/store
                                                 * state to change */
 #define CM_SCACHEFLAG_PURERO           0x400   /* read-only (not even backup);