|
|
14f8ab |
From 9fd966aa6879ac9867381629f82eca24b950d731 Mon Sep 17 00:00:00 2001
|
|
|
14f8ab |
From: Mohammed Rafi KC <rkavunga@redhat.com>
|
|
|
14f8ab |
Date: Sun, 2 Jun 2019 01:36:33 +0530
|
|
|
14f8ab |
Subject: [PATCH 175/178] ec/fini: Fix race between xlator cleanup and on going
|
|
|
14f8ab |
async fop
|
|
|
14f8ab |
|
|
|
14f8ab |
Problem:
|
|
|
14f8ab |
While we process a cleanup, there is a chance for a race between
|
|
|
14f8ab |
async operations, for example ec_launch_replace_heal. So this can
|
|
|
14f8ab |
lead to invalid mem access.
|
|
|
14f8ab |
|
|
|
14f8ab |
Solution:
|
|
|
14f8ab |
Just like we track on going heal fops, we can also track fops like
|
|
|
14f8ab |
ec_launch_replace_heal, so that we can decide when to send a
|
|
|
14f8ab |
PARENT_DOWN request.
|
|
|
14f8ab |
|
|
|
14f8ab |
> upstream patch : https://review.gluster.org/#/c/glusterfs/+/22798/
|
|
|
14f8ab |
|
|
|
14f8ab |
>Change-Id: I055391c5c6c34d58aef7336847f3b570cb831298
|
|
|
14f8ab |
>fixes: bz#1703948
|
|
|
14f8ab |
>Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
|
|
|
14f8ab |
|
|
|
14f8ab |
Change-Id: I055391c5c6c34d58aef7336847f3b570cb831298
|
|
|
14f8ab |
BUG: 1714588
|
|
|
14f8ab |
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
|
|
|
14f8ab |
Reviewed-on: https://code.engineering.redhat.com/gerrit/172801
|
|
|
14f8ab |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
14f8ab |
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
|
|
|
14f8ab |
---
|
|
|
14f8ab |
xlators/cluster/ec/src/ec-common.c | 10 ++++++++++
|
|
|
14f8ab |
xlators/cluster/ec/src/ec-common.h | 2 ++
|
|
|
14f8ab |
xlators/cluster/ec/src/ec-data.c | 4 +++-
|
|
|
14f8ab |
xlators/cluster/ec/src/ec-heal.c | 17 +++++++++++++++--
|
|
|
14f8ab |
xlators/cluster/ec/src/ec-types.h | 1 +
|
|
|
14f8ab |
xlators/cluster/ec/src/ec.c | 37 +++++++++++++++++++++++++------------
|
|
|
14f8ab |
6 files changed, 56 insertions(+), 15 deletions(-)
|
|
|
14f8ab |
|
|
|
14f8ab |
diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
|
|
|
14f8ab |
index e85aa8b..9cc6395 100644
|
|
|
14f8ab |
--- a/xlators/cluster/ec/src/ec-common.c
|
|
|
14f8ab |
+++ b/xlators/cluster/ec/src/ec-common.c
|
|
|
14f8ab |
@@ -2955,3 +2955,13 @@ ec_manager(ec_fop_data_t *fop, int32_t error)
|
|
|
14f8ab |
|
|
|
14f8ab |
__ec_manager(fop, error);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
+
|
|
|
14f8ab |
+gf_boolean_t
|
|
|
14f8ab |
+__ec_is_last_fop(ec_t *ec)
|
|
|
14f8ab |
+{
|
|
|
14f8ab |
+ if ((list_empty(&ec->pending_fops)) &&
|
|
|
14f8ab |
+ (GF_ATOMIC_GET(ec->async_fop_count) == 0)) {
|
|
|
14f8ab |
+ return _gf_true;
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
+ return _gf_false;
|
|
|
14f8ab |
+}
|
|
|
14f8ab |
diff --git a/xlators/cluster/ec/src/ec-common.h b/xlators/cluster/ec/src/ec-common.h
|
|
|
14f8ab |
index e948342..bf6c97d 100644
|
|
|
14f8ab |
--- a/xlators/cluster/ec/src/ec-common.h
|
|
|
14f8ab |
+++ b/xlators/cluster/ec/src/ec-common.h
|
|
|
14f8ab |
@@ -204,4 +204,6 @@ void
|
|
|
14f8ab |
ec_reset_entry_healing(ec_fop_data_t *fop);
|
|
|
14f8ab |
char *
|
|
|
14f8ab |
ec_msg_str(ec_fop_data_t *fop);
|
|
|
14f8ab |
+gf_boolean_t
|
|
|
14f8ab |
+__ec_is_last_fop(ec_t *ec);
|
|
|
14f8ab |
#endif /* __EC_COMMON_H__ */
|
|
|
14f8ab |
diff --git a/xlators/cluster/ec/src/ec-data.c b/xlators/cluster/ec/src/ec-data.c
|
|
|
14f8ab |
index 6ef9340..8d2d9a1 100644
|
|
|
14f8ab |
--- a/xlators/cluster/ec/src/ec-data.c
|
|
|
14f8ab |
+++ b/xlators/cluster/ec/src/ec-data.c
|
|
|
14f8ab |
@@ -202,11 +202,13 @@ ec_handle_last_pending_fop_completion(ec_fop_data_t *fop, gf_boolean_t *notify)
|
|
|
14f8ab |
{
|
|
|
14f8ab |
ec_t *ec = fop->xl->private;
|
|
|
14f8ab |
|
|
|
14f8ab |
+ *notify = _gf_false;
|
|
|
14f8ab |
+
|
|
|
14f8ab |
if (!list_empty(&fop->pending_list)) {
|
|
|
14f8ab |
LOCK(&ec->lock);
|
|
|
14f8ab |
{
|
|
|
14f8ab |
list_del_init(&fop->pending_list);
|
|
|
14f8ab |
- *notify = list_empty(&ec->pending_fops);
|
|
|
14f8ab |
+ *notify = __ec_is_last_fop(ec);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
UNLOCK(&ec->lock);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
diff --git a/xlators/cluster/ec/src/ec-heal.c b/xlators/cluster/ec/src/ec-heal.c
|
|
|
14f8ab |
index 8844c29..237fea2 100644
|
|
|
14f8ab |
--- a/xlators/cluster/ec/src/ec-heal.c
|
|
|
14f8ab |
+++ b/xlators/cluster/ec/src/ec-heal.c
|
|
|
14f8ab |
@@ -2814,8 +2814,20 @@ int
|
|
|
14f8ab |
ec_replace_heal_done(int ret, call_frame_t *heal, void *opaque)
|
|
|
14f8ab |
{
|
|
|
14f8ab |
ec_t *ec = opaque;
|
|
|
14f8ab |
+ gf_boolean_t last_fop = _gf_false;
|
|
|
14f8ab |
|
|
|
14f8ab |
+ if (GF_ATOMIC_DEC(ec->async_fop_count) == 0) {
|
|
|
14f8ab |
+ LOCK(&ec->lock);
|
|
|
14f8ab |
+ {
|
|
|
14f8ab |
+ last_fop = __ec_is_last_fop(ec);
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
+ UNLOCK(&ec->lock);
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
gf_msg_debug(ec->xl->name, 0, "getxattr on bricks is done ret %d", ret);
|
|
|
14f8ab |
+
|
|
|
14f8ab |
+ if (last_fop)
|
|
|
14f8ab |
+ ec_pending_fops_completed(ec);
|
|
|
14f8ab |
+
|
|
|
14f8ab |
return 0;
|
|
|
14f8ab |
}
|
|
|
14f8ab |
|
|
|
14f8ab |
@@ -2869,14 +2881,15 @@ ec_launch_replace_heal(ec_t *ec)
|
|
|
14f8ab |
{
|
|
|
14f8ab |
int ret = -1;
|
|
|
14f8ab |
|
|
|
14f8ab |
- if (!ec)
|
|
|
14f8ab |
- return ret;
|
|
|
14f8ab |
ret = synctask_new(ec->xl->ctx->env, ec_replace_brick_heal_wrap,
|
|
|
14f8ab |
ec_replace_heal_done, NULL, ec);
|
|
|
14f8ab |
+
|
|
|
14f8ab |
if (ret < 0) {
|
|
|
14f8ab |
gf_msg_debug(ec->xl->name, 0, "Heal failed for replace brick ret = %d",
|
|
|
14f8ab |
ret);
|
|
|
14f8ab |
+ ec_replace_heal_done(-1, NULL, ec);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
+
|
|
|
14f8ab |
return ret;
|
|
|
14f8ab |
}
|
|
|
14f8ab |
|
|
|
14f8ab |
diff --git a/xlators/cluster/ec/src/ec-types.h b/xlators/cluster/ec/src/ec-types.h
|
|
|
14f8ab |
index 1c295c0..4dbf4a3 100644
|
|
|
14f8ab |
--- a/xlators/cluster/ec/src/ec-types.h
|
|
|
14f8ab |
+++ b/xlators/cluster/ec/src/ec-types.h
|
|
|
14f8ab |
@@ -643,6 +643,7 @@ struct _ec {
|
|
|
14f8ab |
uintptr_t xl_notify; /* Bit flag representing
|
|
|
14f8ab |
notification for bricks. */
|
|
|
14f8ab |
uintptr_t node_mask;
|
|
|
14f8ab |
+ gf_atomic_t async_fop_count; /* Number of on going asynchronous fops. */
|
|
|
14f8ab |
xlator_t **xl_list;
|
|
|
14f8ab |
gf_lock_t lock;
|
|
|
14f8ab |
gf_timer_t *timer;
|
|
|
14f8ab |
diff --git a/xlators/cluster/ec/src/ec.c b/xlators/cluster/ec/src/ec.c
|
|
|
14f8ab |
index df5912c..f0d58c0 100644
|
|
|
14f8ab |
--- a/xlators/cluster/ec/src/ec.c
|
|
|
14f8ab |
+++ b/xlators/cluster/ec/src/ec.c
|
|
|
14f8ab |
@@ -355,6 +355,7 @@ ec_notify_cbk(void *data)
|
|
|
14f8ab |
ec_t *ec = data;
|
|
|
14f8ab |
glusterfs_event_t event = GF_EVENT_MAXVAL;
|
|
|
14f8ab |
gf_boolean_t propagate = _gf_false;
|
|
|
14f8ab |
+ gf_boolean_t launch_heal = _gf_false;
|
|
|
14f8ab |
|
|
|
14f8ab |
LOCK(&ec->lock);
|
|
|
14f8ab |
{
|
|
|
14f8ab |
@@ -384,6 +385,11 @@ ec_notify_cbk(void *data)
|
|
|
14f8ab |
* still bricks DOWN, they will be healed when they
|
|
|
14f8ab |
* come up. */
|
|
|
14f8ab |
ec_up(ec->xl, ec);
|
|
|
14f8ab |
+
|
|
|
14f8ab |
+ if (ec->shd.iamshd && !ec->shutdown) {
|
|
|
14f8ab |
+ launch_heal = _gf_true;
|
|
|
14f8ab |
+ GF_ATOMIC_INC(ec->async_fop_count);
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
}
|
|
|
14f8ab |
|
|
|
14f8ab |
propagate = _gf_true;
|
|
|
14f8ab |
@@ -391,13 +397,12 @@ ec_notify_cbk(void *data)
|
|
|
14f8ab |
unlock:
|
|
|
14f8ab |
UNLOCK(&ec->lock);
|
|
|
14f8ab |
|
|
|
14f8ab |
+ if (launch_heal) {
|
|
|
14f8ab |
+ /* We have just brought the volume UP, so we trigger
|
|
|
14f8ab |
+ * a self-heal check on the root directory. */
|
|
|
14f8ab |
+ ec_launch_replace_heal(ec);
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
if (propagate) {
|
|
|
14f8ab |
- if ((event == GF_EVENT_CHILD_UP) && ec->shd.iamshd) {
|
|
|
14f8ab |
- /* We have just brought the volume UP, so we trigger
|
|
|
14f8ab |
- * a self-heal check on the root directory. */
|
|
|
14f8ab |
- ec_launch_replace_heal(ec);
|
|
|
14f8ab |
- }
|
|
|
14f8ab |
-
|
|
|
14f8ab |
default_notify(ec->xl, event, NULL);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
}
|
|
|
14f8ab |
@@ -425,7 +430,7 @@ ec_disable_delays(ec_t *ec)
|
|
|
14f8ab |
{
|
|
|
14f8ab |
ec->shutdown = _gf_true;
|
|
|
14f8ab |
|
|
|
14f8ab |
- return list_empty(&ec->pending_fops);
|
|
|
14f8ab |
+ return __ec_is_last_fop(ec);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
|
|
|
14f8ab |
void
|
|
|
14f8ab |
@@ -603,7 +608,10 @@ ec_notify(xlator_t *this, int32_t event, void *data, void *data2)
|
|
|
14f8ab |
if (event == GF_EVENT_CHILD_UP) {
|
|
|
14f8ab |
/* We need to trigger a selfheal if a brick changes
|
|
|
14f8ab |
* to UP state. */
|
|
|
14f8ab |
- needs_shd_check = ec_set_up_state(ec, mask, mask);
|
|
|
14f8ab |
+ if (ec_set_up_state(ec, mask, mask) && ec->shd.iamshd &&
|
|
|
14f8ab |
+ !ec->shutdown) {
|
|
|
14f8ab |
+ needs_shd_check = _gf_true;
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
} else if (event == GF_EVENT_CHILD_DOWN) {
|
|
|
14f8ab |
ec_set_up_state(ec, mask, 0);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
@@ -633,17 +641,21 @@ ec_notify(xlator_t *this, int32_t event, void *data, void *data2)
|
|
|
14f8ab |
}
|
|
|
14f8ab |
} else {
|
|
|
14f8ab |
propagate = _gf_false;
|
|
|
14f8ab |
+ needs_shd_check = _gf_false;
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
+
|
|
|
14f8ab |
+ if (needs_shd_check) {
|
|
|
14f8ab |
+ GF_ATOMIC_INC(ec->async_fop_count);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
}
|
|
|
14f8ab |
unlock:
|
|
|
14f8ab |
UNLOCK(&ec->lock);
|
|
|
14f8ab |
|
|
|
14f8ab |
done:
|
|
|
14f8ab |
+ if (needs_shd_check) {
|
|
|
14f8ab |
+ ec_launch_replace_heal(ec);
|
|
|
14f8ab |
+ }
|
|
|
14f8ab |
if (propagate) {
|
|
|
14f8ab |
- if (needs_shd_check && ec->shd.iamshd) {
|
|
|
14f8ab |
- ec_launch_replace_heal(ec);
|
|
|
14f8ab |
- }
|
|
|
14f8ab |
-
|
|
|
14f8ab |
error = default_notify(this, event, data);
|
|
|
14f8ab |
}
|
|
|
14f8ab |
|
|
|
14f8ab |
@@ -705,6 +717,7 @@ init(xlator_t *this)
|
|
|
14f8ab |
ec->xl = this;
|
|
|
14f8ab |
LOCK_INIT(&ec->lock);
|
|
|
14f8ab |
|
|
|
14f8ab |
+ GF_ATOMIC_INIT(ec->async_fop_count, 0);
|
|
|
14f8ab |
INIT_LIST_HEAD(&ec->pending_fops);
|
|
|
14f8ab |
INIT_LIST_HEAD(&ec->heal_waiting);
|
|
|
14f8ab |
INIT_LIST_HEAD(&ec->healing);
|
|
|
14f8ab |
--
|
|
|
14f8ab |
1.8.3.1
|
|
|
14f8ab |
|