From 1fc54cf7c5e2a88cf8f59d98f6e0eb7df485ae80 Mon Sep 17 00:00:00 2001 From: Anand Avati Date: Mon, 23 Apr 2012 00:39:14 -0700 Subject: 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 Reviewed-on: http://review.gluster.com/3210 Tested-by: Gluster Build System Reviewed-by: Amar Tumballi Reviewed-by: Raghavendra Bhat Reviewed-by: Vijay Bellur --- libglusterfs/src/inode.c | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-) (limited to 'libglusterfs') diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c index 7a50fe22..fed23ae3 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); } -- cgit