summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPranith Kumar K <pkarampu@redhat.com>2016-12-08 14:53:04 +0530
committerPranith Kumar Karampuri <pkarampu@redhat.com>2016-12-13 07:24:51 -0800
commita0a4163ce6a8dd8bb83b60a4484578fadd02c88f (patch)
treec7113d924d58a0d888fb16aaacd04c9b1245dfd0
parent212c7600d2070a4414bc89fd7d2c186b5994cd54 (diff)
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>
-rw-r--r--libglusterfs/src/lkowner.h6
-rw-r--r--xlators/cluster/ec/src/ec-common.c14
-rw-r--r--xlators/cluster/ec/src/ec-fops.h16
-rw-r--r--xlators/cluster/ec/src/ec-heal.c9
-rw-r--r--xlators/cluster/ec/src/ec-helpers.c5
-rw-r--r--xlators/cluster/ec/src/ec-locks.c24
-rw-r--r--xlators/cluster/ec/src/ec.c8
7 files changed, 48 insertions, 34 deletions
diff --git a/libglusterfs/src/lkowner.h b/libglusterfs/src/lkowner.h
index b6a950f5e12..9712f176f30 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 7cc1becfdb0..79bf2487216 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;
}
@@ -1784,6 +1785,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;
@@ -1792,12 +1794,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 395641cde08..4e17ec509fd 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 b135979af3c..1859d73d12b 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 2391b2de3ae..195bb6377fa 100644
--- a/xlators/cluster/ec/src/ec-helpers.c
+++ b/xlators/cluster/ec/src/ec-helpers.c
@@ -692,10 +692,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 ed835f1aadc..bd525723ddf 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 97786657071..677761256b4 100644
--- a/xlators/cluster/ec/src/ec.c
+++ b/xlators/cluster/ec/src/ec.c
@@ -878,8 +878,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;
}
@@ -891,8 +891,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;
}