cluster/ec: Correctly cleanup delayed locks

When a delayed lock is pending, a graph switch doesn't correctly
terminate it. This means that the update of version and size xattrs
is lost, causing EIO errors.

This patch handles GF_EVENT_PARENT_DOWN event to correctly finish
pending udpdates before completing the graph switch.

Change-Id: I394f3b8d41df8d83cdd36636aeb62330f30a66d5
BUG: 1188145
Signed-off-by: Xavier Hernandez <xhernandez@datalab.es>
Reviewed-on: http://review.gluster.org/10787
Tested-by: NetBSD Build System
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

1 files changed, 57 insertions, 17 deletions

diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
index afd46c095f3..9f312e0c37c 100644
--- a/xlators/cluster/ec/src/ec-common.c
+++ b/xlators/cluster/ec/src/ec-common.c
@@ -1531,31 +1531,46 @@ void ec_unlock_now(ec_fop_data_t *fop, ec_lock_t *lock)
     ec_resume(fop, 0);
 }
 
-void ec_unlock_timer_cbk(void *data)
+void
+ec_unlock_timer_del(ec_fop_data_t *fop, ec_lock_t *lock)
 {
-    ec_lock_link_t *link = data;
-    ec_lock_t *lock = link->lock;
-    ec_fop_data_t *fop = NULL;
+        inode_t *inode;
+        gf_boolean_t now = _gf_false;
+
+        /* A race condition can happen if timer expires, calls this function
+         * and the lock is released (lock->loc is wiped) but the fop is not
+         * fully completed yet (it's still on the list of pending fops). In
+         * this case, this function can also be called if ec_unlock_force() is
+         * called. */
+        inode = lock->loc.inode;
+        if (inode == NULL) {
+                return;
+        }
 
-    LOCK(&lock->loc.inode->lock);
+        LOCK(&inode->lock);
 
-    if (lock->timer != NULL) {
-        fop = link->fop;
+        if (lock->timer != NULL) {
+                ec_trace("UNLOCK_DELAYED", fop, "lock=%p", lock);
 
-        ec_trace("UNLOCK_DELAYED", fop, "lock=%p", lock);
+                gf_timer_call_cancel(fop->xl->ctx, lock->timer);
+                lock->timer = NULL;
+                *lock->plock = NULL;
 
-        GF_ASSERT(lock->refs == 1);
+                now = _gf_true;
+        }
 
-        gf_timer_call_cancel(fop->xl->ctx, lock->timer);
-        lock->timer = NULL;
-        *lock->plock = NULL;
-    }
+        UNLOCK(&inode->lock);
 
-    UNLOCK(&lock->loc.inode->lock);
+        if (now) {
+                ec_unlock_now(fop, lock);
+        }
+}
 
-    if (fop != NULL) {
-        ec_unlock_now(fop, lock);
-    }
+void ec_unlock_timer_cbk(void *data)
+{
+        ec_lock_link_t *link = data;
+
+        ec_unlock_timer_del(link->fop, link->lock);
 }
 
 void ec_unlock_timer_add(ec_lock_link_t *link)
@@ -1626,6 +1641,18 @@ void ec_unlock(ec_fop_data_t *fop)
     }
 }
 
+void
+ec_unlock_force(ec_fop_data_t *fop)
+{
+        int32_t i;
+
+        for (i = 0; i < fop->lock_count; i++) {
+                ec_trace("UNLOCK_FORCED", fop, "lock=%p", &fop->locks[i]);
+
+                ec_unlock_timer_del(fop, fop->locks[i].lock);
+        }
+}
+
 void ec_flush_size_version(ec_fop_data_t * fop)
 {
     ec_lock_t * lock;
@@ -1740,8 +1767,21 @@ void __ec_manager(ec_fop_data_t * fop, int32_t error)
         }
 
         if ((fop->state == EC_STATE_END) || (fop->state == -EC_STATE_END)) {
+            gf_boolean_t notify;
+
+            LOCK(&ec->lock);
+
+            list_del_init(&fop->pending_list);
+            notify = list_empty(&ec->pending_fops);
+
+            UNLOCK(&ec->lock);
+
             ec_fop_data_release(fop);
 
+            if (notify) {
+                ec_pending_fops_completed(ec);
+            }
+
             break;
         }