Windows: Modify signature of buf_CleanAsync and buf_CleanAsyncLocked
authorJeffrey Altman <jaltman@your-file-system.com>
Fri, 3 Sep 2010 01:17:24 +0000 (21:17 -0400)
committerJeffrey Altman <jaltman@openafs.org>
Mon, 6 Sep 2010 03:57:06 +0000 (20:57 -0700)
The buf_CleanAsync() and buf_CleanAsyncLocked() signature does
not include a cm_scache_t pointer even though buf_CleanAsyncLocked()
needs a pointer to the matching cm_scache_t object.  There are
some calls when the cm_scache_t object is already known.  For those
cases it is more efficient to avoid the additional lookup especially
when buf_CleanAsync*() is being called on every buffer associated
with the cm_scache_t object.

At the same time add a flags field and a constant
CM_BUF_WRITE_SCP_LOCKED to permit the lock state of the cm_scache_t
to be passed in.

Finally, fix up the usage in buf_FlushCleanPages() which gains
the most from these changes.

LICENSE MIT

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

src/WINNT/afsd/cm_buf.c
src/WINNT/afsd/cm_buf.h
src/WINNT/afsd/cm_dcache.c

index af7651b..5a30adc 100644 (file)
@@ -249,7 +249,7 @@ buf_Sync(int quitOnShutdown)
             case vl_unknown:
                 cm_InitReq(&req);
                 req.flags |= CM_REQ_NORETRY;
-                buf_CleanAsyncLocked(bp, &req, &dirty);
+                buf_CleanAsyncLocked(NULL, bp, &req, 0, &dirty);
                 wasDirty |= dirty;
             }
             cm_PutVolume(volp);
