viced: Clarify comment explaining cba sorting

The current comment here is very brief; it may not be immediately
clear to a reader why we are sorting these, and so why we need the
given CBAs in an array. Expand on it a bit.

Note that it seems like it might be possible to refactor multi_Rx to
not require all calls to be created before any packets are sent. If
multi_Rx were changed to send data as we create calls, it may be
possible to eliminate this sorting, and allow for slightly more
efficient callback traversal when breaking callbacks.

Change-Id: I966cdcf6d40aa5c02f8768f4dd76c580c811ccaf
Reviewed-on: http://gerrit.openafs.org/10170
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

diff --git a/src/viced/callback.c b/src/viced/callback.c
index cfaaa63..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 */