afs: release the packets used by rx on shutdown

When the OpenAFS client is unmounted on DARWIN, the blocks of packets
allocated by RX are released. Historically, the memory used by those
packets was never properly released.

Before 230dcebcd61064cc9aab6d20d34ff866a5c575ea, only the last block of
packets used to be released:

...
struct rx_packet *rx_mallocedP = 0;
...
void
rxi_MorePackets(int apackets)
{
    ...
    getme = apackets * sizeof(struct rx_packet);
    p = rx_mallocedP = (struct rx_packet *)osi_Alloc(getme);
    ...
}
...
void
rxi_FreeAllPackets(void)
{
    ...
    osi_Free(rx_mallocedP, ...);
    ...
}
...

As we can see, ‘rx_mallocedP’ is a global pointer that stores the
first address of the last allocated block of packets. As a result, when
‘rxi_FreeAllPackets’ is called, only the last block is released.

However, 230dcebcd61064cc9aab6d20d34ff866a5c575ea moved the global
pointer in question to the end of the last block. As a result, when the
OpenAFS client is unmounted on DARWIN, the ‘rxi_FreeAllPackets’
function releases the wrong block of memory. This problem was exposed
on OS X 10.12 Sierra where the system crashes when the OpenAFS client
is unmounted.

To fix this problem, store the address of every single block of packets
in a queue and release one by one when the OpenAFS client is unmounted.

Reviewed-on: https://gerrit.openafs.org/12427
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 5b28061fb593f5f48df549b07f0ccd848348b93c)

Change-Id: Id8606b1c1444861df69ed4af8169e343964a691d
Reviewed-on: https://gerrit.openafs.org/12602
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

diff --git a/src/afs/afs_call.c b/src/afs/afs_call.c
index 65d485cbb38489b045a43d58a477d30398c0aaf4..1c1f48d52a31d15381ba7cbd79370cfb879935c5 100644 (file)
--- a/src/afs/afs_call.c
+++ b/src/afs/afs_call.c
@@ -1442,6 +1442,7 @@ afs_shutdown(void)
     afs_warn("NetIfPoller... ");
     osi_StopNetIfPoller();
 #endif
+    rxi_FreeAllPackets();
 
     afs_termState = AFSOP_STOP_COMPLETE;
 

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 10ebf5ef26ad999ca0309fa4977c3cbaed4d6acb..9770180768c469f0b609ed3966f1a79249e5e5b5 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -263,6 +263,9 @@ rxi_InitPthread(void)
 
     MUTEX_INIT(&rx_rpc_stats, "rx_rpc_stats", MUTEX_DEFAULT, 0);
     MUTEX_INIT(&rx_freePktQ_lock, "rx_freePktQ_lock", MUTEX_DEFAULT, 0);
+    MUTEX_INIT(&rx_mallocedPktQ_lock, "rx_mallocedPktQ_lock", MUTEX_DEFAULT,
+              0);
+
 #ifdef RX_ENABLE_LOCKS
 #ifdef RX_LOCKS_DB
     rxdb_init();
@@ -523,6 +526,9 @@ rx_InitHost(u_int host, u_int port)
     MUTEX_INIT(&rx_connHashTable_lock, "rx_connHashTable_lock", MUTEX_DEFAULT,
               0);
     MUTEX_INIT(&rx_serverPool_lock, "rx_serverPool_lock", MUTEX_DEFAULT, 0);
+    MUTEX_INIT(&rx_mallocedPktQ_lock, "rx_mallocedPktQ_lock", MUTEX_DEFAULT,
+              0);
+
 #if defined(AFS_HPUX110_ENV)
     if (!uniprocessor)
        rx_sleepLock = alloc_spinlock(LAST_HELD_ORDER - 10, "rx_sleepLock");
@@ -546,6 +552,7 @@ rx_InitHost(u_int host, u_int port)
     queue_Init(&rx_freePacketQueue);
     rxi_NeedMorePackets = FALSE;
     rx_nPackets = 0;   /* rx_nPackets is managed by rxi_MorePackets* */
+    queue_Init(&rx_mallocedPacketQueue);
 
     /* enforce a minimum number of allocated packets */
     if (rx_extraPackets < rxi_nSendFrags * rx_maxSendWindow)
@@ -8182,8 +8189,6 @@ shutdown_rx(void)
          rx_hashTableSize * sizeof(struct rx_connection *));
     UNPIN(rx_peerHashTable, rx_hashTableSize * sizeof(struct rx_peer *));
 
-    rxi_FreeAllPackets();
-
     MUTEX_ENTER(&rx_quota_mutex);
     rxi_dataQuota = RX_MAX_QUOTA;
     rxi_availProcs = rxi_totalMin = rxi_minDeficit = 0;

diff --git a/src/rx/rx_globals.h b/src/rx/rx_globals.h
index 7723d61a89ba4912c6d39082222e15d246341769..ad960be527af19aa1aa83be75041c27779fd205d 100644 (file)
--- a/src/rx/rx_globals.h
+++ b/src/rx/rx_globals.h
@@ -249,6 +249,18 @@ EXT struct rx_queue rx_freePacketQueue;
 EXT afs_kmutex_t rx_freePktQ_lock;
 #endif /* RX_ENABLE_LOCKS */
 
