[PATCH] rcu_queue: add QSLIST functions

Paolo Bonzini posted 1 patch 4 years, 2 months ago
Test docker-quick@centos7 passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200220103828.24525-1-pbonzini@redhat.com
include/qemu/queue.h     | 15 +++++++++++--
include/qemu/rcu_queue.h | 47 ++++++++++++++++++++++++++++++++++++++++
tests/Makefile.include   |  2 ++
tests/test-rcu-list.c    | 16 ++++++++++++++
tests/test-rcu-slist.c   |  2 ++
5 files changed, 80 insertions(+), 2 deletions(-)
create mode 100644 tests/test-rcu-slist.c
[PATCH] rcu_queue: add QSLIST functions
Posted by Paolo Bonzini 4 years, 2 months ago
QSLIST is the only family of lists for which we do not have RCU-friendly accessors,
add them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/queue.h     | 15 +++++++++++--
 include/qemu/rcu_queue.h | 47 ++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include   |  2 ++
 tests/test-rcu-list.c    | 16 ++++++++++++++
 tests/test-rcu-slist.c   |  2 ++
 5 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 tests/test-rcu-slist.c

diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 19425f973f..fcecb70228 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -211,9 +211,20 @@ struct {                                                                \
         (head)->slh_first = (head)->slh_first->field.sle_next;          \
 } while (/*CONSTCOND*/0)
 
-#define QSLIST_REMOVE_AFTER(slistelm, field) do {                        \
+#define QSLIST_REMOVE_AFTER(slistelm, field) do {                       \
         (slistelm)->field.sle_next =                                    \
-            QSLIST_NEXT(QSLIST_NEXT((slistelm), field), field);           \
+            QSLIST_NEXT(QSLIST_NEXT((slistelm), field), field);         \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_REMOVE(head, elm, type, field) do {                      \
+    if ((head)->slh_first == (elm)) {                                   \
+        QSLIST_REMOVE_HEAD((head), field);                              \
+    } else {                                                            \
+        struct type *curelm = (head)->slh_first;                        \
+        while (curelm->field.sle_next != (elm))                         \
+            curelm = curelm->field.sle_next;                            \
+        curelm->field.sle_next = curelm->field.sle_next->field.sle_next; \
+    }                                                                   \
 } while (/*CONSTCOND*/0)
 
 #define QSLIST_FOREACH(var, head, field)                                 \
diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h
index 2d386f303e..558961cc27 100644
--- a/include/qemu/rcu_queue.h
+++ b/include/qemu/rcu_queue.h
@@ -262,6 +262,53 @@ extern "C" {
          (var) && ((next) = atomic_rcu_read(&(var)->field.tqe_next), 1); \
          (var) = (next))
 
