From 28328417bff73b13392e95a07c3d359c881bebc8 Mon Sep 17 00:00:00 2001 From: Raghavendra Bhat Date: Thu, 9 Aug 2012 17:47:34 +0530 Subject: performance/io-cache: use pthread_mutex_trylock to hold mutex in statedumps Do not use pthread_mutex_lock and gf_log functions while dumping information to statedump, to avoid deadlocks. Change-Id: I6569366856fc2bc0fefb49c8379e2e4337717ce4 BUG: 843787 Signed-off-by: Raghavendra Bhat Reviewed-on: http://review.gluster.com/3799 Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/performance/io-cache/src/io-cache.c | 99 +++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 18 deletions(-) diff --git a/xlators/performance/io-cache/src/io-cache.c b/xlators/performance/io-cache/src/io-cache.c index 7e6e0fb2edb..ec45b61aed8 100644 --- a/xlators/performance/io-cache/src/io-cache.c +++ b/xlators/performance/io-cache/src/io-cache.c @@ -1798,7 +1798,16 @@ void __ioc_page_dump (ioc_page_t *page, char *prefix) { - ioc_page_lock (page); + int ret = -1; + + if (!page) + return; + /* ioc_page_lock can be used to hold the mutex. But in statedump + * its better to use trylock to avoid deadlocks. + */ + ret = pthread_mutex_trylock (&page->page_lock); + if (ret) + goto out; { gf_proc_dump_write ("offset", "%"PRId64, page->offset); gf_proc_dump_write ("size", "%"PRId64, page->size); @@ -1806,7 +1815,14 @@ __ioc_page_dump (ioc_page_t *page, char *prefix) gf_proc_dump_write ("ready", "%s", page->ready ? "yes" : "no"); ioc_page_waitq_dump (page, prefix); } - ioc_page_unlock (page); + pthread_mutex_unlock (&page->page_lock); + +out: + if (ret && page) + gf_proc_dump_write ("Unable to dump the page information", + "(Lock acquisition failed) %p", page); + + return; } void @@ -1851,40 +1867,76 @@ out: } -void -ioc_inode_dump (ioc_inode_t *ioc_inode, char *prefix) +int +ioc_inode_dump (xlator_t *this, inode_t *inode) { char *path = NULL; + int ret = -1; + ioc_inode_t *ioc_inode = NULL; + char key_prefix[GF_DUMP_MAX_BUF_LEN] = {0, }; + uint64_t tmp_ioc_inode = 0; + gf_boolean_t section_added = _gf_false; - if ((ioc_inode == NULL) || (prefix == NULL)) { + if (this == NULL || inode == NULL) { goto out; } - ioc_inode_lock (ioc_inode); + gf_proc_dump_build_key (key_prefix, "xlator.performance.io-cache", + "inode"); + + inode_ctx_get (inode, this, &tmp_ioc_inode); + ioc_inode = (ioc_inode_t *)(long)tmp_ioc_inode; + if (ioc_inode == NULL) + goto out; + + gf_proc_dump_add_section (key_prefix); + section_added = _gf_true; + + /* Similar to ioc_page_dump function its better to use + * pthread_mutex_trylock and not to use gf_log in statedump + * to avoid deadlocks. + */ + ret = pthread_mutex_trylock (&ioc_inode->inode_lock); + if (ret) + goto out; + else { gf_proc_dump_write ("inode.weight", "%d", ioc_inode->weight); - inode_path (ioc_inode->inode, NULL, &path); + + //inode_path takes blocking lock on the itable. + __inode_path (ioc_inode->inode, NULL, &path); + if (path) { gf_proc_dump_write ("path", "%s", path); GF_FREE (path); } gf_proc_dump_write ("uuid", "%s", uuid_utoa (ioc_inode->inode->gfid)); - __ioc_cache_dump (ioc_inode, prefix); - __ioc_inode_waitq_dump (ioc_inode, prefix); + __ioc_cache_dump (ioc_inode, key_prefix); + __ioc_inode_waitq_dump (ioc_inode, key_prefix); + + pthread_mutex_unlock (&ioc_inode->inode_lock); } - ioc_inode_unlock (ioc_inode); + out: - return; + if (ret && ioc_inode) { + if (section_added == _gf_false) + gf_proc_dump_add_section (key_prefix); + gf_proc_dump_write ("Unable to print the status of ioc_inode", + "(Lock acquisition failed) %s", + uuid_utoa (inode->gfid)); + } + return ret; } int ioc_priv_dump (xlator_t *this) { ioc_table_t *priv = NULL; - ioc_inode_t *ioc_inode = NULL; char key_prefix[GF_DUMP_MAX_BUF_LEN] = {0, }; + int ret = -1; + gf_boolean_t add_section = _gf_false; if (!this || !this->private) goto out; @@ -1893,8 +1945,11 @@ ioc_priv_dump (xlator_t *this) gf_proc_dump_build_key (key_prefix, "xlator.performance.io-cache", "priv"); gf_proc_dump_add_section (key_prefix); + add_section = _gf_true; - ioc_table_lock (priv); + ret = pthread_mutex_trylock (&priv->table_lock); + if (ret) + goto out; { gf_proc_dump_write ("page_size", "%ld", priv->page_size); gf_proc_dump_write ("cache_size", "%ld", priv->cache_size); @@ -1903,13 +1958,20 @@ ioc_priv_dump (xlator_t *this) gf_proc_dump_write ("cache_timeout", "%u", priv->cache_timeout); gf_proc_dump_write ("min-file-size", "%u", priv->min_file_size); gf_proc_dump_write ("max-file-size", "%u", priv->max_file_size); - - list_for_each_entry (ioc_inode, &priv->inodes, inode_list) { - ioc_inode_dump (ioc_inode, key_prefix); - } } - ioc_table_unlock (priv); + pthread_mutex_unlock (&priv->table_lock); out: + if (ret && priv) { + if (!add_section) { + gf_proc_dump_build_key (key_prefix, "xlator." + "performance.io-cache", "priv"); + gf_proc_dump_add_section (key_prefix); + } + gf_proc_dump_write ("Unable to dump the state of private " + "structure of io-cache xlator", "(Lock " + "acquisition failed) %s", this->name); + } + return 0; } @@ -1974,6 +2036,7 @@ struct xlator_fops fops = { struct xlator_dumpops dumpops = { .priv = ioc_priv_dump, + .inodectx = ioc_inode_dump, }; struct xlator_cbks cbks = { -- cgit