* [PATCH] rcu: refactor rcu register/unregister functions
@ 2024-09-06 20:20 Doug Foster
2024-10-11 9:18 ` David Marchand
0 siblings, 1 reply; 3+ messages in thread
From: Doug Foster @ 2024-09-06 20:20 UTC (permalink / raw)
To: Thomas Monjalon, Honnappa Nagarahalli
Cc: dev, Doug Foster, Wathsala Vithanage
This simplifies the implementation of rte_rcu_qsbr_thread_register()
and rte_rcu_thread_unregister() functions. The simplified implementation
is easier to read.
Signed-off-by: Doug Foster <doug.foster@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
---
.mailmap | 1 +
lib/rcu/rte_rcu_qsbr.c | 77 ++++++++++++------------------------------
2 files changed, 23 insertions(+), 55 deletions(-)
diff --git a/.mailmap b/.mailmap
index f76037213d..63d0f4187a 100644
--- a/.mailmap
+++ b/.mailmap
@@ -360,6 +360,7 @@ Don Provan <dprovan@bivio.net>
Don Wallwork <donw@xsightlabs.com>
Doug Dziggel <douglas.a.dziggel@intel.com>
Douglas Flint <douglas.flint@broadcom.com>
+Doug Foster <doug.foster@arm.com>
Dr. David Alan Gilbert <dgilbert@redhat.com>
Drocula Lambda <quzeyao@gmail.com>
Dror Birkman <dror.birkman@lightcyber.com>
diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
index f08d974d07..40d7c566c8 100644
--- a/lib/rcu/rte_rcu_qsbr.c
+++ b/lib/rcu/rte_rcu_qsbr.c
@@ -81,8 +81,8 @@ rte_rcu_qsbr_init(struct rte_rcu_qsbr *v, uint32_t max_threads)
int
rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v, unsigned int thread_id)
{
- unsigned int i, id, success;
- uint64_t old_bmap, new_bmap;
+ unsigned int i, id;
+ uint64_t old_bmap;
if (v == NULL || thread_id >= v->max_threads) {
RCU_LOG(ERR, "Invalid input parameter");
@@ -97,31 +97,15 @@ rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v, unsigned int thread_id)
id = thread_id & __RTE_QSBR_THRID_MASK;
i = thread_id >> __RTE_QSBR_THRID_INDEX_SHIFT;
- /* Make sure that the counter for registered threads does not
- * go out of sync. Hence, additional checks are required.
- */
- /* Check if the thread is already registered */
- old_bmap = rte_atomic_load_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
- rte_memory_order_relaxed);
- if (old_bmap & 1UL << id)
- return 0;
+ /* Add the thread to the bitmap of registered threads */
+ old_bmap = rte_atomic_fetch_or_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
+ (1UL << id), rte_memory_order_release);
- do {
- new_bmap = old_bmap | (1UL << id);
- success = rte_atomic_compare_exchange_strong_explicit(
- __RTE_QSBR_THRID_ARRAY_ELM(v, i),
- &old_bmap, new_bmap,
- rte_memory_order_release, rte_memory_order_relaxed);
-
- if (success)
- rte_atomic_fetch_add_explicit(&v->num_threads,
- 1, rte_memory_order_relaxed);
- else if (old_bmap & (1UL << id))
- /* Someone else registered this thread.
- * Counter should not be incremented.
- */
- return 0;
- } while (success == 0);
+ /* Increment the number of threads registered only if the thread was not already
+ * registered
+ */
+ if (!(old_bmap & (1UL << id)))
+ rte_atomic_fetch_add_explicit(&v->num_threads, 1, rte_memory_order_relaxed);
return 0;
}
@@ -132,8 +116,8 @@ rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v, unsigned int thread_id)
int
rte_rcu_qsbr_thread_unregister(struct rte_rcu_qsbr *v, unsigned int thread_id)
{
- unsigned int i, id, success;
- uint64_t old_bmap, new_bmap;
+ unsigned int i, id;
+ uint64_t old_bmap;
if (v == NULL || thread_id >= v->max_threads) {
RCU_LOG(ERR, "Invalid input parameter");
@@ -148,35 +132,18 @@ rte_rcu_qsbr_thread_unregister(struct rte_rcu_qsbr *v, unsigned int thread_id)
id = thread_id & __RTE_QSBR_THRID_MASK;
i = thread_id >> __RTE_QSBR_THRID_INDEX_SHIFT;
- /* Make sure that the counter for registered threads does not
- * go out of sync. Hence, additional checks are required.
+ /* Make sure any loads of the shared data structure are
+ * completed before removal of the thread from the bitmap of
+ * reporting threads.
*/
- /* Check if the thread is already unregistered */
- old_bmap = rte_atomic_load_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
- rte_memory_order_relaxed);
- if (!(old_bmap & (1UL << id)))
- return 0;
+ old_bmap = rte_atomic_fetch_and_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
+ ~(1UL << id), rte_memory_order_release);
- do {
- new_bmap = old_bmap & ~(1UL << id);
- /* Make sure any loads of the shared data structure are
- * completed before removal of the thread from the list of
- * reporting threads.
- */
- success = rte_atomic_compare_exchange_strong_explicit(
- __RTE_QSBR_THRID_ARRAY_ELM(v, i),
- &old_bmap, new_bmap,
- rte_memory_order_release, rte_memory_order_relaxed);
-
- if (success)
- rte_atomic_fetch_sub_explicit(&v->num_threads,
- 1, rte_memory_order_relaxed);
- else if (!(old_bmap & (1UL << id)))
- /* Someone else unregistered this thread.
- * Counter should not be incremented.
- */
- return 0;
- } while (success == 0);
+ /* Decrement the number of threads unregistered only if the thread was not already
+ * unregistered
+ */
+ if (old_bmap & (1UL << id))
+ rte_atomic_fetch_sub_explicit(&v->num_threads, 1, rte_memory_order_relaxed);
return 0;
}
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rcu: refactor rcu register/unregister functions
2024-09-06 20:20 [PATCH] rcu: refactor rcu register/unregister functions Doug Foster
@ 2024-10-11 9:18 ` David Marchand
0 siblings, 0 replies; 3+ messages in thread
From: David Marchand @ 2024-10-11 9:18 UTC (permalink / raw)
To: Doug Foster
Cc: Thomas Monjalon, Honnappa Nagarahalli, dev, Wathsala Vithanage
On Sun, Sep 8, 2024 at 10:43 AM Doug Foster <doug.foster@arm.com> wrote:
>
> This simplifies the implementation of rte_rcu_qsbr_thread_register()
> and rte_rcu_thread_unregister() functions. The simplified implementation
> is easier to read.
>
> Signed-off-by: Doug Foster <doug.foster@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
Applied, thanks Doug.
--
David Marchand
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] rcu: refactor rcu register/unregister functions
@ 2024-09-06 20:52 Doug Foster
0 siblings, 0 replies; 3+ messages in thread
From: Doug Foster @ 2024-09-06 20:52 UTC (permalink / raw)
To: Thomas Monjalon, Honnappa Nagarahalli
Cc: dev, Doug Foster, Wathsala Vithanage
This simplifies the implementation of rte_rcu_qsbr_thread_register()
and rte_rcu_thread_unregister() functions. The simplified implementation
is easier to read.
Signed-off-by: Doug Foster <doug.foster@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
---
.mailmap | 1 +
lib/rcu/rte_rcu_qsbr.c | 77 ++++++++++++------------------------------
2 files changed, 23 insertions(+), 55 deletions(-)
diff --git a/.mailmap b/.mailmap
index f76037213d..63d0f4187a 100644
--- a/.mailmap
+++ b/.mailmap
@@ -360,6 +360,7 @@ Don Provan <dprovan@bivio.net>
Don Wallwork <donw@xsightlabs.com>
Doug Dziggel <douglas.a.dziggel@intel.com>
Douglas Flint <douglas.flint@broadcom.com>
+Doug Foster <doug.foster@arm.com>
Dr. David Alan Gilbert <dgilbert@redhat.com>
Drocula Lambda <quzeyao@gmail.com>
Dror Birkman <dror.birkman@lightcyber.com>
diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c
index f08d974d07..40d7c566c8 100644
--- a/lib/rcu/rte_rcu_qsbr.c
+++ b/lib/rcu/rte_rcu_qsbr.c
@@ -81,8 +81,8 @@ rte_rcu_qsbr_init(struct rte_rcu_qsbr *v, uint32_t max_threads)
int
rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v, unsigned int thread_id)
{
- unsigned int i, id, success;
- uint64_t old_bmap, new_bmap;
+ unsigned int i, id;
+ uint64_t old_bmap;
if (v == NULL || thread_id >= v->max_threads) {
RCU_LOG(ERR, "Invalid input parameter");
@@ -97,31 +97,15 @@ rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v, unsigned int thread_id)
id = thread_id & __RTE_QSBR_THRID_MASK;
i = thread_id >> __RTE_QSBR_THRID_INDEX_SHIFT;
- /* Make sure that the counter for registered threads does not
- * go out of sync. Hence, additional checks are required.
- */
- /* Check if the thread is already registered */
- old_bmap = rte_atomic_load_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
- rte_memory_order_relaxed);
- if (old_bmap & 1UL << id)
- return 0;
+ /* Add the thread to the bitmap of registered threads */
+ old_bmap = rte_atomic_fetch_or_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
+ (1UL << id), rte_memory_order_release);
- do {
- new_bmap = old_bmap | (1UL << id);
- success = rte_atomic_compare_exchange_strong_explicit(
- __RTE_QSBR_THRID_ARRAY_ELM(v, i),
- &old_bmap, new_bmap,
- rte_memory_order_release, rte_memory_order_relaxed);
-
- if (success)
- rte_atomic_fetch_add_explicit(&v->num_threads,
- 1, rte_memory_order_relaxed);
- else if (old_bmap & (1UL << id))
- /* Someone else registered this thread.
- * Counter should not be incremented.
- */
- return 0;
- } while (success == 0);
+ /* Increment the number of threads registered only if the thread was not already
+ * registered
+ */
+ if (!(old_bmap & (1UL << id)))
+ rte_atomic_fetch_add_explicit(&v->num_threads, 1, rte_memory_order_relaxed);
return 0;
}
@@ -132,8 +116,8 @@ rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v, unsigned int thread_id)
int
rte_rcu_qsbr_thread_unregister(struct rte_rcu_qsbr *v, unsigned int thread_id)
{
- unsigned int i, id, success;
- uint64_t old_bmap, new_bmap;
+ unsigned int i, id;
+ uint64_t old_bmap;
if (v == NULL || thread_id >= v->max_threads) {
RCU_LOG(ERR, "Invalid input parameter");
@@ -148,35 +132,18 @@ rte_rcu_qsbr_thread_unregister(struct rte_rcu_qsbr *v, unsigned int thread_id)
id = thread_id & __RTE_QSBR_THRID_MASK;
i = thread_id >> __RTE_QSBR_THRID_INDEX_SHIFT;
- /* Make sure that the counter for registered threads does not
- * go out of sync. Hence, additional checks are required.
+ /* Make sure any loads of the shared data structure are
+ * completed before removal of the thread from the bitmap of
+ * reporting threads.
*/
- /* Check if the thread is already unregistered */
- old_bmap = rte_atomic_load_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
- rte_memory_order_relaxed);
- if (!(old_bmap & (1UL << id)))
- return 0;
+ old_bmap = rte_atomic_fetch_and_explicit(__RTE_QSBR_THRID_ARRAY_ELM(v, i),
+ ~(1UL << id), rte_memory_order_release);
- do {
- new_bmap = old_bmap & ~(1UL << id);
- /* Make sure any loads of the shared data structure are
- * completed before removal of the thread from the list of
- * reporting threads.
- */
- success = rte_atomic_compare_exchange_strong_explicit(
- __RTE_QSBR_THRID_ARRAY_ELM(v, i),
- &old_bmap, new_bmap,
- rte_memory_order_release, rte_memory_order_relaxed);
-
- if (success)
- rte_atomic_fetch_sub_explicit(&v->num_threads,
- 1, rte_memory_order_relaxed);
- else if (!(old_bmap & (1UL << id)))
- /* Someone else unregistered this thread.
- * Counter should not be incremented.
- */
- return 0;
- } while (success == 0);
+ /* Decrement the number of threads unregistered only if the thread was not already
+ * unregistered
+ */
+ if (old_bmap & (1UL << id))
+ rte_atomic_fetch_sub_explicit(&v->num_threads, 1, rte_memory_order_relaxed);
return 0;
}
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-11 9:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-06 20:20 [PATCH] rcu: refactor rcu register/unregister functions Doug Foster
2024-10-11 9:18 ` David Marchand
2024-09-06 20:52 Doug Foster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).