From a5d00b8f38ac970d39d1cc9b90f4a01eff651ce3 Mon Sep 17 00:00:00 2001
From: Pranith Kumar K <pkarampu@redhat.com>
Date: Thu, 8 Dec 2016 14:53:04 +0530
Subject: [PATCH 241/246] cluster/ec: Fix lk-owner set race in ec_unlock
Problem:
Rename does two locks. There is a case where when it tries to unlock it sends
xattrop of the directory with new version, callback of these two xattrops can
be picked up by two separate epoll threads. Both of them will try to set the
lk-owner for unlock in parallel on the same frame so one of these unlocks will
fail because the lk-owner doesn't match.
Fix:
Specify the lk-owner which will be set on inodelk frame which will not be over
written by any other thread/operation.
>BUG: 1402710
>Change-Id: I666ffc931440dc5253d72df666efe0ef1d73f99a
>Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
>Reviewed-on: http://review.gluster.org/16074
>Reviewed-by: Xavier Hernandez <xhernandez@datalab.es>
>Smoke: Gluster Build System <jenkins@build.gluster.org>
>NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
>CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
BUG: 1400093
Change-Id: I8e593308dfa20b7d4b6bbded669307ab20533073
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/92849
---
libglusterfs/src/lkowner.h | 6 ++++++
xlators/cluster/ec/src/ec-common.c | 14 ++++++++------
xlators/cluster/ec/src/ec-fops.h | 16 ++++++++--------
xlators/cluster/ec/src/ec-heal.c | 9 ++++++---
xlators/cluster/ec/src/ec-helpers.c | 5 ++---
xlators/cluster/ec/src/ec-locks.c | 24 ++++++++++++++----------
xlators/cluster/ec/src/ec.c | 8 ++++----
7 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/libglusterfs/src/lkowner.h b/libglusterfs/src/lkowner.h
index b6a950f..9712f17 100644
--- a/libglusterfs/src/lkowner.h
+++ b/libglusterfs/src/lkowner.h
@@ -84,4 +84,10 @@ out:
return is_null;
}
+static inline void
+lk_owner_copy (gf_lkowner_t *dst, gf_lkowner_t *src)
+{
+ dst->len = src->len;
+ memcpy(dst->data, src->data, src->len);
+}
#endif /* _LK_OWNER_H */
diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
index 6ff87ad..6688ac1 100644
--- a/xlators/cluster/ec/src/ec-common.c
+++ b/xlators/cluster/ec/src/ec-common.c
@@ -1473,20 +1473,21 @@ gf_boolean_t ec_lock_acquire(ec_lock_link_t *link)
{
ec_lock_t *lock;
ec_fop_data_t *fop;
+ gf_lkowner_t lk_owner;
lock = link->lock;
fop = link->fop;
if (!lock->acquired) {
- ec_owner_set(fop->frame, lock);
+ set_lk_owner_from_ptr(&lk_owner, lock);
ec_trace("LOCK_ACQUIRE", fop, "lock=%p, inode=%p", lock,
lock->loc.inode);
lock->flock.l_type = F_WRLCK;
- ec_inodelk(fop->frame, fop->xl, -1, EC_MINIMUM_ALL, ec_locked,
- link, fop->xl->name, &lock->loc, F_SETLKW, &lock->flock,
- NULL);
+ ec_inodelk(fop->frame, fop->xl, &lk_owner, -1, EC_MINIMUM_ALL,
+ ec_locked, link, fop->xl->name, &lock->loc, F_SETLKW,
+ &lock->flock, NULL);
return _gf_false;
}
@@ -1785,6 +1786,7 @@ void ec_unlock_lock(ec_lock_link_t *link)
{
ec_lock_t *lock;
ec_fop_data_t *fop;
+ gf_lkowner_t lk_owner;
lock = link->lock;
fop = link->fop;
@@ -1793,12 +1795,12 @@ void ec_unlock_lock(ec_lock_link_t *link)
ec_clear_inode_info(fop, lock->loc.inode);
if ((lock->mask != 0) && lock->acquired) {
- ec_owner_set(fop->frame, lock);
+ set_lk_owner_from_ptr(&lk_owner, lock);
lock->flock.l_type = F_UNLCK;
ec_trace("UNLOCK_INODELK", fop, "lock=%p, inode=%p", lock,
lock->loc.inode);
- ec_inodelk(fop->frame, fop->xl, lock->mask, EC_MINIMUM_ONE,
+ ec_inodelk(fop->frame, fop->xl, &lk_owner, lock->mask, EC_MINIMUM_ONE,
ec_unlocked, link, fop->xl->name, &lock->loc, F_SETLK,
&lock->flock, NULL);
} else {
diff --git a/xlators/cluster/ec/src/ec-fops.h b/xlators/cluster/ec/src/ec-fops.h
index ca6d6b3..053c6b4 100644
--- a/xlators/cluster/ec/src/ec-fops.h
+++ b/xlators/cluster/ec/src/ec-fops.h
@@ -63,16 +63,16 @@ void ec_fheal(call_frame_t * frame, xlator_t * this, uintptr_t target,
int32_t minimum, fop_fheal_cbk_t func, void *data, fd_t * fd,
int32_t partial, dict_t *xdata);
-void ec_inodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
- int32_t minimum, fop_inodelk_cbk_t func, void *data,
- const char * volume, loc_t * loc, int32_t cmd,
- struct gf_flock * flock, dict_t * xdata);
-
-void ec_finodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
- int32_t minimum, fop_finodelk_cbk_t func, void *data,
- const char * volume, fd_t * fd, int32_t cmd,
+void ec_inodelk (call_frame_t *frame, xlator_t *this, gf_lkowner_t *owner,
+ uintptr_t target, int32_t minimum, fop_inodelk_cbk_t func,
+ void *data, const char *volume, loc_t *loc, int32_t cmd,
struct gf_flock * flock, dict_t * xdata);
+void ec_finodelk(call_frame_t *frame, xlator_t *this, gf_lkowner_t *owner,
+ uintptr_t target, int32_t minimum, fop_finodelk_cbk_t func,
+ void *data, const char *volume, fd_t *fd, int32_t cmd,
+ struct gf_flock *flock, dict_t *xdata);
+
void ec_link(call_frame_t * frame, xlator_t * this, uintptr_t target,
int32_t minimum, fop_link_cbk_t func, void *data, loc_t * oldloc,
loc_t * newloc, dict_t * xdata);
diff --git a/xlators/cluster/ec/src/ec-heal.c b/xlators/cluster/ec/src/ec-heal.c
index f86b2cd..8503b6d 100644
--- a/xlators/cluster/ec/src/ec-heal.c
+++ b/xlators/cluster/ec/src/ec-heal.c
@@ -177,14 +177,17 @@ void ec_heal_lock(ec_heal_t *heal, int32_t type, fd_t *fd, loc_t *loc,
if (fd != NULL)
{
- ec_finodelk(heal->fop->frame, heal->xl, heal->fop->mask,
+ ec_finodelk(heal->fop->frame, heal->xl,
+ &heal->fop->frame->root->lk_owner, heal->fop->mask,
EC_MINIMUM_ALL, cbk, heal, heal->xl->name, fd, F_SETLKW,
&flock, NULL);
}
else
{
- ec_inodelk(heal->fop->frame, heal->xl, heal->fop->mask, EC_MINIMUM_ALL,
- cbk, heal, heal->xl->name, loc, F_SETLKW, &flock, NULL);
+ ec_inodelk(heal->fop->frame, heal->xl,
+ &heal->fop->frame->root->lk_owner, heal->fop->mask,
+ EC_MINIMUM_ALL, cbk, heal, heal->xl->name, loc, F_SETLKW,
+ &flock, NULL);
}
}
diff --git a/xlators/cluster/ec/src/ec-helpers.c b/xlators/cluster/ec/src/ec-helpers.c
index 7cf8232..7df8312 100644
--- a/xlators/cluster/ec/src/ec-helpers.c
+++ b/xlators/cluster/ec/src/ec-helpers.c
@@ -643,10 +643,9 @@ void ec_owner_set(call_frame_t * frame, void * owner)
set_lk_owner_from_ptr(&frame->root->lk_owner, owner);
}
-void ec_owner_copy(call_frame_t * frame, gf_lkowner_t * owner)
+void ec_owner_copy(call_frame_t *frame, gf_lkowner_t *owner)
{
- frame->root->lk_owner.len = owner->len;
- memcpy(frame->root->lk_owner.data, owner->data, owner->len);
+ lk_owner_copy (&frame->root->lk_owner, owner);
}
ec_inode_t * __ec_inode_get(inode_t * inode, xlator_t * xl)
diff --git a/xlators/cluster/ec/src/ec-locks.c b/xlators/cluster/ec/src/ec-locks.c
index ed835f1..bd52572 100644
--- a/xlators/cluster/ec/src/ec-locks.c
+++ b/xlators/cluster/ec/src/ec-locks.c
@@ -608,12 +608,14 @@ int32_t ec_manager_inodelk(ec_fop_data_t * fop, int32_t state)
flock.l_owner.len = 0;
if (fop->id == GF_FOP_INODELK) {
- ec_inodelk(fop->frame, fop->xl, mask, 1,
+ ec_inodelk(fop->frame, fop->xl,
+ &fop->frame->root->lk_owner, mask, 1,
ec_lock_unlocked, NULL, fop->str[0],
&fop->loc[0], F_SETLK, &flock,
fop->xdata);
} else {
- ec_finodelk(fop->frame, fop->xl, mask, 1,
+ ec_finodelk(fop->frame, fop->xl,
+ &fop->frame->root->lk_owner, mask, 1,
ec_lock_unlocked, NULL, fop->str[0],
fop->fd, F_SETLK, &flock, fop->xdata);
}
@@ -692,10 +694,10 @@ int32_t ec_manager_inodelk(ec_fop_data_t * fop, int32_t state)
}
}
-void ec_inodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
- int32_t minimum, fop_inodelk_cbk_t func, void * data,
- const char * volume, loc_t * loc, int32_t cmd,
- struct gf_flock * flock, dict_t * xdata)
+void ec_inodelk (call_frame_t *frame, xlator_t *this, gf_lkowner_t *owner,
+ uintptr_t target, int32_t minimum, fop_inodelk_cbk_t func,
+ void *data, const char *volume, loc_t *loc, int32_t cmd,
+ struct gf_flock *flock, dict_t *xdata)
{
ec_cbk_t callback = { .inodelk = func };
ec_fop_data_t * fop = NULL;
@@ -715,6 +717,7 @@ void ec_inodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
}
fop->int32 = cmd;
+ ec_owner_copy (fop->frame, owner);
if (volume != NULL) {
fop->str[0] = gf_strdup(volume);
@@ -828,10 +831,10 @@ void ec_wind_finodelk(ec_t * ec, ec_fop_data_t * fop, int32_t idx)
fop->xdata);
}
-void ec_finodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
- int32_t minimum, fop_finodelk_cbk_t func, void * data,
- const char * volume, fd_t * fd, int32_t cmd,
- struct gf_flock * flock, dict_t * xdata)
+void ec_finodelk(call_frame_t *frame, xlator_t *this, gf_lkowner_t *owner,
+ uintptr_t target, int32_t minimum, fop_finodelk_cbk_t func,
+ void *data, const char *volume, fd_t *fd, int32_t cmd,
+ struct gf_flock *flock, dict_t *xdata)
{
ec_cbk_t callback = { .finodelk = func };
ec_fop_data_t * fop = NULL;
@@ -853,6 +856,7 @@ void ec_finodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
fop->use_fd = 1;
fop->int32 = cmd;
+ ec_owner_copy (fop->frame, owner);
if (volume != NULL) {
fop->str[0] = gf_strdup(volume);
diff --git a/xlators/cluster/ec/src/ec.c b/xlators/cluster/ec/src/ec.c
index 63fb0dc..61a28ed 100644
--- a/xlators/cluster/ec/src/ec.c
+++ b/xlators/cluster/ec/src/ec.c
@@ -851,8 +851,8 @@ int32_t ec_gf_inodelk(call_frame_t * frame, xlator_t * this,
if (flock->l_type == F_UNLCK)
minimum = EC_MINIMUM_ONE;
- ec_inodelk(frame, this, -1, minimum, default_inodelk_cbk, NULL,
- volume, loc, cmd, flock, xdata);
+ ec_inodelk(frame, this, &frame->root->lk_owner, -1, minimum,
+ default_inodelk_cbk, NULL, volume, loc, cmd, flock, xdata);
return 0;
}
@@ -864,8 +864,8 @@ int32_t ec_gf_finodelk(call_frame_t * frame, xlator_t * this,
int32_t minimum = EC_MINIMUM_ALL;
if (flock->l_type == F_UNLCK)
minimum = EC_MINIMUM_ONE;
- ec_finodelk(frame, this, -1, minimum, default_finodelk_cbk, NULL,
- volume, fd, cmd, flock, xdata);
+ ec_finodelk(frame, this, &frame->root->lk_owner, -1, minimum,
+ default_finodelk_cbk, NULL, volume, fd, cmd, flock, xdata);
return 0;
}
--
2.9.3