From: Michael Vrable <mvrable@cs.ucsd.edu>
Date: Thu, 28 Jan 2010 23:15:14 +0000 (-0800)
Subject: Fix a deadlock and a few memory leaks.
X-Git-Url: https://git.vrable.net/?a=commitdiff_plain;h=79fd07ce3738aceebc08fd363c14795ae9353057;p=bluesky.git

Fix a deadlock and a few memory leaks.
---

diff --git a/bluesky/bluesky.h b/bluesky/bluesky.h
index 421d07d..7f0ac85 100644
--- a/bluesky/bluesky.h
+++ b/bluesky/bluesky.h
@@ -134,6 +134,9 @@ bluesky_time_hires bluesky_now_hires();
  *     acquire locks on parents in the filesystem tree before children.
  *     (TODO: What about rename when we acquire locks in unrelated parts of the
  *     filesystem?)
+ *   - An inode should not be locked while the filesystem lock is already held,
+ *     since some code may do an inode lookup (which acquires the filesystem
+ *     lock) while a different inode is locked.
  * */
 typedef struct {
     GMutex *lock;
diff --git a/bluesky/cache.c b/bluesky/cache.c
index 41348e2..979c917 100644
--- a/bluesky/cache.c
+++ b/bluesky/cache.c
@@ -36,7 +36,7 @@ static void writeback_complete(gpointer a, gpointer i)
     g_mutex_unlock(inode->lock);
 }
 
-static void flushd_inode(gpointer key, gpointer value, gpointer user_data)
+static void flushd_inode(gpointer value, gpointer user_data)
 {
     BlueSkyFS *fs = (BlueSkyFS *)user_data;
 
@@ -46,6 +46,7 @@ static void flushd_inode(gpointer key, gpointer value, gpointer user_data)
 
     if (inode->change_count == inode->change_commit) {
         g_mutex_unlock(inode->lock);
+        bluesky_inode_unref(inode);
         return;
     }
 
@@ -53,6 +54,7 @@ static void flushd_inode(gpointer key, gpointer value, gpointer user_data)
         /* Waiting for an earlier writeback to finish, so don't start a new
          * writeback yet. */
         g_mutex_unlock(inode->lock);
+        bluesky_inode_unref(inode);
         return;
     }
 
@@ -60,6 +62,7 @@ static void flushd_inode(gpointer key, gpointer value, gpointer user_data)
     if (elapsed < WRITEBACK_DELAY) {
         /* Give a bit more time before starting writeback. */
         g_mutex_unlock(inode->lock);
+        bluesky_inode_unref(inode);
         return;
     }
 
@@ -81,15 +84,35 @@ static void flushd_inode(gpointer key, gpointer value, gpointer user_data)
     bluesky_store_async_unref(barrier);
 
     g_mutex_unlock(inode->lock);
+    bluesky_inode_unref(inode);
 }
 
 /* Scan through the cache for dirty data and start flushing it to stable
  * storage.  This does not guarantee that data is committed when it returns.
  * Instead, this can be called occasionally to ensure that dirty data is
- * gradually flushed. */
+ * gradually flushed.
+ *
+ * We do not want to hold the filesystem lock while flushing individual inodes,
+ * a that could lead to deadlock.  So first scan through the inode table to get
+ * a reference to all inodes, then process that queue of inodes after dropping
+ * the filesystem lock. */
+static void gather_inodes(gpointer key, gpointer value, gpointer user_data)
+{
+    GSList **list = (GSList **)user_data;
+    *list = g_slist_prepend(*list, value);
+    bluesky_inode_ref((BlueSkyInode *)value);
+}
+
 void bluesky_flushd_invoke(BlueSkyFS *fs)
 {
+    GSList *list = NULL;
+
     g_mutex_lock(fs->lock);
-    g_hash_table_foreach(fs->inodes, flushd_inode, fs);
+    g_hash_table_foreach(fs->inodes, gather_inodes, &list);
     g_mutex_unlock(fs->lock);
+
+    list = g_slist_reverse(list);
+    g_slist_foreach(list, flushd_inode, fs);
+
+    g_slist_free(list);
 }
diff --git a/bluesky/inode.c b/bluesky/inode.c
index cfa6697..30974ca 100644
--- a/bluesky/inode.c
+++ b/bluesky/inode.c
@@ -312,6 +312,8 @@ void bluesky_inode_fetch(BlueSkyFS *fs, uint64_t inum)
     if (bluesky_options.sync_inode_fetches) {
         bluesky_store_async_wait(async);
     }
+
+    bluesky_store_async_unref(async);
 }
 
 /* Synchronize filesystem superblock to stable storage. */
diff --git a/nfs3/rpc.c b/nfs3/rpc.c
index cda1330..2c640d7 100644
--- a/nfs3/rpc.c
+++ b/nfs3/rpc.c
@@ -209,6 +209,7 @@ async_rpc_send_failure(RPCRequest *req, enum accept_stat stat)
         if (!req->xdr_args_free(&xdr, req->args)) {
             fprintf(stderr, "unable to free arguments");
         }
+        g_free(req->args);
     }
 
     if (req->raw_args != NULL)
@@ -270,6 +271,7 @@ async_rpc_send_reply(RPCRequest *req, void *result)
         if (!req->xdr_args_free(&xdr, req->args)) {
             fprintf(stderr, "unable to free arguments");
         }
+        g_free(req->args);
     }
 
     if (req->raw_args != NULL)