summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra Bhat <raghavendra@redhat.com>2019-04-30 15:38:55 -0400
committerXavi Hernandez <xhernandez@redhat.com>2019-06-25 16:21:03 +0000
commitd58594d802d617ed81b8f991fd312c224f1f85d4 (patch)
treea5db1a417f5c10c89b7ac23f64f4037878f2c546
parentbee5d7bd8a602184c94c12115c7e30572eb7588c (diff)
* core: do not assert in inode_unref if the inode table cleanup has started
There is a good chance that, the inode on which unref came has already been zero refed and added to the purge list. This can happen when inode table is being destroyed (glfs_fini is something which destroys the inode table). Consider a directory 'a' which has a file 'b'. Now as part of inode table destruction zero refing of inodes does not happen from leaf to the root. It happens in the order inodes are present in the list. So, in this example, the dentry of 'b' would have its parent set to the inode of 'a'. So if 'a' gets zero refed first (as part of inode table cleanup) and then 'b' has to zero refed, then dentry_unset is called on the dentry of 'b' and it further goes on to call inode_unref on b's parent which is 'a'. In this situation, GF_ASSERT would be called as the refcount of 'a' has been already set to zero. So, return the inode (in the function inode_unref without doing anything) if the inode table cleanup has already started and inode's refcount is zero. Change-Id: I91e0a807d5c9ce0daae5a611c38da379fd11076e fixes: bz#1722546 Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>
-rw-r--r--libglusterfs/src/glusterfs/inode.h4
-rw-r--r--libglusterfs/src/inode.c38
2 files changed, 42 insertions, 0 deletions
diff --git a/libglusterfs/src/glusterfs/inode.h b/libglusterfs/src/glusterfs/inode.h
index 5cf2ab5080b..424e50fd935 100644
--- a/libglusterfs/src/glusterfs/inode.h
+++ b/libglusterfs/src/glusterfs/inode.h
@@ -61,6 +61,10 @@ struct _inode_table {
xlator_t *invalidator_xl;
struct list_head invalidate; /* inodes which are in invalidation queue */
uint32_t invalidate_size; /* count of inodes in invalidation list */
+
+ /* flag to indicate whether the cleanup of the inode
+ table started or not */
+ gf_boolean_t cleanup_started;
};
struct _dentry {
diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c
index 3a1f097e1af..b5147f2010c 100644
--- a/libglusterfs/src/inode.c
+++ b/libglusterfs/src/inode.c
@@ -473,6 +473,41 @@ __inode_unref(inode_t *inode, bool clear)
if (__is_root_gfid(inode->gfid))
return inode;
+ /*
+ * No need to acquire inode table's lock
+ * as __inode_unref is called after acquiding
+ * the inode table's lock.
+ */
+ if (inode->table->cleanup_started && !inode->ref)
+ /*
+ * There is a good chance that, the inode
+ * on which unref came has already been
+ * zero refed and added to the purge list.
+ * This can happen when inode table is
+ * being destroyed (glfs_fini is something
+ * which destroys the inode table).
+ *
+ * Consider a directory 'a' which has a file
+ * 'b'. Now as part of inode table destruction
+ * zero refing of inodes does not happen from
+ * leaf to the root. It happens in the order
+ * inodes are present in the list. So, in this
+ * example, the dentry of 'b' would have its
+ * parent set to the inode of 'a'. So if
+ * 'a' gets zero refed first (as part of
+ * inode table cleanup) and then 'b' has to
+ * zero refed, then dentry_unset is called on
+ * the dentry of 'b' and it further goes on to
+ * call inode_unref on b's parent which is 'a'.
+ * In this situation, GF_ASSERT would be called
+ * below as the refcount of 'a' has been already set
+ * to zero.
+ *
+ * So return the inode if the inode table cleanup
+ * has already started and inode refcount is 0.
+ */
+ return inode;
+
this = THIS;
if (clear && inode->invalidate_sent) {
@@ -1679,6 +1714,8 @@ inode_table_with_invalidator(uint32_t lru_limit, xlator_t *xl,
;
}
+ new->cleanup_started = _gf_false;
+
__inode_table_init_root(new);
pthread_mutex_init(&new->lock, NULL);
@@ -1829,6 +1866,7 @@ inode_table_destroy(inode_table_t *inode_table)
*/
pthread_mutex_lock(&inode_table->lock);
{
+ inode_table->cleanup_started = _gf_true;
/* Process lru list first as we need to unset their dentry
* entries (the ones which may not be unset during
* '__inode_passivate' as they were hashed) which in turn