diff options
author | Pranith Kumar K <pkarampu@redhat.com> | 2019-06-20 17:05:49 +0530 |
---|---|---|
committer | hari gowtham <hari.gowtham005@gmail.com> | 2020-02-24 07:55:55 +0000 |
commit | ab30da4ff9d26935fa157b2732b9f3c36dd2519e (patch) | |
tree | 01bbcce5adf759b7c64b3dff1b8b461eb60a39f4 /xlators | |
parent | d6584baa65c73577b29b99f47ac9820b3b8b3b00 (diff) |
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
Fixes: bz#1805050
Change-Id: Icac2ab39c0b1e7e154387800fbededc561612865
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Diffstat (limited to 'xlators')
-rw-r--r-- | xlators/cluster/ec/src/ec-common.c | 13 |
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 bceff105d93..d273a15ef9f 100644 --- a/xlators/cluster/ec/src/ec-common.c +++ b/xlators/cluster/ec/src/ec-common.c @@ -1403,6 +1403,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. */ @@ -1410,20 +1414,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 @@ -1434,6 +1434,7 @@ ec_get_size_version(ec_lock_link_t *link) GF_ASSERT(!changed_flags); } +unlock: UNLOCK(&lock->loc.inode->lock); if (!changed_flags) |