Fix locking in afs_buffer.c
[openafs.git] / src / afs / afs_buffer.c
index 3f3e5de..220b496 100644 (file)
@@ -10,8 +10,6 @@
 #include <afsconfig.h>
 #include "afs/param.h"
 
-RCSID
-    ("$Header$");
 
 #include "afs/sysincludes.h"
 #include "afsincludes.h"
@@ -78,9 +76,23 @@ extern struct buf *geteblk();
 #ifdef AFS_FBSD_ENV
 #define timecounter afs_timecounter
 #endif
-/* The locks for individual buffer entries are now sometimes obtained while holding the
- * afs_bufferLock. Thus we now have a locking hierarchy: afs_bufferLock -> Buffers[].lock.
+
+/* A note on locking in 'struct buffer'
+ *
+ * afs_bufferLock protects the hash chain, and the 'lockers' field where that
+ * has a zero value. It must be held whenever lockers is incremented from zero.
+ *
+ * The individual buffer lock protects the contents of the structure, including
+ * the lockers field.
+ *
+ * For safety: afs_bufferLock and the individual buffer lock must be held
+ * when obtaining a reference on a structure. Only the individual buffer lock
+ * need be held when releasing a reference.
+ *
+ * The locking hierarchy is afs_bufferLock-> buffer.lock
+ *
  */
+
 static afs_lock_t afs_bufferLock;
 static struct buffer *phTable[PHSIZE]; /* page hash table */
 static int nbuffers;
@@ -130,7 +142,7 @@ DInit(int abuffers)
        /* Fill in each buffer with an empty indication. */
        tb = &Buffers[i];
        tb->fid = NULLIDX;
-       tb->inode = 0;
+       afs_reset_inode(&tb->inode);
        tb->accesstime = 0;
        tb->lockers = 0;
 #if defined(AFS_USEBUFFERS)
