diff options
| author | Xavier Hernandez <xhernandez@datalab.es> | 2014-12-03 16:42:31 +0100 | 
|---|---|---|
| committer | Raghavendra Bhat <raghavendra@redhat.com> | 2015-01-04 22:58:30 -0800 | 
| commit | 8bfe216b8297129833d3af5a66984b9b9bf0046c (patch) | |
| tree | dab61920cc7fe3b92a837c72240d47eb249158b2 | |
| parent | 0111ab923dd4c8f0985dd566c172cda31492b0f7 (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.c | 14 | ||||
| -rw-r--r-- | xlators/cluster/ec/src/ec-common.c | 16 | 
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;  | 
