diff options
author | Raghavendra Bhat <raghavendra@redhat.com> | 2019-04-30 15:38:55 -0400 |
---|---|---|
committer | Xavi Hernandez <xhernandez@redhat.com> | 2019-06-25 16:21:03 +0000 |
commit | d58594d802d617ed81b8f991fd312c224f1f85d4 (patch) | |
tree | a5db1a417f5c10c89b7ac23f64f4037878f2c546 /libglusterfs | |
parent | bee5d7bd8a602184c94c12115c7e30572eb7588c (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>
Diffstat (limited to 'libglusterfs')
-rw-r--r-- | libglusterfs/src/glusterfs/inode.h | 4 | ||||
-rw-r--r-- | libglusterfs/src/inode.c | 38 |
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 |