* [Qemu-devel] [PATCH v2 01/11] rcu_queue: use atomic_set in QLIST_REMOVE_RCU
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 02/11] rcu_queue: remove barrier from QLIST_EMPTY_RCU Emilio G. Cota
` (10 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
To avoid undefined behaviour.
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
include/qemu/rcu_queue.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h
index 01be77407b..dd7b3be043 100644
--- a/include/qemu/rcu_queue.h
+++ b/include/qemu/rcu_queue.h
@@ -112,7 +112,7 @@ extern "C" {
(elm)->field.le_next->field.le_prev = \
(elm)->field.le_prev; \
} \
- *(elm)->field.le_prev = (elm)->field.le_next; \
+ atomic_set((elm)->field.le_prev, (elm)->field.le_next); \
} while (/*CONSTCOND*/0)
/* List traversal must occur within an RCU critical section. */
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 02/11] rcu_queue: remove barrier from QLIST_EMPTY_RCU
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 01/11] rcu_queue: use atomic_set in QLIST_REMOVE_RCU Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 03/11] rcu_queue: add RCU QSIMPLEQ Emilio G. Cota
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
It's unnecessary because the pointer isn't dereferenced.
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
include/qemu/rcu_queue.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h
index dd7b3be043..6881ea5274 100644
--- a/include/qemu/rcu_queue.h
+++ b/include/qemu/rcu_queue.h
@@ -36,7 +36,7 @@ extern "C" {
/*
* List access methods.
*/
-#define QLIST_EMPTY_RCU(head) (atomic_rcu_read(&(head)->lh_first) == NULL)
+#define QLIST_EMPTY_RCU(head) (atomic_read(&(head)->lh_first) == NULL)
#define QLIST_FIRST_RCU(head) (atomic_rcu_read(&(head)->lh_first))
#define QLIST_NEXT_RCU(elm, field) (atomic_rcu_read(&(elm)->field.le_next))
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 03/11] rcu_queue: add RCU QSIMPLEQ
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 01/11] rcu_queue: use atomic_set in QLIST_REMOVE_RCU Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 02/11] rcu_queue: remove barrier from QLIST_EMPTY_RCU Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 04/11] rcu_queue: add RCU QTAILQ Emilio G. Cota
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
include/qemu/rcu_queue.h | 65 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h
index 6881ea5274..e0395c989a 100644
--- a/include/qemu/rcu_queue.h
+++ b/include/qemu/rcu_queue.h
@@ -128,6 +128,71 @@ extern "C" {
((next_var) = atomic_rcu_read(&(var)->field.le_next), 1); \
(var) = (next_var))
+/*
+ * RCU simple queue
+ */
+
+/* Simple queue access methods */
+#define QSIMPLEQ_EMPTY_RCU(head) (atomic_read(&(head)->sqh_first) == NULL)
+#define QSIMPLEQ_FIRST_RCU(head) atomic_rcu_read(&(head)->sqh_first)
+#define QSIMPLEQ_NEXT_RCU(elm, field) atomic_rcu_read(&(elm)->field.sqe_next)
+
+/* Simple queue functions */
+#define QSIMPLEQ_INSERT_HEAD_RCU(head, elm, field) do { \
+ (elm)->field.sqe_next = (head)->sqh_first; \
+ if ((elm)->field.sqe_next == NULL) { \
+ (head)->sqh_last = &(elm)->field.sqe_next; \
+ } \
+ atomic_rcu_set(&(head)->sqh_first, (elm)); \
+} while (/*CONSTCOND*/0)
+
+#define QSIMPLEQ_INSERT_TAIL_RCU(head, elm, field) do { \
+ (elm)->field.sqe_next = NULL; \
+ atomic_rcu_set((head)->sqh_last, (elm)); \
+ (head)->sqh_last = &(elm)->field.sqe_next; \
+} while (/*CONSTCOND*/0)
+
+#define QSIMPLEQ_INSERT_AFTER_RCU(head, listelm, elm, field) do { \
+ (elm)->field.sqe_next = (listelm)->field.sqe_next; \
+ if ((elm)->field.sqe_next == NULL) { \
+ (head)->sqh_last = &(elm)->field.sqe_next; \
+ } \
+ atomic_rcu_set(&(listelm)->field.sqe_next, (elm)); \
+} while (/*CONSTCOND*/0)
+
+#define QSIMPLEQ_REMOVE_HEAD_RCU(head, field) do { \
+ atomic_set(&(head)->sqh_first, (head)->sqh_first->field.sqe_next); \
+ if ((head)->sqh_first == NULL) { \
+ (head)->sqh_last = &(head)->sqh_first; \
+ } \
+} while (/*CONSTCOND*/0)
+
+#define QSIMPLEQ_REMOVE_RCU(head, elm, type, field) do { \
+ if ((head)->sqh_first == (elm)) { \
+ QSIMPLEQ_REMOVE_HEAD_RCU((head), field); \
+ } else { \
+ struct type *curr = (head)->sqh_first; \
+ while (curr->field.sqe_next != (elm)) { \
+ curr = curr->field.sqe_next; \
+ } \
+ atomic_set(&curr->field.sqe_next, \
+ curr->field.sqe_next->field.sqe_next); \
+ if (curr->field.sqe_next == NULL) { \
+ (head)->sqh_last = &(curr)->field.sqe_next; \
+ } \
+ } \
+} while (/*CONSTCOND*/0)
+
+#define QSIMPLEQ_FOREACH_RCU(var, head, field) \
+ for ((var) = atomic_rcu_read(&(head)->sqh_first); \
+ (var); \
+ (var) = atomic_rcu_read(&(var)->field.sqe_next))
+
+#define QSIMPLEQ_FOREACH_SAFE_RCU(var, head, field, next) \
+ for ((var) = atomic_rcu_read(&(head)->sqh_first); \
+ (var) && ((next) = atomic_rcu_read(&(var)->field.sqe_next), 1); \
+ (var) = (next))
+
#ifdef __cplusplus
}
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 04/11] rcu_queue: add RCU QTAILQ
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (2 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 03/11] rcu_queue: add RCU QSIMPLEQ Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 05/11] test-rcu-list: access goflag with atomics Emilio G. Cota
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
include/qemu/rcu_queue.h | 66 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h
index e0395c989a..904b3372dc 100644
--- a/include/qemu/rcu_queue.h
+++ b/include/qemu/rcu_queue.h
@@ -193,6 +193,72 @@ extern "C" {
(var) && ((next) = atomic_rcu_read(&(var)->field.sqe_next), 1); \
(var) = (next))
+/*
+ * RCU tail queue
+ */
+
+/* Tail queue access methods */
+#define QTAILQ_EMPTY_RCU(head) (atomic_read(&(head)->tqh_first) == NULL)
+#define QTAILQ_FIRST_RCU(head) atomic_rcu_read(&(head)->tqh_first)
+#define QTAILQ_NEXT_RCU(elm, field) atomic_rcu_read(&(elm)->field.tqe_next)
+
+/* Tail queue functions */
+#define QTAILQ_INSERT_HEAD_RCU(head, elm, field) do { \
+ (elm)->field.tqe_next = (head)->tqh_first; \
+ if ((elm)->field.tqe_next != NULL) { \
+ (head)->tqh_first->field.tqe_prev = &(elm)->field.tqe_next; \
+ } else { \
+ (head)->tqh_last = &(elm)->field.tqe_next; \
+ } \
+ atomic_rcu_set(&(head)->tqh_first, (elm)); \
+ (elm)->field.tqe_prev = &(head)->tqh_first; \
+} while (/*CONSTCOND*/0)
+
+#define QTAILQ_INSERT_TAIL_RCU(head, elm, field) do { \
+ (elm)->field.tqe_next = NULL; \
+ (elm)->field.tqe_prev = (head)->tqh_last; \
+ atomic_rcu_set((head)->tqh_last, (elm)); \
+ (head)->tqh_last = &(elm)->field.tqe_next; \
+} while (/*CONSTCOND*/0)
+
+#define QTAILQ_INSERT_AFTER_RCU(head, listelm, elm, field) do { \
+ (elm)->field.tqe_next = (listelm)->field.tqe_next; \
+ if ((elm)->field.tqe_next != NULL) { \
+ (elm)->field.tqe_next->field.tqe_prev = &(elm)->field.tqe_next; \
+ } else { \
+ (head)->tqh_last = &(elm)->field.tqe_next; \
+ } \
+ atomic_rcu_set(&(listelm)->field.tqe_next, (elm)); \
+ (elm)->field.tqe_prev = &(listelm)->field.tqe_next; \
+} while (/*CONSTCOND*/0)
+
+#define QTAILQ_INSERT_BEFORE_RCU(listelm, elm, field) do { \
+ (elm)->field.tqe_prev = (listelm)->field.tqe_prev; \
+ (elm)->field.tqe_next = (listelm); \
+ atomic_rcu_set((listelm)->field.tqe_prev, (elm)); \
+ (listelm)->field.tqe_prev = &(elm)->field.tqe_next; \
+ } while (/*CONSTCOND*/0)
+
+#define QTAILQ_REMOVE_RCU(head, elm, field) do { \
+ if (((elm)->field.tqe_next) != NULL) { \
+ (elm)->field.tqe_next->field.tqe_prev = (elm)->field.tqe_prev; \
+ } else { \
+ (head)->tqh_last = (elm)->field.tqe_prev; \
+ } \
+ atomic_set((elm)->field.tqe_prev, (elm)->field.tqe_next); \
+ (elm)->field.tqe_prev = NULL; \
+} while (/*CONSTCOND*/0)
+
+#define QTAILQ_FOREACH_RCU(var, head, field) \
+ for ((var) = atomic_rcu_read(&(head)->tqh_first); \
+ (var); \
+ (var) = atomic_rcu_read(&(var)->field.tqe_next))
+
+#define QTAILQ_FOREACH_SAFE_RCU(var, head, field, next) \
+ for ((var) = atomic_rcu_read(&(head)->tqh_first); \
+ (var) && ((next) = atomic_rcu_read(&(var)->field.tqe_next), 1); \
+ (var) = (next))
+
#ifdef __cplusplus
}
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 05/11] test-rcu-list: access goflag with atomics
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (3 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 04/11] rcu_queue: add RCU QTAILQ Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 06/11] test-rcu-list: access counters " Emilio G. Cota
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Instead of declaring it volatile.
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
tests/test-rcu-list.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index 1514d7ec97..b4ed130081 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -44,7 +44,7 @@ static int nthreadsrunning;
#define GOFLAG_RUN 1
#define GOFLAG_STOP 2
-static volatile int goflag = GOFLAG_INIT;
+static int goflag = GOFLAG_INIT;
#define RCU_READ_RUN 1000
#define RCU_UPDATE_RUN 10
@@ -107,15 +107,15 @@ static void *rcu_q_reader(void *arg)
*(struct rcu_reader_data **)arg = &rcu_reader;
atomic_inc(&nthreadsrunning);
- while (goflag == GOFLAG_INIT) {
+ while (atomic_read(&goflag) == GOFLAG_INIT) {
g_usleep(1000);
}
- while (goflag == GOFLAG_RUN) {
+ while (atomic_read(&goflag) == GOFLAG_RUN) {
rcu_read_lock();
QLIST_FOREACH_RCU(el, &Q_list_head, entry) {
n_reads_local++;
- if (goflag == GOFLAG_STOP) {
+ if (atomic_read(&goflag) == GOFLAG_STOP) {
break;
}
}
@@ -142,11 +142,11 @@ static void *rcu_q_updater(void *arg)
*(struct rcu_reader_data **)arg = &rcu_reader;
atomic_inc(&nthreadsrunning);
- while (goflag == GOFLAG_INIT) {
+ while (atomic_read(&goflag) == GOFLAG_INIT) {
g_usleep(1000);
}
- while (goflag == GOFLAG_RUN) {
+ while (atomic_read(&goflag) == GOFLAG_RUN) {
target_el = select_random_el(RCU_Q_LEN);
j = 0;
/* FOREACH_RCU could work here but let's use both macros */
@@ -160,7 +160,7 @@ static void *rcu_q_updater(void *arg)
break;
}
}
- if (goflag == GOFLAG_STOP) {
+ if (atomic_read(&goflag) == GOFLAG_STOP) {
break;
}
target_el = select_random_el(RCU_Q_LEN);
@@ -209,9 +209,9 @@ static void rcu_qtest_run(int duration, int nreaders)
g_usleep(1000);
}
- goflag = GOFLAG_RUN;
+ atomic_set(&goflag, GOFLAG_RUN);
sleep(duration);
- goflag = GOFLAG_STOP;
+ atomic_set(&goflag, GOFLAG_STOP);
wait_all_threads();
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 06/11] test-rcu-list: access counters with atomics
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (4 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 05/11] test-rcu-list: access goflag with atomics Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 07/11] test-rcu-list: abstract the list implementation Emilio G. Cota
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
tests/test-rcu-list.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index b4ed130081..dc58091996 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -93,7 +93,7 @@ static void reclaim_list_el(struct rcu_head *prcu)
struct list_element *el = container_of(prcu, struct list_element, rcu);
g_free(el);
/* Accessed only from call_rcu thread. */
- n_reclaims++;
+ atomic_set(&n_reclaims, n_reclaims + 1);
}
static QLIST_HEAD(q_list_head, list_element) Q_list_head;
@@ -182,7 +182,7 @@ static void *rcu_q_updater(void *arg)
qemu_mutex_lock(&counts_mutex);
n_nodes += n_nodes_local;
n_updates += n_updates_local;
- n_nodes_removed += n_removed_local;
+ atomic_set(&n_nodes_removed, n_nodes_removed + n_removed_local);
qemu_mutex_unlock(&counts_mutex);
return NULL;
}
@@ -239,16 +239,18 @@ static void rcu_qtest(const char *test, int duration, int nreaders)
n_nodes_removed += n_removed_local;
qemu_mutex_unlock(&counts_mutex);
synchronize_rcu();
- while (n_nodes_removed > n_reclaims) {
+ while (atomic_read(&n_nodes_removed) > atomic_read(&n_reclaims)) {
g_usleep(100);
synchronize_rcu();
}
if (g_test_in_charge) {
- g_assert_cmpint(n_nodes_removed, ==, n_reclaims);
+ g_assert_cmpint(atomic_read(&n_nodes_removed), ==,
+ atomic_read(&n_reclaims));
} else {
printf("%s: %d readers; 1 updater; nodes read: " \
"%lld, nodes removed: %lld; nodes reclaimed: %lld\n",
- test, nthreadsrunning - 1, n_reads, n_nodes_removed, n_reclaims);
+ test, nthreadsrunning - 1, n_reads,
+ atomic_read(&n_nodes_removed), atomic_read(&n_reclaims));
exit(0);
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 07/11] test-rcu-list: abstract the list implementation
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (5 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 06/11] test-rcu-list: access counters " Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 08/11] tests: add test-list-simpleq Emilio G. Cota
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
So that we can test other implementations.
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
tests/test-rcu-list.c | 42 ++++++++++++++++++++++++++++++------------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index dc58091996..9bd11367a0 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -82,9 +82,16 @@ static void wait_all_threads(void)
n_threads = 0;
}
+#ifndef TEST_LIST_TYPE
+#define TEST_LIST_TYPE 1
+#endif
struct list_element {
+#if TEST_LIST_TYPE == 1
QLIST_ENTRY(list_element) entry;
+#else
+#error Invalid TEST_LIST_TYPE
+#endif
struct rcu_head rcu;
};
@@ -96,8 +103,19 @@ static void reclaim_list_el(struct rcu_head *prcu)
atomic_set(&n_reclaims, n_reclaims + 1);
}
+#if TEST_LIST_TYPE == 1
static QLIST_HEAD(q_list_head, list_element) Q_list_head;
+#define TEST_NAME "qlist"
+#define TEST_LIST_REMOVE_RCU QLIST_REMOVE_RCU
+#define TEST_LIST_INSERT_AFTER_RCU QLIST_INSERT_AFTER_RCU
+#define TEST_LIST_INSERT_HEAD_RCU QLIST_INSERT_HEAD_RCU
+#define TEST_LIST_FOREACH_RCU QLIST_FOREACH_RCU
+#define TEST_LIST_FOREACH_SAFE_RCU QLIST_FOREACH_SAFE_RCU
+#else
+#error Invalid TEST_LIST_TYPE
+#endif
+
static void *rcu_q_reader(void *arg)
{
long long n_reads_local = 0;
@@ -113,7 +131,7 @@ static void *rcu_q_reader(void *arg)
while (atomic_read(&goflag) == GOFLAG_RUN) {
rcu_read_lock();
- QLIST_FOREACH_RCU(el, &Q_list_head, entry) {
+ TEST_LIST_FOREACH_RCU(el, &Q_list_head, entry) {
n_reads_local++;
if (atomic_read(&goflag) == GOFLAG_STOP) {
break;
@@ -150,10 +168,10 @@ static void *rcu_q_updater(void *arg)
target_el = select_random_el(RCU_Q_LEN);
j = 0;
/* FOREACH_RCU could work here but let's use both macros */
- QLIST_FOREACH_SAFE_RCU(prev_el, &Q_list_head, entry, el) {
+ TEST_LIST_FOREACH_SAFE_RCU(prev_el, &Q_list_head, entry, el) {
j++;
if (target_el == j) {
- QLIST_REMOVE_RCU(prev_el, entry);
+ TEST_LIST_REMOVE_RCU(prev_el, entry);
/* may be more than one updater in the future */
call_rcu1(&prev_el->rcu, reclaim_list_el);
n_removed_local++;
@@ -165,12 +183,12 @@ static void *rcu_q_updater(void *arg)
}
target_el = select_random_el(RCU_Q_LEN);
j = 0;
- QLIST_FOREACH_RCU(el, &Q_list_head, entry) {
+ TEST_LIST_FOREACH_RCU(el, &Q_list_head, entry) {
j++;
if (target_el == j) {
- prev_el = g_new(struct list_element, 1);
+ struct list_element *new_el = g_new(struct list_element, 1);
n_nodes += n_nodes_local;
- QLIST_INSERT_BEFORE_RCU(el, prev_el, entry);
+ TEST_LIST_INSERT_AFTER_RCU(el, new_el, entry);
break;
}
}
@@ -195,7 +213,7 @@ static void rcu_qtest_init(void)
srand(time(0));
for (i = 0; i < RCU_Q_LEN; i++) {
new_el = g_new(struct list_element, 1);
- QLIST_INSERT_HEAD_RCU(&Q_list_head, new_el, entry);
+ TEST_LIST_INSERT_HEAD_RCU(&Q_list_head, new_el, entry);
}
qemu_mutex_lock(&counts_mutex);
n_nodes += RCU_Q_LEN;
@@ -230,8 +248,8 @@ static void rcu_qtest(const char *test, int duration, int nreaders)
create_thread(rcu_q_updater);
rcu_qtest_run(duration, nreaders);
- QLIST_FOREACH_SAFE_RCU(prev_el, &Q_list_head, entry, el) {
- QLIST_REMOVE_RCU(prev_el, entry);
+ TEST_LIST_FOREACH_SAFE_RCU(prev_el, &Q_list_head, entry, el) {
+ TEST_LIST_REMOVE_RCU(prev_el, entry);
call_rcu1(&prev_el->rcu, reclaim_list_el);
n_removed_local++;
}
@@ -292,9 +310,9 @@ int main(int argc, char *argv[])
} else {
gtest_seconds = 20;
}
- g_test_add_func("/rcu/qlist/single-threaded", gtest_rcuq_one);
- g_test_add_func("/rcu/qlist/short-few", gtest_rcuq_few);
- g_test_add_func("/rcu/qlist/long-many", gtest_rcuq_many);
+ g_test_add_func("/rcu/"TEST_NAME"/single-threaded", gtest_rcuq_one);
+ g_test_add_func("/rcu/"TEST_NAME"/short-few", gtest_rcuq_few);
+ g_test_add_func("/rcu/"TEST_NAME"/long-many", gtest_rcuq_many);
g_test_in_charge = 1;
return g_test_run();
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 08/11] tests: add test-list-simpleq
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (6 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 07/11] test-rcu-list: abstract the list implementation Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 09/11] tests: add test-rcu-tailq Emilio G. Cota
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
tests/test-rcu-list.c | 17 +++++++++++++++++
tests/test-rcu-simpleq.c | 2 ++
tests/Makefile.include | 4 ++++
3 files changed, 23 insertions(+)
create mode 100644 tests/test-rcu-simpleq.c
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index 9bd11367a0..c8bdf49743 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -89,6 +89,8 @@ static void wait_all_threads(void)
struct list_element {
#if TEST_LIST_TYPE == 1
QLIST_ENTRY(list_element) entry;
+#elif TEST_LIST_TYPE == 2
+ QSIMPLEQ_ENTRY(list_element) entry;
#else
#error Invalid TEST_LIST_TYPE
#endif
@@ -112,6 +114,21 @@ static QLIST_HEAD(q_list_head, list_element) Q_list_head;
#define TEST_LIST_INSERT_HEAD_RCU QLIST_INSERT_HEAD_RCU
#define TEST_LIST_FOREACH_RCU QLIST_FOREACH_RCU
#define TEST_LIST_FOREACH_SAFE_RCU QLIST_FOREACH_SAFE_RCU
+
+#elif TEST_LIST_TYPE == 2
+static QSIMPLEQ_HEAD(, list_element) Q_list_head =
+ QSIMPLEQ_HEAD_INITIALIZER(Q_list_head);
+
+#define TEST_NAME "qsimpleq"
+#define TEST_LIST_REMOVE_RCU(el, f) \
+ QSIMPLEQ_REMOVE_RCU(&Q_list_head, el, list_element, f)
+
+#define TEST_LIST_INSERT_AFTER_RCU(list_el, el, f) \
+ QSIMPLEQ_INSERT_AFTER_RCU(&Q_list_head, list_el, el, f)
+
+#define TEST_LIST_INSERT_HEAD_RCU QSIMPLEQ_INSERT_HEAD_RCU
+#define TEST_LIST_FOREACH_RCU QSIMPLEQ_FOREACH_RCU
+#define TEST_LIST_FOREACH_SAFE_RCU QSIMPLEQ_FOREACH_SAFE_RCU
#else
#error Invalid TEST_LIST_TYPE
#endif
diff --git a/tests/test-rcu-simpleq.c b/tests/test-rcu-simpleq.c
new file mode 100644
index 0000000000..057f7d33f7
--- /dev/null
+++ b/tests/test-rcu-simpleq.c
@@ -0,0 +1,2 @@
+#define TEST_LIST_TYPE 2
+#include "test-rcu-list.c"
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 760a0f18b6..997c27421a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -116,6 +116,8 @@ check-unit-y += tests/rcutorture$(EXESUF)
gcov-files-rcutorture-y = util/rcu.c
check-unit-y += tests/test-rcu-list$(EXESUF)
gcov-files-test-rcu-list-y = util/rcu.c
+check-unit-y += tests/test-rcu-simpleq$(EXESUF)
+gcov-files-test-rcu-simpleq-y = util/rcu.c
check-unit-y += tests/test-qdist$(EXESUF)
gcov-files-test-qdist-y = util/qdist.c
check-unit-y += tests/test-qht$(EXESUF)
@@ -600,6 +602,7 @@ test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o \
tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
tests/test-opts-visitor.o tests/test-qmp-event.o \
tests/rcutorture.o tests/test-rcu-list.o \
+ tests/test-rcu-simpleq.o \
tests/test-qdist.o tests/test-shift128.o \
tests/test-qht.o tests/qht-bench.o tests/test-qht-par.o \
tests/atomic_add-bench.o
@@ -649,6 +652,7 @@ tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-util-obj-y)
tests/test-int128$(EXESUF): tests/test-int128.o
tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
+tests/test-rcu-simpleq$(EXESUF): tests/test-rcu-simpleq.o $(test-util-obj-y)
tests/test-qdist$(EXESUF): tests/test-qdist.o $(test-util-obj-y)
tests/test-qht$(EXESUF): tests/test-qht.o $(test-util-obj-y)
tests/test-qht-par$(EXESUF): tests/test-qht-par.o tests/qht-bench$(EXESUF) $(test-util-obj-y)
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 09/11] tests: add test-rcu-tailq
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (7 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 08/11] tests: add test-list-simpleq Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 10/11] spapr: do not use CPU_FOREACH_REVERSE Emilio G. Cota
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
tests/test-rcu-list.c | 15 +++++++++++++++
tests/test-rcu-tailq.c | 2 ++
tests/Makefile.include | 4 ++++
3 files changed, 21 insertions(+)
create mode 100644 tests/test-rcu-tailq.c
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index c8bdf49743..8434700746 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -91,6 +91,8 @@ struct list_element {
QLIST_ENTRY(list_element) entry;
#elif TEST_LIST_TYPE == 2
QSIMPLEQ_ENTRY(list_element) entry;
+#elif TEST_LIST_TYPE == 3
+ QTAILQ_ENTRY(list_element) entry;
#else
#error Invalid TEST_LIST_TYPE
#endif
@@ -129,6 +131,19 @@ static QSIMPLEQ_HEAD(, list_element) Q_list_head =
#define TEST_LIST_INSERT_HEAD_RCU QSIMPLEQ_INSERT_HEAD_RCU
#define TEST_LIST_FOREACH_RCU QSIMPLEQ_FOREACH_RCU
#define TEST_LIST_FOREACH_SAFE_RCU QSIMPLEQ_FOREACH_SAFE_RCU
+
+#elif TEST_LIST_TYPE == 3
+static QTAILQ_HEAD(, list_element) Q_list_head;
+
+#define TEST_NAME "qtailq"
+#define TEST_LIST_REMOVE_RCU(el, f) QTAILQ_REMOVE_RCU(&Q_list_head, el, f)
+
+#define TEST_LIST_INSERT_AFTER_RCU(list_el, el, f) \
+ QTAILQ_INSERT_AFTER_RCU(&Q_list_head, list_el, el, f)
+
+#define TEST_LIST_INSERT_HEAD_RCU QTAILQ_INSERT_HEAD_RCU
+#define TEST_LIST_FOREACH_RCU QTAILQ_FOREACH_RCU
+#define TEST_LIST_FOREACH_SAFE_RCU QTAILQ_FOREACH_SAFE_RCU
#else
#error Invalid TEST_LIST_TYPE
#endif
diff --git a/tests/test-rcu-tailq.c b/tests/test-rcu-tailq.c
new file mode 100644
index 0000000000..8d487e0ee0
--- /dev/null
+++ b/tests/test-rcu-tailq.c
@@ -0,0 +1,2 @@
+#define TEST_LIST_TYPE 3
+#include "test-rcu-list.c"
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 997c27421a..5fe32fcfd0 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -118,6 +118,8 @@ check-unit-y += tests/test-rcu-list$(EXESUF)
gcov-files-test-rcu-list-y = util/rcu.c
check-unit-y += tests/test-rcu-simpleq$(EXESUF)
gcov-files-test-rcu-simpleq-y = util/rcu.c
+check-unit-y += tests/test-rcu-tailq$(EXESUF)
+gcov-files-test-rcu-tailq-y = util/rcu.c
check-unit-y += tests/test-qdist$(EXESUF)
gcov-files-test-qdist-y = util/qdist.c
check-unit-y += tests/test-qht$(EXESUF)
@@ -603,6 +605,7 @@ test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o \
tests/test-opts-visitor.o tests/test-qmp-event.o \
tests/rcutorture.o tests/test-rcu-list.o \
tests/test-rcu-simpleq.o \
+ tests/test-rcu-tailq.o \
tests/test-qdist.o tests/test-shift128.o \
tests/test-qht.o tests/qht-bench.o tests/test-qht-par.o \
tests/atomic_add-bench.o
@@ -653,6 +656,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
tests/test-rcu-simpleq$(EXESUF): tests/test-rcu-simpleq.o $(test-util-obj-y)
+tests/test-rcu-tailq$(EXESUF): tests/test-rcu-tailq.o $(test-util-obj-y)
tests/test-qdist$(EXESUF): tests/test-qdist.o $(test-util-obj-y)
tests/test-qht$(EXESUF): tests/test-qht.o $(test-util-obj-y)
tests/test-qht-par$(EXESUF): tests/test-qht-par.o tests/qht-bench$(EXESUF) $(test-util-obj-y)
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 10/11] spapr: do not use CPU_FOREACH_REVERSE
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (8 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 09/11] tests: add test-rcu-tailq Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 11/11] qom: convert the CPU list to RCU Emilio G. Cota
2018-08-20 9:30 ` [Qemu-devel] [PATCH v2 00/11] convert " Paolo Bonzini
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
This paves the way for implementing the CPU list with an RCU list,
which cannot be traversed in reverse order.
Note that this is the only caller of CPU_FOREACH_REVERSE.
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
hw/ppc/spapr.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 421b2dd09b..2ef5be2790 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -622,9 +622,12 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
{
+ CPUState **rev;
CPUState *cs;
+ int n_cpus;
int cpus_offset;
char *nodename;
+ int i;
cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
_FDT(cpus_offset);
@@ -635,8 +638,19 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
* We walk the CPUs in reverse order to ensure that CPU DT nodes
* created by fdt_add_subnode() end up in the right order in FDT
* for the guest kernel the enumerate the CPUs correctly.
+ *
+ * The CPU list cannot be traversed in reverse order, so we need
+ * to do extra work.
*/
- CPU_FOREACH_REVERSE(cs) {
+ n_cpus = 0;
+ rev = NULL;
+ CPU_FOREACH(cs) {
+ rev = g_renew(CPUState *, rev, n_cpus + 1);
+ rev[n_cpus++] = cs;
+ }
+
+ for (i = n_cpus - 1; i >= 0; i--) {
+ CPUState *cs = rev[i];
PowerPCCPU *cpu = POWERPC_CPU(cs);
int index = spapr_get_vcpu_id(cpu);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 11/11] qom: convert the CPU list to RCU
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (9 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 10/11] spapr: do not use CPU_FOREACH_REVERSE Emilio G. Cota
@ 2018-08-19 9:13 ` Emilio G. Cota
2018-08-20 9:30 ` [Qemu-devel] [PATCH v2 00/11] convert " Paolo Bonzini
11 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-19 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
Iterating over the list without using atomics is undefined behaviour,
since the list can be modified concurrently by other threads (e.g.
every time a new thread is created in user-mode).
Fix it by implementing the CPU list as an RCU QTAILQ. This requires
a little bit of extra work to traverse list in reverse order (see
previous patch), but other than that the conversion is trivial.
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
include/qom/cpu.h | 11 +++++------
cpus-common.c | 4 ++--
cpus.c | 2 +-
linux-user/main.c | 2 +-
linux-user/syscall.c | 2 +-
target/s390x/cpu_models.c | 2 +-
6 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index ecf6ed556a..dc130cd307 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -26,6 +26,7 @@
#include "exec/memattrs.h"
#include "qapi/qapi-types-run-state.h"
#include "qemu/bitmap.h"
+#include "qemu/rcu_queue.h"
#include "qemu/queue.h"
#include "qemu/thread.h"
@@ -442,13 +443,11 @@ struct CPUState {
QTAILQ_HEAD(CPUTailQ, CPUState);
extern struct CPUTailQ cpus;
-#define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node)
-#define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node)
+#define first_cpu QTAILQ_FIRST_RCU(&cpus)
+#define CPU_NEXT(cpu) QTAILQ_NEXT_RCU(cpu, node)
+#define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, &cpus, node)
#define CPU_FOREACH_SAFE(cpu, next_cpu) \
- QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu)
-#define CPU_FOREACH_REVERSE(cpu) \
- QTAILQ_FOREACH_REVERSE(cpu, &cpus, CPUTailQ, node)
-#define first_cpu QTAILQ_FIRST(&cpus)
+ QTAILQ_FOREACH_SAFE_RCU(cpu, &cpus, node, next_cpu)
extern __thread CPUState *current_cpu;
diff --git a/cpus-common.c b/cpus-common.c
index 59f751ecf9..98dd8c6ff1 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -84,7 +84,7 @@ void cpu_list_add(CPUState *cpu)
} else {
assert(!cpu_index_auto_assigned);
}
- QTAILQ_INSERT_TAIL(&cpus, cpu, node);
+ QTAILQ_INSERT_TAIL_RCU(&cpus, cpu, node);
qemu_mutex_unlock(&qemu_cpu_list_lock);
finish_safe_work(cpu);
@@ -101,7 +101,7 @@ void cpu_list_remove(CPUState *cpu)
assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
- QTAILQ_REMOVE(&cpus, cpu, node);
+ QTAILQ_REMOVE_RCU(&cpus, cpu, node);
cpu->cpu_index = UNASSIGNED_CPU_INDEX;
qemu_mutex_unlock(&qemu_cpu_list_lock);
}
diff --git a/cpus.c b/cpus.c
index b5844b7103..d60f8603fd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1491,7 +1491,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
atomic_mb_set(&cpu->exit_request, 0);
}
- qemu_tcg_rr_wait_io_event(cpu ? cpu : QTAILQ_FIRST(&cpus));
+ qemu_tcg_rr_wait_io_event(cpu ? cpu : first_cpu);
deal_with_unplugged_cpus();
}
diff --git a/linux-user/main.c b/linux-user/main.c
index ea00dd9057..923cbb753a 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -126,7 +126,7 @@ void fork_end(int child)
Discard information about the parent threads. */
CPU_FOREACH_SAFE(cpu, next_cpu) {
if (cpu != thread_cpu) {
- QTAILQ_REMOVE(&cpus, cpu, node);
+ QTAILQ_REMOVE_RCU(&cpus, cpu, node);
}
}
qemu_init_cpu_list();
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index bb42a225eb..95ac0102bf 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8051,7 +8051,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
TaskState *ts;
/* Remove the CPU from the list. */
- QTAILQ_REMOVE(&cpus, cpu, node);
+ QTAILQ_REMOVE_RCU(&cpus, cpu, node);
cpu_list_unlock();
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 604898a882..d1a45bd8c0 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -1110,7 +1110,7 @@ void s390_set_qemu_cpu_model(uint16_t type, uint8_t gen, uint8_t ec_ga,
const S390CPUDef *def = s390_find_cpu_def(type, gen, ec_ga, NULL);
g_assert(def);
- g_assert(QTAILQ_EMPTY(&cpus));
+ g_assert(QTAILQ_EMPTY_RCU(&cpus));
/* TCG emulates some features that can usually not be enabled with
* the emulated machine generation. Make sure they can be enabled
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU
2018-08-19 9:13 [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU Emilio G. Cota
` (10 preceding siblings ...)
2018-08-19 9:13 ` [Qemu-devel] [PATCH v2 11/11] qom: convert the CPU list to RCU Emilio G. Cota
@ 2018-08-20 9:30 ` Paolo Bonzini
2018-08-31 20:29 ` Emilio G. Cota
11 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2018-08-20 9:30 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
On 19/08/2018 11:13, Emilio G. Cota wrote:
> - Add some fixes for test-rcu-list. I wanted to be able to get no
> races with ThreadSanitizer, but it still warns about two races.
> I'm appending the report just in case, but I think tsan is getting
> confused.
I cannot understand the first. The second might be fixed by this untested
patch (the second hunk; the first is an optimization and clarification):
diff --git a/util/rcu.c b/util/rcu.c
index 5676c22bd1..314b7db1a6 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -192,7 +192,9 @@ static void enqueue(struct rcu_head *node)
node->next = NULL;
old_tail = atomic_xchg(&tail, &node->next);
- atomic_mb_set(old_tail, node);
+
+ /* Pairs with smb_mb_acquire() in try_dequeue. */
+ atomic_store_release(old_tail, node);
}
static struct rcu_head *try_dequeue(void)
@@ -213,7 +215,10 @@ retry:
* wrong and we need to wait until its enqueuer finishes the update.
*/
node = head;
- next = atomic_mb_read(&head->next);
+ smp_mb_acquire();
+
+ /* atomic_read is enough because the pointer is never dereferenced. */
+ next = atomic_read(&head->next);
if (!next) {
return NULL;
}
The idea being that enqueue() writes so to speak node->prev->next in
the atomic_store_release when it enqueues node; try_dequeue() instead reads
node->next, so there is no synchronizes-with edge. However, I'm not that
convinced that it's necessary...
Paolo
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU
2018-08-20 9:30 ` [Qemu-devel] [PATCH v2 00/11] convert " Paolo Bonzini
@ 2018-08-31 20:29 ` Emilio G. Cota
0 siblings, 0 replies; 14+ messages in thread
From: Emilio G. Cota @ 2018-08-31 20:29 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Peter Crosthwaite, Richard Henderson, David Gibson,
Alexander Graf, Riku Voipio, Laurent Vivier, qemu-ppc
On Mon, Aug 20, 2018 at 11:30:07 +0200, Paolo Bonzini wrote:
> On 19/08/2018 11:13, Emilio G. Cota wrote:
> > - Add some fixes for test-rcu-list. I wanted to be able to get no
> > races with ThreadSanitizer, but it still warns about two races.
> > I'm appending the report just in case, but I think tsan is getting
> > confused.
>
> I cannot understand the first. The second might be fixed by this untested
> patch (the second hunk; the first is an optimization and clarification):
>
> diff --git a/util/rcu.c b/util/rcu.c
> index 5676c22bd1..314b7db1a6 100644
> --- a/util/rcu.c
> +++ b/util/rcu.c
> @@ -192,7 +192,9 @@ static void enqueue(struct rcu_head *node)
>
> node->next = NULL;
> old_tail = atomic_xchg(&tail, &node->next);
> - atomic_mb_set(old_tail, node);
> +
> + /* Pairs with smb_mb_acquire() in try_dequeue. */
> + atomic_store_release(old_tail, node);
> }
>
> static struct rcu_head *try_dequeue(void)
> @@ -213,7 +215,10 @@ retry:
> * wrong and we need to wait until its enqueuer finishes the update.
> */
> node = head;
> - next = atomic_mb_read(&head->next);
> + smp_mb_acquire();
> +
> + /* atomic_read is enough because the pointer is never dereferenced. */
> + next = atomic_read(&head->next);
> if (!next) {
> return NULL;
> }
>
>
> The idea being that enqueue() writes so to speak node->prev->next in
> the atomic_store_release when it enqueues node; try_dequeue() instead reads
> node->next, so there is no synchronizes-with edge. However, I'm not that
> convinced that it's necessary...
This doesn't seem to help :-( I'd try to avoid standalone barriers
as much as possible, otherwise TSan gets confused (BTW this is why
seqlocks cannot be "understood" by TSan).
The cause of the warnings seem to be the calls to g_usleep (both in rcu.c
and in the test file), which lead TSan to believe that we might be
coordinating threads via sleep calls.
Substituting the sleep calls with cpu_relax gets rid of the warnings,
so I would say these warnings can be safely ignored.
Thanks,
Emilio
^ permalink raw reply [flat|nested] 14+ messages in thread