viced: Clarify comment explaining cba sorting

[openafs.git] / src / viced / callback.c

diff --git a/src/viced/callback.c b/src/viced/callback.c
index 5460702..6618f88 100644 (file)
--- a/src/viced/callback.c
+++ b/src/viced/callback.c
@@ -683,7 +683,20 @@ MultiBreakCallBack_r(struct cbstruct cba[], int ncbas,
 
     opr_Assert(ncbas <= MAX_CB_HOSTS);
 
-    /* sort cba list to avoid makecall issues */
+    /*
+     * When we issue a multi_Rx callback break, we must rx_NewCall a call for
+     * each host before we do anything. If there are no call channels
+     * available on the conn, we must wait for one of the existing calls to
+     * finish. If another thread is breaking callbacks at the same time, it is
+     * possible for us to be waiting on NewCall for one of their multi_Rx
+     * CallBack calls to finish, but they are waiting on NewCall for one of
+     * our calls to finish. So we deadlock.
+     *
+     * This can be thought of as similar to obtaining multiple locks at the
+     * same time. So if we establish an ordering, the possibility of deadlock
+     * goes away. Here we provide such an ordering, by sorting our CBAs
+     * according to CompareCBA.
+     */
     qsort(cba, ncbas, sizeof(struct cbstruct), CompareCBA);
 
     /* set up conns for multi-call */
@@ -2646,6 +2659,8 @@ cb_OldToNew(struct fs_dump_state * state, afs_uint32 old, afs_uint32 * new)
 }
 #endif /* AFS_DEMAND_ATTACH_FS */
 
+#define DumpBytes(fd,buf,req) if (write(fd, buf, req) < 0) ; /* don't care */
+
 static int
 DumpCallBackState_r(void)
 {
@@ -2663,19 +2678,23 @@ DumpCallBackState_r(void)
                 AFSDIR_SERVER_CBKDUMP_FILEPATH));
        return 0;
     }
-    (void)write(fd, &magic, sizeof(magic));
-    (void)write(fd, &now, sizeof(now));
-    (void)write(fd, &cbstuff, sizeof(cbstuff));
-    (void)write(fd, TimeOuts, sizeof(TimeOuts));
-    (void)write(fd, timeout, sizeof(timeout));
-    (void)write(fd, &tfirst, sizeof(tfirst));
+    /*
+     * Collect but ignoring the return value of write(2) here,
+     * to avoid compiler warnings on some platforms.
+     */
+    DumpBytes(fd, &magic, sizeof(magic));
+    DumpBytes(fd, &now, sizeof(now));
+    DumpBytes(fd, &cbstuff, sizeof(cbstuff));
+    DumpBytes(fd, TimeOuts, sizeof(TimeOuts));
+    DumpBytes(fd, timeout, sizeof(timeout));
+    DumpBytes(fd, &tfirst, sizeof(tfirst));
     freelisthead = cbtoi((struct CallBack *)CBfree);
-    (void)write(fd, &freelisthead, sizeof(freelisthead));      /* This is a pointer */
+    DumpBytes(fd, &freelisthead, sizeof(freelisthead));        /* This is a pointer */
     freelisthead = fetoi((struct FileEntry *)FEfree);
-    (void)write(fd, &freelisthead, sizeof(freelisthead));      /* This is a pointer */
-    (void)write(fd, HashTable, sizeof(HashTable));
-    (void)write(fd, &CB[1], sizeof(CB[1]) * cbstuff.nblks);    /* CB stuff */
-    (void)write(fd, &FE[1], sizeof(FE[1]) * cbstuff.nblks);    /* FE stuff */
+    DumpBytes(fd, &freelisthead, sizeof(freelisthead));        /* This is a pointer */
+    DumpBytes(fd, HashTable, sizeof(HashTable));
+    DumpBytes(fd, &CB[1], sizeof(CB[1]) * cbstuff.nblks);      /* CB stuff */
+    DumpBytes(fd, &FE[1], sizeof(FE[1]) * cbstuff.nblks);      /* FE stuff */
     close(fd);
 
     return 0;
@@ -2696,6 +2715,22 @@ DumpCallBackState(void) {
 
 #ifdef INTERPRET_DUMP
 
+static void
+ReadBytes(int fd, void *buf, size_t req)
+{
+    ssize_t count;
+
+    count = read(fd, buf, req);
+    if (count < 0) {
+       perror("read");
+       exit(-1);
+    } else if (count != req) {
+       fprintf(stderr, "read: premature EOF (expected %lu, got %lu)\n",
+               (unsigned long)req, (unsigned long)count);
+       exit(-1);
+    }
+}
+
 /* This is only compiled in for the callback analyzer program */
 /* Returns the time of the dump */
 time_t
@@ -2715,7 +2750,7 @@ ReadDump(char *file, int timebits)
        fprintf(stderr, "Couldn't read dump file %s\n", file);
        exit(1);
     }
-    read(fd, &magic, sizeof(magic));
+    ReadBytes(fd, &magic, sizeof(magic));
     if (magic == MAGICV2) {
        timebits = 32;
     } else {
@@ -2729,26 +2764,26 @@ ReadDump(char *file, int timebits)
        }
     }
     if (timebits == 64) {
-       read(fd, &now64, sizeof(afs_int64));
+       ReadBytes(fd, &now64, sizeof(afs_int64));
        now = (afs_int32) now64;
     } else
-       read(fd, &now, sizeof(afs_int32));
+       ReadBytes(fd, &now, sizeof(afs_int32));
 
-    read(fd, &cbstuff, sizeof(cbstuff));
-    read(fd, TimeOuts, sizeof(TimeOuts));
-    read(fd, timeout, sizeof(timeout));
-    read(fd, &tfirst, sizeof(tfirst));
-    read(fd, &freelisthead, sizeof(freelisthead));
+    ReadBytes(fd, &cbstuff, sizeof(cbstuff));
+    ReadBytes(fd, TimeOuts, sizeof(TimeOuts));
+    ReadBytes(fd, timeout, sizeof(timeout));
+    ReadBytes(fd, &tfirst, sizeof(tfirst));
+    ReadBytes(fd, &freelisthead, sizeof(freelisthead));
     CB = ((struct CallBack
           *)(calloc(cbstuff.nblks, sizeof(struct CallBack)))) - 1;
     FE = ((struct FileEntry
           *)(calloc(cbstuff.nblks, sizeof(struct FileEntry)))) - 1;
     CBfree = (struct CallBack *)itocb(freelisthead);
-    read(fd, &freelisthead, sizeof(freelisthead));
+    ReadBytes(fd, &freelisthead, sizeof(freelisthead));
     FEfree = (struct FileEntry *)itofe(freelisthead);
-    read(fd, HashTable, sizeof(HashTable));
-    read(fd, &CB[1], sizeof(CB[1]) * cbstuff.nblks);   /* CB stuff */
-    read(fd, &FE[1], sizeof(FE[1]) * cbstuff.nblks);   /* FE stuff */
+    ReadBytes(fd, HashTable, sizeof(HashTable));
+    ReadBytes(fd, &CB[1], sizeof(CB[1]) * cbstuff.nblks);      /* CB stuff */
+    ReadBytes(fd, &FE[1], sizeof(FE[1]) * cbstuff.nblks);      /* FE stuff */
     if (close(fd)) {
        perror("Error reading dumpfile");
        exit(1);