+/*
+ * RCU singly-linked list
+ */
+
+/* Singly-linked list access methods */
+#define QSLIST_EMPTY_RCU(head)      (atomic_read(&(head)->slh_first) == NULL)
+#define QSLIST_FIRST_RCU(head)       atomic_rcu_read(&(head)->slh_first)
+#define QSLIST_NEXT_RCU(elm, field)  atomic_rcu_read(&(elm)->field.sle_next)
+
+/* Singly-linked list functions */
+#define QSLIST_INSERT_HEAD_RCU(head, elm, field) do {           \
+    (elm)->field.sle_next = (head)->slh_first;                  \
+    atomic_rcu_set(&(head)->slh_first, (elm));                  \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_INSERT_AFTER_RCU(head, listelm, elm, field) do {         \
+    (elm)->field.sle_next = (listelm)->field.sle_next;                  \
+    atomic_rcu_set(&(listelm)->field.sle_next, (elm));                  \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_REMOVE_HEAD_RCU(head, field) do {                       \
+    atomic_set(&(head)->slh_first, (head)->slh_first->field.sle_next); \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_REMOVE_RCU(head, elm, type, field) do {              \
+    if ((head)->slh_first == (elm)) {                               \
+        QSLIST_REMOVE_HEAD_RCU((head), field);                      \
+    } else {                                                        \
+        struct type *curr = (head)->slh_first;                      \
+        while (curr->field.sle_next != (elm)) {                     \
+            curr = curr->field.sle_next;                            \
+        }                                                           \
+        atomic_set(&curr->field.sle_next,                           \
+                   curr->field.sle_next->field.sle_next);           \
+    }                                                               \
+} while (/*CONSTCOND*/0)
+
+#define QSLIST_FOREACH_RCU(var, head, field)                          \
+    for ((var) = atomic_rcu_read(&(head)->slh_first);                   \
+         (var);                                                         \
+         (var) = atomic_rcu_read(&(var)->field.sle_next))
+
+#define QSLIST_FOREACH_SAFE_RCU(var, head, field, next)                \
+    for ((var) = atomic_rcu_read(&(head)->slh_first);                    \
+         (var) && ((next) = atomic_rcu_read(&(var)->field.sle_next), 1); \
+         (var) = (next))
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2f1cafed72..edcbd475aa 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -98,6 +98,7 @@ check-unit-y += tests/rcutorture$(EXESUF)
 check-unit-y += tests/test-rcu-list$(EXESUF)
 check-unit-y += tests/test-rcu-simpleq$(EXESUF)
 check-unit-y += tests/test-rcu-tailq$(EXESUF)
+check-unit-y += tests/test-rcu-slist$(EXESUF)
 check-unit-y += tests/test-qdist$(EXESUF)
 check-unit-y += tests/test-qht$(EXESUF)
 check-unit-y += tests/test-qht-par$(EXESUF)
@@ -415,6 +416,7 @@ 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-rcu-slist$(EXESUF): tests/test-rcu-slist.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)
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index 6f076473e0..1442c0c982 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -93,6 +93,8 @@ struct list_element {
     QSIMPLEQ_ENTRY(list_element) entry;
 #elif TEST_LIST_TYPE == 3
     QTAILQ_ENTRY(list_element) entry;
+#elif TEST_LIST_TYPE == 4
+    QSLIST_ENTRY(list_element) entry;
 #else
 #error Invalid TEST_LIST_TYPE
 #endif
@@ -144,6 +146,20 @@ static QTAILQ_HEAD(, list_element) Q_list_head;
 #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
+
+#elif TEST_LIST_TYPE == 4
+static QSLIST_HEAD(, list_element) Q_list_head;
+
+#define TEST_NAME "qslist"
+#define TEST_LIST_REMOVE_RCU(el, f)                              \
+	 QSLIST_REMOVE_RCU(&Q_list_head, el, list_element, f)
+
+#define TEST_LIST_INSERT_AFTER_RCU(list_el, el, f)               \
+         QSLIST_INSERT_AFTER_RCU(&Q_list_head, list_el, el, f)
+
+#define TEST_LIST_INSERT_HEAD_RCU   QSLIST_INSERT_HEAD_RCU
+#define TEST_LIST_FOREACH_RCU       QSLIST_FOREACH_RCU
+#define TEST_LIST_FOREACH_SAFE_RCU  QSLIST_FOREACH_SAFE_RCU
 #else
 #error Invalid TEST_LIST_TYPE
 #endif
diff --git a/tests/test-rcu-slist.c b/tests/test-rcu-slist.c
new file mode 100644
index 0000000000..868e1e472e
--- /dev/null
+++ b/tests/test-rcu-slist.c
@@ -0,0 +1,2 @@
+#define TEST_LIST_TYPE 4
+#include "test-rcu-list.c"
-- 
2.21.1


Re: [PATCH] rcu_queue: add QSLIST functions
Posted by no-reply@patchew.org 4 years, 2 months ago
Patchew URL: https://patchew.org/QEMU/20200220103828.24525-1-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] rcu_queue: add QSLIST functions
Message-id: 20200220103828.24525-1-pbonzini@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200220041118.23264-1-alxndr@bu.edu -> patchew/20200220041118.23264-1-alxndr@bu.edu
 * [new tag]         patchew/20200220103828.24525-1-pbonzini@redhat.com -> patchew/20200220103828.24525-1-pbonzini@redhat.com
Switched to a new branch 'test'
909114b rcu_queue: add QSLIST functions

=== OUTPUT BEGIN ===
WARNING: Block comments use a leading /* on a separate line
#26: FILE: include/qemu/queue.h:217:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#61: FILE: include/qemu/rcu_queue.h:278:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#66: FILE: include/qemu/rcu_queue.h:283:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#70: FILE: include/qemu/rcu_queue.h:287:
+} while (/*CONSTCOND*/0)

WARNING: Block comments use a leading /* on a separate line
#83: FILE: include/qemu/rcu_queue.h:300:
+} while (/*CONSTCOND*/0)

ERROR: code indent should never use tabs
#141: FILE: tests/test-rcu-list.c:155:
+^I QSLIST_REMOVE_RCU(&Q_list_head, el, list_element, f)$

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#153: 
new file mode 100644

total: 1 errors, 6 warnings, 119 lines checked

Commit 909114b5ff89 (rcu_queue: add QSLIST functions) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200220103828.24525-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] rcu_queue: add QSLIST functions
Posted by Stefan Hajnoczi 4 years, 2 months ago
On Thu, Feb 20, 2020 at 11:38:28AM +0100, Paolo Bonzini wrote:
> QSLIST is the only family of lists for which we do not have RCU-friendly accessors,
> add them.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/queue.h     | 15 +++++++++++--
>  include/qemu/rcu_queue.h | 47 ++++++++++++++++++++++++++++++++++++++++
>  tests/Makefile.include   |  2 ++
>  tests/test-rcu-list.c    | 16 ++++++++++++++
>  tests/test-rcu-slist.c   |  2 ++
>  5 files changed, 80 insertions(+), 2 deletions(-)
>  create mode 100644 tests/test-rcu-slist.c

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan
Re: [PATCH] rcu_queue: add QSLIST functions
Posted by Stefan Hajnoczi 4 years, 2 months ago
On Thu, Feb 20, 2020 at 11:38:28AM +0100, Paolo Bonzini wrote:
> QSLIST is the only family of lists for which we do not have RCU-friendly accessors,
> add them.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/queue.h     | 15 +++++++++++--
>  include/qemu/rcu_queue.h | 47 ++++++++++++++++++++++++++++++++++++++++
>  tests/Makefile.include   |  2 ++
>  tests/test-rcu-list.c    | 16 ++++++++++++++
>  tests/test-rcu-slist.c   |  2 ++
>  5 files changed, 80 insertions(+), 2 deletions(-)
>  create mode 100644 tests/test-rcu-slist.c

I'll include this in the pull request that introduces O(1) BHs.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>