summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavier Hernandez <xhernandez@datalab.es>2014-12-03 16:42:31 +0100
committerRaghavendra Bhat <raghavendra@redhat.com>2015-01-04 22:58:30 -0800
commit8bfe216b8297129833d3af5a66984b9b9bf0046c (patch)
treedab61920cc7fe3b92a837c72240d47eb249158b2
parent0111ab923dd4c8f0985dd566c172cda31492b0f7 (diff)
ec: Fix mutex related coverity scan issues
This patch solves 3 issues detected by coverity scan: CID1241484 Data race condition CID1241486 Data race condition CID1256173 Thread deadlock CID1257622 Thread deadlock With this patch, inode lock is never acquired inside a region locked with fop->lock. This is a backport of http://review.gluster.org/9230/ and http://review.gluster.org/9263/ Change-Id: I35c4633efd1b68b9f72b42661fa7c728b1f52c6a BUG: 1170954 Signed-off-by: Xavier Hernandez <xhernandez@datalab.es> Reviewed-on: http://review.gluster.org/9244 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Dan Lambright <dlambrig@redhat.com> Reviewed-by: Raghavendra Bhat <raghavendra@redhat.com>
-rw-r--r--xlators/cluster/ec/src/ec-combine.c14
-rw-r--r--xlators/cluster/ec/src/ec-common.c16
2 files changed, 19 insertions, 11 deletions
diff --git a/xlators/cluster/ec/src/ec-combine.c b/xlators/cluster/ec/src/ec-combine.c
index a98380adfb8..e605f7ba567 100644
--- a/xlators/cluster/ec/src/ec-combine.c
+++ b/xlators/cluster/ec/src/ec-combine.c
@@ -851,12 +851,9 @@ void ec_combine(ec_cbk_data_t * cbk, ec_combine_f combine)
ec_trace("ANSWER", fop, "combine=%s[%d]",
ec_bin(str, sizeof(str), cbk->mask, 0), cbk->count);
- if ((cbk->count == fop->expected) && (fop->answer == NULL))
- {
+ if ((cbk->count == fop->expected) && (fop->answer == NULL)) {
fop->answer = cbk;
- ec_update_bad(fop, cbk->mask);
-
resume = 1;
}
@@ -865,12 +862,11 @@ void ec_combine(ec_cbk_data_t * cbk, ec_combine_f combine)
UNLOCK(&fop->lock);
- if (needed > 0)
- {
+ if (needed > 0) {
ec_dispatch_next(fop, cbk->idx);
- }
- else if (resume)
- {
+ } else if (resume) {
+ ec_update_bad(fop, cbk->mask);
+
ec_resume(fop, 0);
}
}
diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
index 88cc103e513..dbf2238ea8e 100644
--- a/xlators/cluster/ec/src/ec-common.c
+++ b/xlators/cluster/ec/src/ec-common.c
@@ -322,7 +322,7 @@ void ec_resume_parent(ec_fop_data_t * fop, int32_t error)
void ec_complete(ec_fop_data_t * fop)
{
ec_cbk_data_t * cbk = NULL;
- int32_t resume = 0;
+ int32_t resume = 0, update = 0;
LOCK(&fop->lock);
@@ -336,7 +336,7 @@ void ec_complete(ec_fop_data_t * fop)
((cbk->op_ret >= 0) || (cbk->op_errno != ENOTCONN))) {
fop->answer = cbk;
- ec_update_bad(fop, cbk->mask);
+ update = 1;
}
}
@@ -350,6 +350,14 @@ void ec_complete(ec_fop_data_t * fop)
UNLOCK(&fop->lock);
+ /* ec_update_bad() locks inode->lock. This may cause deadlocks with
+ fop->lock when used in another order. Since ec_update_bad() will not
+ be called more than once for each fop, it can be called from outside
+ the fop->lock locked region. */
+ if (update) {
+ ec_update_bad(fop, cbk->mask);
+ }
+
if (resume)
{
ec_resume(fop, 0);
@@ -862,9 +870,13 @@ void ec_lock(ec_fop_data_t * fop)
list_add_tail(&fop->locks[fop->locked].wait_list, &lock->waiting);
+ LOCK(&fop->lock);
+
fop->jobs++;
fop->refs++;
+ UNLOCK(&fop->lock);
+
UNLOCK(&lock->loc.inode->lock);
break;