@@ -741,45 +741,45 @@ cm_buf_t *buf_FindAll(struct cm_scache *scp, osi_hyper_t *offsetp, afs_uint32 fl
  *
  * Returns non-zero if the buffer was dirty.
  */
-afs_uint32 buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp, afs_uint32 *pisdirty)
+afs_uint32 buf_CleanAsyncLocked(cm_scache_t *scp, cm_buf_t *bp, cm_req_t *reqp,
+                                afs_uint32 flags, afs_uint32 *pisdirty)
 {
     afs_uint32 code = 0;
     afs_uint32 isdirty = 0;
-    cm_scache_t * scp = NULL;
     osi_hyper_t offset;
+    int release_scp = 0;
 
     osi_assertx(bp->magic == CM_BUF_MAGIC, "invalid cm_buf_t magic");
 
+    if (scp = cm_FindSCache(&bp->fid))
+        release_scp = 1;
+
+    if (!scp) {
+        osi_Log1(buf_logp, "buf_CleanAsyncLocked unable to start I/O - scp not found buf 0x%p", bp);
+        code = CM_ERROR_NOSUCHFILE;
+    }
+
     while ((bp->flags & CM_BUF_DIRTY) == CM_BUF_DIRTY) {
-       isdirty = 1;
+        isdirty = 1;
         lock_ReleaseMutex(&bp->mx);
 
-       scp = cm_FindSCache(&bp->fid);
-       if (scp) {
-           osi_Log2(buf_logp, "buf_CleanAsyncLocked starts I/O on scp 0x%p buf 0x%p", scp, bp);
+        osi_Log2(buf_logp, "buf_CleanAsyncLocked starts I/O on scp 0x%p buf 0x%p", scp, bp);
 
-            offset = bp->offset;
-            LargeIntegerAdd(offset, ConvertLongToLargeInteger(bp->dirty_offset));
-           code = (*cm_buf_opsp->Writep)(scp, &offset, 
+        offset = bp->offset;
+        LargeIntegerAdd(offset, ConvertLongToLargeInteger(bp->dirty_offset));
+        code = (*cm_buf_opsp->Writep)(scp, &offset,
 #if 1
-                                           /* we might as well try to write all of the contiguous 
-                                            * dirty buffers in one RPC 
-                                            */
-                                           cm_chunkSize,
+                                       /* we might as well try to write all of the contiguous
+                                       * dirty buffers in one RPC
+                                       */
+                                       cm_chunkSize,
 #else
-                                          bp->dirty_length, 
+                                       bp->dirty_length,
 #endif
-                                          0, bp->userp, reqp);
-           osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
+                                       flags, bp->userp, reqp);
+        osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
 
-           cm_ReleaseSCache(scp);
-           scp = NULL;
-       } else {
-           osi_Log1(buf_logp, "buf_CleanAsyncLocked unable to start I/O - scp not found buf 0x%p", bp);
-           code = CM_ERROR_NOSUCHFILE;
-       }    
-        
-       lock_ObtainMutex(&bp->mx);
+        lock_ObtainMutex(&bp->mx);
        /* if the Write routine returns No Such File, clear the dirty flag
         * because we aren't going to be able to write this data to the file
         * server.
@@ -818,6 +818,9 @@ afs_uint32 buf_CleanAsyncLocked(cm_buf_t *bp, cm_req_t *reqp, afs_uint32 *pisdir
         }
     }
 
+    if (release_scp)
+        cm_ReleaseSCache(scp);
+
     /* if someone was waiting for the I/O that just completed or failed,
      * wake them up.
      */
@@ -1018,7 +1021,7 @@ long buf_GetNewLocked(struct cm_scache *scp, osi_hyper_t *offsetp, cm_req_t *req
                  * have the WRITING flag set, so we won't get
                  * back here.
                  */
-                buf_CleanAsync(bp, reqp, NULL);
+                buf_CleanAsync(scp, bp, reqp, 0, NULL);
 
                 /* now put it back and go around again */
                 buf_Release(bp);
@@ -1322,13 +1325,14 @@ long buf_CountFreeList(void)
 }
 
 /* clean a buffer synchronously */
-afs_uint32 buf_CleanAsync(cm_buf_t *bp, cm_req_t *reqp, afs_uint32 *pisdirty)
+afs_uint32 buf_CleanAsync(cm_scache_t *scp, cm_buf_t *bp, cm_req_t *reqp, afs_uint32 flags, afs_uint32 *pisdirty)
 {
     long code;
     osi_assertx(bp->magic == CM_BUF_MAGIC, "invalid cm_buf_t magic");
+    osi_assertx(!(flags & CM_BUF_WRITE_SCP_LOCKED), "scp->rw must not be held when calling buf_CleanAsync");
 
     lock_ObtainMutex(&bp->mx);
-    code = buf_CleanAsyncLocked(bp, reqp, pisdirty);
+    code = buf_CleanAsyncLocked(scp, bp, reqp, flags, pisdirty);
     lock_ReleaseMutex(&bp->mx);
 
     return code;
@@ -1459,7 +1463,7 @@ long buf_CleanAndReset(void)
                 cm_InitReq(&req);
                req.flags |= CM_REQ_NORETRY;
 
-               buf_CleanAsync(bp, &req, NULL);
+               buf_CleanAsync(NULL, bp, &req, 0, NULL);
                buf_CleanWait(NULL, bp, FALSE);
 
                 /* relock and release buffer */
@@ -1672,23 +1676,17 @@ long buf_FlushCleanPages(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp)
         /* clean buffer synchronously */
         if (cm_FidCmp(&bp->fid, &scp->fid) == 0) {
 
-            /*
-             * if the object is not located on a read/write volume
-             * we must stabilize the object to ensure that buffer
-             * changes cannot occur while the flush is performed.
-             */
-            if (!stable && code == 0 && !(scp->flags & CM_SCACHEFLAG_RO)) {
+            if (code == 0 && !stable && (bp->flags & CM_BUF_DIRTY)) {
+                /*
+                 * we must stabilize the object to ensure that buffer
+                 * changes cannot occur while the flush is performed.
+                 * However, we do not want to Stabilize if we do not
+                 * need to because Stabilize obtains a callback.
+                 */
                 code = (*cm_buf_opsp->Stabilizep)(scp, userp, reqp);
                 stable = (code == 0);
             }
 
-            lock_ObtainMutex(&bp->mx);
-
-            /* start cleaning the buffer, and wait for it to finish */
-            buf_CleanAsyncLocked(bp, reqp, NULL);
-            buf_WaitIO(scp, bp);
-            lock_ReleaseMutex(&bp->mx);
-
             if (code == CM_ERROR_BADFD) {
                 /* if the scp's FID is bad its because we received VNOVNODE 
                  * when attempting to FetchStatus before the write.  This
@@ -1703,8 +1701,18 @@ long buf_FlushCleanPages(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp)
                 bp->dataVersion = CM_BUF_VERSION_BAD;  /* known bad */
                 bp->dirtyCounter++;
                 lock_ReleaseMutex(&bp->mx);
-            } else if (!stable && code) {
-                goto skip;
+            } else if (!(scp->flags & CM_SCACHEFLAG_RO)) {
+                if (code) {
+                    goto skip;
+                }
+
+                lock_ObtainMutex(&bp->mx);
+
+                /* start cleaning the buffer, and wait for it to finish */
+                buf_CleanAsyncLocked(scp, bp, reqp, 0, NULL);
+                buf_WaitIO(scp, bp);
+
+                lock_ReleaseMutex(&bp->mx);
             }
 
             /* actually, we only know that buffer is clean if ref
@@ -1834,7 +1842,7 @@ long buf_CleanVnode(struct cm_scache *scp, cm_user_t *userp, cm_req_t *reqp)
                      */
                     break;
                 default:
-                    code = buf_CleanAsyncLocked(bp, reqp, &wasDirty);
+                    code = buf_CleanAsyncLocked(scp, bp, reqp, 0, &wasDirty);
                     if (bp->flags & CM_BUF_ERROR) {
                         code = bp->error;
                         if (code == 0)
index 46a1efa..018e982 100644 (file)
@@ -115,13 +115,18 @@ typedef struct cm_buf {
 #define CM_BUF_EOF     0x80    /* read 0 bytes; used for detecting EOF */
 
 typedef struct cm_buf_ops {
-    long (*Writep)(void *, osi_hyper_t *, long, long, struct cm_user *,
-                       struct cm_req *);
-    long (*Readp)(cm_buf_t *, long, long *, struct cm_user *);
-    long (*Stabilizep)(void *, struct cm_user *, struct cm_req *);
-    long (*Unstabilizep)(void *, struct cm_user *);
+    long (*Writep)(void *vscp, osi_hyper_t *offsetp,
+                   long length, long flags,
+                   struct cm_user *userp,
+                   struct cm_req *reqp);
+    long (*Readp)(cm_buf_t *bufp, long length,
+                  long *bytesReadp, struct cm_user *userp);
+    long (*Stabilizep)(void *vscp, struct cm_user *userp, struct cm_req *reqp);
+    long (*Unstabilizep)(void *vscp, struct cm_user *userp);
 } cm_buf_ops_t;
 
+#define CM_BUF_WRITE_SCP_LOCKED 0x1
+
 /* global locks */
 extern osi_rwlock_t buf_globalLock;
 
@@ -170,9 +175,9 @@ extern long buf_Get(struct cm_scache *, osi_hyper_t *, cm_req_t *, cm_buf_t **);
 
 extern long buf_GetNew(struct cm_scache *, osi_hyper_t *, cm_req_t *, cm_buf_t **);
 
-extern afs_uint32 buf_CleanAsyncLocked(cm_buf_t *, cm_req_t *, afs_uint32 *);
+extern afs_uint32 buf_CleanAsyncLocked(cm_scache_t *, cm_buf_t *, cm_req_t *, afs_uint32 flags, afs_uint32 *);
 
-extern afs_uint32 buf_CleanAsync(cm_buf_t *, cm_req_t *, afs_uint32 *);
+extern afs_uint32 buf_CleanAsync(cm_scache_t *, cm_buf_t *, cm_req_t *, afs_uint32 flags, afs_uint32 *);
 
 extern void buf_CleanWait(cm_scache_t *, cm_buf_t *, afs_uint32 locked);
 
index 2821e48..9742d40 100644 (file)
@@ -75,6 +75,7 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
     cm_bulkIO_t biod;          /* bulk IO descriptor */
     int require_64bit_ops = 0;
     int call_was_64bit = 0;
+    int scp_locked = flags & CM_BUF_WRITE_SCP_LOCKED;
 
     osi_assertx(userp != NULL, "null cm_user_t");
     osi_assertx(scp != NULL, "null cm_scache_t");
@@ -85,10 +86,11 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
      * drops lots of locks, and may indeed return a properly initialized
      * buffer, although more likely it will just return a new, empty, buffer.
      */
-
-    lock_ObtainWrite(&scp->rw);
+    if (!scp_locked)
+        lock_ObtainWrite(&scp->rw);
     if (scp->flags & CM_SCACHEFLAG_DELETED) {
-       lock_ReleaseWrite(&scp->rw);
+        if (!scp_locked)
+            lock_ReleaseWrite(&scp->rw);
        return CM_ERROR_NOSUCHFILE;
     }
 
@@ -101,7 +103,8 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
     if (code) {
         osi_Log1(afsd_logp, "cm_SetupStoreBIOD code %x", code);
         cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STOREDATA_EXCL);
-        lock_ReleaseWrite(&scp->rw);
+        if (!scp_locked)
+            lock_ReleaseWrite(&scp->rw);
         return code;
     }
 
@@ -109,7 +112,8 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
         osi_Log0(afsd_logp, "cm_SetupStoreBIOD length 0");
         cm_ReleaseBIOD(&biod, 1, 0, 1);        /* should be a NOOP */
         cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STOREDATA_EXCL);
-        lock_ReleaseWrite(&scp->rw);
+        if (!scp_locked)
+            lock_ReleaseWrite(&scp->rw);
         return 0;
     }
 
@@ -325,7 +329,8 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
         else if (code == CM_ERROR_QUOTA)
             scp->flags |= CM_SCACHEFLAG_OVERQUOTA;
     }
-    lock_ReleaseWrite(&scp->rw);
+    if (!scp_locked)
+        lock_ReleaseWrite(&scp->rw);
 
     return code;
 }