diff options
author | Poornima G <pgurusid@redhat.com> | 2017-02-15 11:18:31 +0530 |
---|---|---|
committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2017-02-20 07:25:29 -0500 |
commit | d10c5375b33520f36fd6acbd47b617d43f529ca2 (patch) | |
tree | 40d8b2f2385e74431fa03bcd4cf5913b8c40e00d /libglusterfs | |
parent | a0f6d7c775fb0de4beff0e326f2865353207b8db (diff) |
libglusterfs: Fix a crash due to race between inode_ctx_set and inode_ref
Issue:
Currently inode ref count is guarded by inode_table->lock, and
inode_ctx is guarded by inode->lock. With the new patch [1]
inode_ref was modified to change the inode_ctx to track the ref
count per xlator. Thus inode_ref performed under inode_table->lock
is modifying inode_ctx which has to be modified only under inode->lock
Solution:
When a inode is created, inode_ctx holder is allocated for all the xlators.
Hence in case of inode_ctx_set instead of using the first free index in
inode ctx holder, we can have predecided index for every xlator in the graph.
Credits Pranith K <pkarampu@redhat.com>
[1] http://review.gluster.org/13736
> Reviewed-on: https://review.gluster.org/16622
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
> Reviewed-by: Niels de Vos <ndevos@redhat.com>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Change-Id: I1bfe111c211fcc4fcd761bba01dc87c4c69b5170
BUG: 1423385
Signed-off-by: Poornima G <pgurusid@redhat.com>
Reviewed-on: https://review.gluster.org/16655
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
Diffstat (limited to 'libglusterfs')
-rw-r--r-- | libglusterfs/src/graph.c | 1 | ||||
-rw-r--r-- | libglusterfs/src/graph.y | 1 | ||||
-rw-r--r-- | libglusterfs/src/inode.c | 48 | ||||
-rw-r--r-- | libglusterfs/src/xlator.h | 3 |
4 files changed, 19 insertions, 34 deletions
diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c index 47f473a580f..0c7d6cf6101 100644 --- a/libglusterfs/src/graph.c +++ b/libglusterfs/src/graph.c @@ -111,6 +111,7 @@ glusterfs_graph_set_first (glusterfs_graph_t *graph, xlator_t *xl) graph->first = xl; graph->xl_count++; + xl->xl_id = graph->xl_count; } diff --git a/libglusterfs/src/graph.y b/libglusterfs/src/graph.y index e6a26058a06..7df3479d701 100644 --- a/libglusterfs/src/graph.y +++ b/libglusterfs/src/graph.y @@ -182,6 +182,7 @@ new_volume (char *name) construct->first = curr; construct->xl_count++; + curr->xl_id = construct->xl_count; gf_msg_trace ("parser", 0, "New node for '%s'", name); diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c index d39a2194921..573ec03819a 100644 --- a/libglusterfs/src/inode.c +++ b/libglusterfs/src/inode.c @@ -451,21 +451,15 @@ static int __inode_get_xl_index (inode_t *inode, xlator_t *xlator) { int set_idx = -1; - int index = 0; - - for (index = 0; index < inode->table->ctxcount; index++) { - if (!inode->_ctx[index].xl_key) { - if (set_idx == -1) - set_idx = index; - /* dont break, to check if key already exists - further on */ - } - if (inode->_ctx[index].xl_key == xlator) { - set_idx = index; - break; - } - } + if ((inode->_ctx[xlator->xl_id].xl_key != NULL) && + (inode->_ctx[xlator->xl_id].xl_key != xlator)) + goto out; + + set_idx = xlator->xl_id; + inode->_ctx[set_idx].xl_key = xlator; + +out: return set_idx; } @@ -2075,12 +2069,8 @@ __inode_ctx_get2 (inode_t *inode, xlator_t *xlator, uint64_t *value1, if (!inode || !xlator || !inode->_ctx) goto out; - for (index = 0; index < inode->table->ctxcount; index++) { - if (inode->_ctx[index].xl_key == xlator) - break; - } - - if (index == inode->table->ctxcount) + index = xlator->xl_id; + if (inode->_ctx[index].xl_key != xlator) goto out; if (inode->_ctx[index].value1) { @@ -2196,13 +2186,8 @@ inode_ctx_del2 (inode_t *inode, xlator_t *xlator, uint64_t *value1, if (!inode->_ctx) goto unlock; - for (index = 0; index < inode->table->ctxcount; - index++) { - if (inode->_ctx[index].xl_key == xlator) - break; - } - - if (index == inode->table->ctxcount) { + index = xlator->xl_id; + if (inode->_ctx[index].xl_key != xlator) { ret = -1; goto unlock; } @@ -2242,13 +2227,8 @@ __inode_ctx_reset2 (inode_t *inode, xlator_t *xlator, uint64_t *value1, LOCK (&inode->lock); { - for (index = 0; index < inode->table->ctxcount; - index++) { - if (inode->_ctx[index].xl_key == xlator) - break; - } - - if (index == inode->table->ctxcount) { + index = xlator->xl_id; + if (inode->_ctx[index].xl_key != xlator) { ret = -1; goto unlock; } diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h index 1e2698bb61f..c2959efbd95 100644 --- a/libglusterfs/src/xlator.h +++ b/libglusterfs/src/xlator.h @@ -953,6 +953,9 @@ struct _xlator { /* Saved volfile ID (used for multiplexing) */ char *volfile_id; + + /* Its used as an index to inode_ctx*/ + uint32_t xl_id; }; typedef struct { |