cluster/ec: Prevent double pre-op xattrops

Problem:
Race:
Thread-1                                    Thread-2
1) Does ec_get_size_version() to perform
pre-op fxattrop as part of write-1
                                           2) Calls ec_set_dirty_flag() in
                                              ec_get_size_version() for write-2.
					      This sets dirty[] to 1
3) Completes executing
ec_prepare_update_cbk leading to
ctx->dirty[] = '1'
					   4) Takes LOCK(inode->lock) to check if there are
					      any flags and sets dirty-flag because
				              lock->waiting_flag is 0 now. This leads to
					      fxattrop to increment on-disk dirty[] to '2'

At the end of the writes the file will be marked for heal even when it doesn't need heal.

Fix:
Perform ec_set_dirty_flag() and other checks inside LOCK() to prevent dirty[] to be marked
as '1' in step 2) above

Updates bz#1739446
Change-Id: Icac2ab39c0b1e7e154387800fbededc561612865
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>

1 files changed, 7 insertions, 6 deletions

diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
index e85aa8bf43f..e2e582f6b27 100644
--- a/xlators/cluster/ec/src/ec-common.c
+++ b/xlators/cluster/ec/src/ec-common.c
@@ -1405,6 +1405,10 @@ ec_get_size_version(ec_lock_link_t *link)
         !ec_is_data_fop(fop->id))
         link->optimistic_changelog = _gf_true;
 
+    memset(&loc, 0, sizeof(loc));
+
+    LOCK(&lock->loc.inode->lock);
+
     set_dirty = ec_set_dirty_flag(link, ctx, dirty);
 
     /* If ec metadata has already been retrieved, do not try again. */
@@ -1412,20 +1416,16 @@ ec_get_size_version(ec_lock_link_t *link)
         if (ec_is_data_fop(fop->id)) {
             fop->healing |= lock->healing;
         }
-        return;
+        goto unlock;
     }
 
     /* Determine if there's something we need to retrieve for the current
      * operation. */
     if (!set_dirty && !lock->query && (lock->loc.inode->ia_type != IA_IFREG) &&
         (lock->loc.inode->ia_type != IA_INVAL)) {
-        return;
+        goto unlock;
     }
 
-    memset(&loc, 0, sizeof(loc));
-
-    LOCK(&lock->loc.inode->lock);
-
     changed_flags = ec_set_xattrop_flags_and_params(lock, link, dirty);
     if (link->waiting_flags) {
         /* This fop needs to wait until all its flags are cleared which
@@ -1436,6 +1436,7 @@ ec_get_size_version(ec_lock_link_t *link)
         GF_ASSERT(!changed_flags);
     }
 
+unlock:
     UNLOCK(&lock->loc.inode->lock);
 
     if (!changed_flags)