+/*!
+ * \brief Queue of allocated packets.
+ *
+ * This queue is used to keep track of the blocks of allocated packets.
+ * This information is used when afs is being unmounted and the memory
+ * used by those packets needs to be released.
+ */
+EXT struct rx_queue rx_mallocedPacketQueue;
+#ifdef RX_ENABLE_LOCKS
+EXT afs_kmutex_t rx_mallocedPktQ_lock;
+#endif /* RX_ENABLE_LOCKS */
+
 #if defined(AFS_PTHREAD_ENV)
 #define RX_ENABLE_TSFPQ
 EXT int rx_TSFPQGlobSize GLOBALSINIT(3); /* number of packets to transfer between global and local queues in one op */

diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c
index 24006d07c574c2c81f0d366c76416c4d6742bb5a..44761cc08356bec4fe15e7c4205cacf1bace5a02 100644 (file)
--- a/src/rx/rx_packet.c
+++ b/src/rx/rx_packet.c
@@ -85,6 +85,15 @@
 #endif
 #endif /* KERNEL */
 
+/*!
+ * \brief structure used to keep track of allocated packets
+ */
+struct rx_mallocedPacket {
+    struct rx_queue entry;     /*!< chained using the queue package */
+    struct rx_packet *addr;    /*!< address of the first element */
+    afs_uint32 size;           /*!< array size in bytes */
+};
+
 #ifdef RX_LOCKS_DB
 /* rxdb_fileID is used to identify the lock location, along with line#. */
 static int rxdb_fileID = RXDB_FILE_RX_PACKET;
@@ -526,6 +535,33 @@ rxi_AllocDataBuf(struct rx_packet *p, int nb, int class)
     return nb;
 }
 
+/**
+ * Register allocated packets.
+ *
+ * @param[in] addr array of packets
+ * @param[in] npkt number of packets
+ *
+ * @return none
+ */
+static void
+registerPackets(struct rx_packet *addr, afs_uint32 npkt)
+{
+    struct rx_mallocedPacket *mp;
+
+    mp = (struct rx_mallocedPacket *)
+       osi_Alloc(sizeof(struct rx_mallocedPacket));
+
+    osi_Assert(mp);
+    memset(mp, 0, sizeof(struct rx_mallocedPacket));
+
+    mp->addr = addr;
+    mp->size = npkt * sizeof(struct rx_packet);
+
+    MUTEX_ENTER(&rx_mallocedPktQ_lock);
+    queue_Append(&rx_mallocedPacketQueue, mp);
+    MUTEX_EXIT(&rx_mallocedPktQ_lock);
+}
+
 /* Add more packet buffers */
 #ifdef RX_ENABLE_TSFPQ
 void
@@ -539,6 +575,7 @@ rxi_MorePackets(int apackets)
     getme = apackets * sizeof(struct rx_packet);
     p = (struct rx_packet *)osi_Alloc(getme);
     osi_Assert(p);
+    registerPackets(p, apackets);
 
     PIN(p, getme);             /* XXXXX */
     memset(p, 0, getme);
@@ -593,6 +630,7 @@ rxi_MorePackets(int apackets)
     getme = apackets * sizeof(struct rx_packet);
     p = (struct rx_packet *)osi_Alloc(getme);
     osi_Assert(p);
+    registerPackets(p, apackets);
 
     PIN(p, getme);             /* XXXXX */
     memset(p, 0, getme);
@@ -635,6 +673,7 @@ rxi_MorePacketsTSFPQ(int apackets, int flush_global, int num_keep_local)
 
     getme = apackets * sizeof(struct rx_packet);
     p = (struct rx_packet *)osi_Alloc(getme);
+    registerPackets(p, apackets);
 
     PIN(p, getme);             /* XXXXX */
     memset(p, 0, getme);
@@ -703,6 +742,7 @@ rxi_MorePacketsNoLock(int apackets)
         }
     } while(p == NULL);
     memset(p, 0, getme);
+    registerPackets(p, apackets);
 
 #ifdef RX_ENABLE_TSFPQ
     RX_TS_INFO_GET(rx_ts_info);
@@ -739,11 +779,18 @@ rxi_MorePacketsNoLock(int apackets)
 void
 rxi_FreeAllPackets(void)
 {
-    /* must be called at proper interrupt level, etcetera */
-    /* MTUXXX need to free all Packets */
-    osi_Free(rx_mallocedP,
-            (rx_maxReceiveWindow + 2) * sizeof(struct rx_packet));
-    UNPIN(rx_mallocedP, (rx_maxReceiveWindow + 2) * sizeof(struct rx_packet));
+    struct rx_mallocedPacket *mp;
+
+    MUTEX_ENTER(&rx_mallocedPktQ_lock);
+
+    while (!queue_IsEmpty(&rx_mallocedPacketQueue)) {
+       mp = queue_First(&rx_mallocedPacketQueue, rx_mallocedPacket);
+       queue_Remove(mp);
+       osi_Free(mp->addr, mp->size);
+       UNPIN(mp->addr, mp->size);
+       osi_Free(mp, sizeof(*mp));
+    }
+    MUTEX_EXIT(&rx_mallocedPktQ_lock);
 }
 
 #ifdef RX_ENABLE_TSFPQ