statedump: fix deadlock during state dump of fds

Existing state dump of FD context tries to be extra safe by trying
to call the fd dump callback outside the inode lock. It acheives
this by taking an fd ref and unreffing it later. This exercise can
be harmful at times when the fd unref performed by state dump ends
up being the last unref and triggers fd_destroy. fd_destroy in turn
triggers inode_unref which blocks on inode table lock, while the
inode table lock was already held by the thread before it even
attempted fd ctx dump.

The fix takes away the dangerous ref/unref of the fd during state
dump and instead calls fd_ctx_dump() whiel the inode lock is held.
This is not a problem as long as the dump functions do not call any
inode function which tries to take an inode lock (none of the
existing fd dump ops do)

Change-Id: Ia4b27ba5a321285d3e64896a679a00363bb1e822
BUG: 815242
Signed-off-by: Anand Avati <avati@redhat.com>
Reviewed-on: http://review.gluster.com/3210
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Raghavendra Bhat <raghavendrabhat@gluster.com>
Reviewed-by: Vijay Bellur <vijay@gluster.com>

1 files changed, 4 insertions, 29 deletions

diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c
index 7a50fe22861..fed23ae3539 100644
--- a/libglusterfs/src/inode.c
+++ b/libglusterfs/src/inode.c
@@ -1571,10 +1571,6 @@ inode_dump (inode_t *inode, char *prefix)
         int                i         = 0;
         fd_t              *fd        = NULL;
         struct _inode_ctx *inode_ctx = NULL;
-        struct  fd_wrapper {
-                fd_t *fd;
-                struct list_head next;
-        } *fd_wrapper, *tmp;
         struct list_head fd_list;
 
         if (!inode)
@@ -1605,21 +1601,12 @@ inode_dump (inode_t *inode, char *prefix)
                         }
                 }
 
-                if (list_empty (&inode->fd_list)) {
-                        goto unlock;
-                }
-
-                list_for_each_entry (fd, &inode->fd_list, inode_list) {
-                        fd_wrapper = GF_CALLOC (1, sizeof (*fd_wrapper),
-                                                gf_common_mt_char);
-                        if (fd_wrapper == NULL) {
-                                goto unlock;
-                        }
+		if (dump_options.xl_options.dump_fdctx != _gf_true)
+			goto unlock;
 
-                        INIT_LIST_HEAD (&fd_wrapper->next);
-                        list_add_tail (&fd_wrapper->next, &fd_list);
 
-                        fd_wrapper->fd = __fd_ref (fd);
+                list_for_each_entry (fd, &inode->fd_list, inode_list) {
+                        fd_ctx_dump (fd, prefix);
                 }
         }
 unlock:
@@ -1635,18 +1622,6 @@ unlock:
                 }
         }
 
-        if (!list_empty (&fd_list)
-            && (dump_options.xl_options.dump_fdctx == _gf_true)) {
-                list_for_each_entry_safe (fd_wrapper, tmp, &fd_list,
-                                          next) {
-                        list_del (&fd_wrapper->next);
-                        fd_ctx_dump (fd_wrapper->fd, prefix);
-
-                        fd_unref (fd_wrapper->fd);
-                        GF_FREE (fd_wrapper);
-                }
-        }
-
         if (inode_ctx != NULL) {
                 GF_FREE (inode_ctx);
         }