From: Jeffrey Altman <jaltman@secure-endpoints.com>
Date: Thu, 14 Aug 2008 03:27:36 +0000 (+0000)
Subject: rx-buffer-allocation-corrected-20080813
X-Git-Tag: openafs-devel-1_5_61~896
X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=919fb5e14bdcde66218e64cfdd7fecccab67d8f5

rx-buffer-allocation-corrected-20080813

LICENSE MIT

a corrected version of delta rx-buffer-allocation-20080812
---

diff --git a/src/rx/rx_globals.h b/src/rx/rx_globals.h
index c054071..2229e25 100644
--- a/src/rx/rx_globals.h
+++ b/src/rx/rx_globals.h
@@ -245,17 +245,22 @@ EXT void rxi_FlushLocalPacketsTSFPQ(void); /* flush all thread-local packets to
 #define RX_TS_FPQ_FLUSH_GLOBAL 1
 #define RX_TS_FPQ_PULL_GLOBAL 1
 #define RX_TS_FPQ_ALLOW_OVERCOMMIT 1
-/* compute the localmax and globsize values from rx_TSFPQMaxProcs and rx_nPackets.
-   arbitarily set local max so that all threads consume 90% of packets, if all local queues are full.
-   arbitarily set transfer glob size to 20% of max local packet queue length.
-   also set minimum values of 15 and 3. */
+/*
+ * compute the localmax and globsize values from rx_TSFPQMaxProcs and rx_nPackets.
+ * arbitarily set local max so that all threads consume 90% of packets, if all local queues are full.
+ * arbitarily set transfer glob size to 20% of max local packet queue length.
+ * also set minimum values of 15 and 3.  Given the algorithms, the number of buffers allocated
+ * by each call to AllocPacketBufs() will increase indefinitely without a cap on the transfer
+ * glob size.  A cap of 64 is selected because that will produce an allocation of greater than
+ * three times that amount which is greater than half of ncalls * maxReceiveWindow. 
+ */
 #define RX_TS_FPQ_COMPUTE_LIMITS \
     do { \
         register int newmax, newglob; \
         newmax = (rx_nPackets * 9) / (10 * rx_TSFPQMaxProcs); \
         newmax = (newmax >= 15) ? newmax : 15; \
         newglob = newmax / 5; \
-        newglob = (newglob >= 3) ? newglob : 3; \
+        newglob = (newglob >= 3) ? (newglob < 64 ? newglob : 64) : 3; \
         rx_TSFPQLocalMax = newmax; \
         rx_TSFPQGlobSize = newglob; \
     } while(0)
@@ -325,10 +330,12 @@ EXT void rxi_FlushLocalPacketsTSFPQ(void); /* flush all thread-local packets to
 /* same as above, except user has direct control over number to transfer */
 #define RX_TS_FPQ_GTOL2(rx_ts_info_p,num_transfer) \
     do { \
-        register int i; \
+        register int i, tsize; \
         register struct rx_packet * p; \
+        tsize = (num_transfer); \
+        if (tsize > rx_nFreePackets) tsize = rx_nFreePackets; \
         for (i=0,p=queue_First(&rx_freePacketQueue, rx_packet); \
-             i < (num_transfer); i++,p=queue_Next(p, rx_packet)); \
+             i < tsize; i++,p=queue_Next(p, rx_packet)); \
         queue_SplitBeforeAppend(&rx_freePacketQueue,&((rx_ts_info_p)->_FPQ),p); \
         (rx_ts_info_p)->_FPQ.len += i; \
         rx_nFreePackets -= i; \
@@ -345,20 +352,23 @@ EXT void rxi_FlushLocalPacketsTSFPQ(void); /* flush all thread-local packets to
         (rx_ts_info_p)->_FPQ.checkout_ops++; \
         (rx_ts_info_p)->_FPQ.checkout_xfer++; \
     } while(0)
-/* checkout multiple packets from the thread-specific free packet queue */
+/* checkout multiple packets from the thread-specific free packet queue.
+ * num_transfer must be a variable.
+ */
 #define RX_TS_FPQ_QCHECKOUT(rx_ts_info_p,num_transfer,q) \
     do { \
         register int i; \
         register struct rx_packet *p; \
+        if (num_transfer > (rx_ts_info_p)->_FPQ.len) num_transfer = (rx_ts_info_p)->_FPQ.len; \
         for (i=0, p=queue_First(&((rx_ts_info_p)->_FPQ), rx_packet); \
-             i < (num_transfer); \
+             i < num_transfer; \
              i++, p=queue_Next(p, rx_packet)) { \
             RX_FPQ_MARK_USED(p); \
         } \
         queue_SplitBeforeAppend(&((rx_ts_info_p)->_FPQ),(q),p); \
-        (rx_ts_info_p)->_FPQ.len -= (num_transfer); \
+        (rx_ts_info_p)->_FPQ.len -= num_transfer; \
         (rx_ts_info_p)->_FPQ.checkout_ops++; \
-        (rx_ts_info_p)->_FPQ.checkout_xfer += (num_transfer); \
+        (rx_ts_info_p)->_FPQ.checkout_xfer += num_transfer; \
     } while(0)
 /* check a packet into the thread-specific free packet queue */
 #define RX_TS_FPQ_CHECKIN(rx_ts_info_p,p) \
diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c
index 7f083de..398ca06 100644
--- a/src/rx/rx_packet.c
+++ b/src/rx/rx_packet.c
@@ -285,7 +285,7 @@ AllocPacketBufs(int class, int num_pkts, struct rx_queue * q)
 	    /* alloc enough for us, plus a few globs for other threads */
 	    alloc = transfer + (3 * rx_TSFPQGlobSize) - rx_nFreePackets;
 	    rxi_MorePacketsNoLock(MAX(alloc, rx_initSendWindow));
-	    transfer += rx_TSFPQGlobSize;
+	    transfer = rx_TSFPQGlobSize;
 	}
 
 	RX_TS_FPQ_GTOL2(rx_ts_info, transfer);
@@ -654,9 +654,14 @@ rxi_MorePacketsNoLock(int apackets)
      * to hold maximal amounts of data */
     apackets += (apackets / 4)
 	* ((rx_maxJumboRecvSize - RX_FIRSTBUFFERSIZE) / RX_CBUFFERSIZE);
-    getme = apackets * sizeof(struct rx_packet);
-    p = rx_mallocedP = (struct rx_packet *)osi_Alloc(getme);
-
+    do {
+        getme = apackets * sizeof(struct rx_packet);
+        p = rx_mallocedP = (struct rx_packet *)osi_Alloc(getme);
+	if (p == NULL) {
+            apackets -= apackets / 4;
+            osi_Assert(apackets > 0);
+        }
+    } while(p == NULL);
     memset((char *)p, 0, getme);
 
     for (e = p + apackets; p < e; p++) {