@@ -172,8 +184,8 @@ DRead(register struct dcache *adc, register int page)
     if ((tb = phTable[pHash(adc->index, page)])) {
        if (bufmatch(tb)) {
            MObtainWriteLock(&tb->lock, 257);
-           ReleaseWriteLock(&afs_bufferLock);
            tb->lockers++;
+           ReleaseWriteLock(&afs_bufferLock);
            tb->accesstime = timecounter++;
            AFS_STATS(afs_stats_cmperf.bufHits++);
            MReleaseWriteLock(&tb->lock);
@@ -185,8 +197,8 @@ DRead(register struct dcache *adc, register int page)
                if (bufmatch(tb2)) {
                    buf_Front(bufhead, tb, tb2);
                    MObtainWriteLock(&tb2->lock, 258);
-                   ReleaseWriteLock(&afs_bufferLock);
                    tb2->lockers++;
+                   ReleaseWriteLock(&afs_bufferLock);
                    tb2->accesstime = timecounter++;
                    AFS_STATS(afs_stats_cmperf.bufHits++);
                    MReleaseWriteLock(&tb2->lock);
@@ -196,8 +208,8 @@ DRead(register struct dcache *adc, register int page)
                    if (bufmatch(tb)) {
                        buf_Front(bufhead, tb2, tb);
                        MObtainWriteLock(&tb->lock, 259);
-                       ReleaseWriteLock(&afs_bufferLock);
                        tb->lockers++;
+                       ReleaseWriteLock(&afs_bufferLock);
                        tb->accesstime = timecounter++;
                        AFS_STATS(afs_stats_cmperf.bufHits++);
                        MReleaseWriteLock(&tb->lock);
@@ -222,23 +234,23 @@ DRead(register struct dcache *adc, register int page)
        return NULL;
     }
     MObtainWriteLock(&tb->lock, 260);
-    MReleaseWriteLock(&afs_bufferLock);
     tb->lockers++;
+    MReleaseWriteLock(&afs_bufferLock);
     if (page * AFS_BUFFER_PAGESIZE >= adc->f.chunkBytes) {
        tb->fid = NULLIDX;
-       tb->inode = 0;
+       afs_reset_inode(&tb->inode);
        tb->lockers--;
        MReleaseWriteLock(&tb->lock);
        return NULL;
     }
-    tfile = afs_CFileOpen(adc->f.inode);
+    tfile = afs_CFileOpen(&adc->f.inode);
     code =
        afs_CFileRead(tfile, tb->page * AFS_BUFFER_PAGESIZE, tb->data,
                      AFS_BUFFER_PAGESIZE);
     afs_CFileClose(tfile);
     if (code < AFS_BUFFER_PAGESIZE) {
        tb->fid = NULLIDX;
-       tb->inode = 0;
+       afs_reset_inode(&tb->inode);
        tb->lockers--;
        MReleaseWriteLock(&tb->lock);
        return NULL;
@@ -337,13 +349,13 @@ afs_newslot(struct dcache *adc, afs_int32 apage, register struct buffer *lp)
         * seems extreme.  To the best of my knowledge, all the callers
         * of DRead are prepared to handle a zero return.  Some of them
         * just panic directly, but not all of them. */
-       afs_warn("all buffers locked");
+       afs_warn("afs: all buffers locked\n");
        return 0;
     }
 
     if (lp->dirty) {
        /* see DFlush for rationale for not getting and locking the dcache */
-       tfile = afs_CFileOpen(lp->inode);
+        tfile = afs_CFileOpen(&lp->inode);
        afs_CFileWrite(tfile, lp->page * AFS_BUFFER_PAGESIZE, lp->data,
                       AFS_BUFFER_PAGESIZE);
        lp->dirty = 0;
@@ -353,7 +365,7 @@ afs_newslot(struct dcache *adc, afs_int32 apage, register struct buffer *lp)
 
     /* Now fill in the header. */
     lp->fid = adc->index;
-    lp->inode = adc->f.inode;
+    afs_copy_inode(&lp->inode, &adc->f.inode);
     lp->page = apage;
     lp->accesstime = timecounter++;
     FixupBucket(lp);           /* move to the right hash bucket */
@@ -362,10 +374,11 @@ afs_newslot(struct dcache *adc, afs_int32 apage, register struct buffer *lp)
 }
 
 void
-DRelease(register struct buffer *bp, int flag)
+DRelease(void *loc, int flag)
 {
     /* Release a buffer, specifying whether or not the buffer has been
      * modified by the locker. */
+    register struct buffer *bp = (struct buffer *)loc;
     register int index;
 #if defined(AFS_USEBUFFERS)
     register struct buffer *tp;
@@ -455,20 +468,55 @@ DZap(struct dcache *adc)
            if (tb->fid == adc->index) {
                MObtainWriteLock(&tb->lock, 262);
                tb->fid = NULLIDX;
-               tb->inode = 0;
+               afs_reset_inode(&tb->inode);
                tb->dirty = 0;
                MReleaseWriteLock(&tb->lock);
            }
     MReleaseReadLock(&afs_bufferLock);
 }
 
+static void
+DFlushBuffer(struct buffer *ab) {
+    struct osi_file *tfile;
+    
+    tfile = afs_CFileOpen(&ab->inode);
+    afs_CFileWrite(tfile, ab->page * AFS_BUFFER_PAGESIZE,
+                  ab->data, AFS_BUFFER_PAGESIZE);
+    ab->dirty = 0;     /* Clear the dirty flag */
+    afs_CFileClose(tfile);
+}
+
+void
+DFlushDCache(struct dcache *adc) 
+{
+    int i;
+    struct buffer *tb;
+
+    ObtainReadLock(&afs_bufferLock);
+
+    for (i = 0; i <= PHPAGEMASK; i++)
+        for (tb = phTable[pHash(adc->index, i)]; tb; tb = tb->hashNext)
+           if (tb->fid == adc->index) {
+               ObtainWriteLock(&tb->lock, 701);
+               tb->lockers++;
+               ReleaseReadLock(&afs_bufferLock);
+               if (tb->dirty) {
+                   DFlushBuffer(tb);
+               }
+               tb->lockers--;
+               ReleaseWriteLock(&tb->lock);
+               ObtainReadLock(&afs_bufferLock);
+           }
+
+    ReleaseReadLock(&afs_bufferLock);
+}
+
 void
 DFlush(void)
 {
     /* Flush all the modified buffers. */
     register int i;
     register struct buffer *tb;
-    struct osi_file *tfile;
 
     AFS_STATCNT(DFlush);
     tb = Buffers;
@@ -490,11 +538,7 @@ DFlush(void)
                 * we cannot lock afs_xdcache). In addition, we cannot obtain
                 * a dcache lock while holding the tb->lock of the same file
                 * since that can deadlock with DRead/DNew */
-               tfile = afs_CFileOpen(tb->inode);
-               afs_CFileWrite(tfile, tb->page * AFS_BUFFER_PAGESIZE,
-                              tb->data, AFS_BUFFER_PAGESIZE);
-               tb->dirty = 0;  /* Clear the dirty flag */
-               afs_CFileClose(tfile);
+               DFlushBuffer(tb);
            }
            tb->lockers--;
            MReleaseWriteLock(&tb->lock);
@@ -525,8 +569,8 @@ DNew(register struct dcache *adc, register int page)
        afs_WriteDCache(adc, 1);
     }
     MObtainWriteLock(&tb->lock, 265);
-    MReleaseWriteLock(&afs_bufferLock);
     tb->lockers++;
+    MReleaseWriteLock(&afs_bufferLock);
     MReleaseWriteLock(&tb->lock);
     return tb->data;
 }
@@ -552,7 +596,7 @@ shutdown_bufferpackage(void)
            /* The following check shouldn't be necessary and it will be removed soon */
            if (!tp->bufp)
                afs_warn
-                   ("shutdown_bufferpackage: bufp == 0!! Shouldn't happen\n");
+                   ("afs: shutdown_bufferpackage: bufp == 0!! Shouldn't happen\n");
            else {
                brelse(tp->bufp);
                tp->bufp = 0;
@@ -564,6 +608,6 @@ shutdown_bufferpackage(void)
        timecounter = 1;
        for (i = 0; i < PHSIZE; i++)
            phTable[i] = 0;
-       memset((char *)&afs_bufferLock, 0, sizeof(afs_lock_t));
+       memset(&afs_bufferLock, 0, sizeof(afs_lock_t));
     }
 }