* [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries
@ 2026-06-10 9:11 Eimear Morrissey
2026-06-10 9:11 ` [PATCH 1/2] eal/pflock: add API to downgrade from wr to rd lock Eimear Morrissey
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Eimear Morrissey @ 2026-06-10 9:11 UTC (permalink / raw)
To: dev
Add new downgrade option for pflock. Add stress tests for this &
by extension the rest of the pflock/rwlock libraries.
Eimear Morrissey (1):
app/test: add stress tests for rwlock and pflock
Konstantin Ananyev (1):
eal/pflock: add API to downgrade from wr to rd lock
app/test/meson.build | 2 +
app/test/test_pflock_stress.c | 76 ++++++
app/test/test_rwlock_stress.c | 59 +++++
app/test/test_rwlock_stress_impl.h | 393 +++++++++++++++++++++++++++++
lib/eal/include/rte_pflock.h | 21 ++
5 files changed, 551 insertions(+)
create mode 100644 app/test/test_pflock_stress.c
create mode 100644 app/test/test_rwlock_stress.c
create mode 100644 app/test/test_rwlock_stress_impl.h
--
2.51.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] eal/pflock: add API to downgrade from wr to rd lock
2026-06-10 9:11 [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Eimear Morrissey
@ 2026-06-10 9:11 ` Eimear Morrissey
2026-06-10 9:11 ` [PATCH 2/2] app/test: add stress tests for rwlock and pflock Eimear Morrissey
2026-06-10 15:59 ` [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Stephen Hemminger
2 siblings, 0 replies; 4+ messages in thread
From: Eimear Morrissey @ 2026-06-10 9:11 UTC (permalink / raw)
To: dev; +Cc: Konstantin Ananyev
From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Add a new API that allows for the caller to downgrade from wrlock
to rdlock. Note that caller is expected to obtain wrlock before calling
that function.
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
lib/eal/include/rte_pflock.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/lib/eal/include/rte_pflock.h b/lib/eal/include/rte_pflock.h
index 6797ce5920..ed5255b3b5 100644
--- a/lib/eal/include/rte_pflock.h
+++ b/lib/eal/include/rte_pflock.h
@@ -179,6 +179,27 @@ rte_pflock_write_unlock(rte_pflock_t *pf)
rte_atomic_fetch_add_explicit(&pf->wr.out, 1, rte_memory_order_release);
}
+/**
+ * Release a pflock held for writing, while keeping lock for reading.
+ *
+ * @param pf
+ * A pointer to a pflock structure.
+ */
+static inline void
+rte_pflock_write_downgrade(rte_pflock_t *pf)
+{
+ /* Migrate from write phase to read phase. */
+ rte_atomic_fetch_add_explicit(&pf->rd.in, RTE_PFLOCK_RINC,
+ rte_memory_order_acq_rel);
+ rte_atomic_fetch_and_explicit(&pf->rd.in, RTE_PFLOCK_LSB,
+ rte_memory_order_release);
+
+ /* Allow other writers to continue. */
+ rte_atomic_fetch_add_explicit(&pf->wr.out, 1,
+ rte_memory_order_release);
+}
+
+
#ifdef __cplusplus
}
#endif
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] app/test: add stress tests for rwlock and pflock
2026-06-10 9:11 [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Eimear Morrissey
2026-06-10 9:11 ` [PATCH 1/2] eal/pflock: add API to downgrade from wr to rd lock Eimear Morrissey
@ 2026-06-10 9:11 ` Eimear Morrissey
2026-06-10 15:59 ` [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Stephen Hemminger
2 siblings, 0 replies; 4+ messages in thread
From: Eimear Morrissey @ 2026-06-10 9:11 UTC (permalink / raw)
To: dev
Stress tests for pflock. Since the logic is generic enough for
rwlock run them against rwlock too.
Signed-off-by: Eimear Morrissey <eimear.morrissey@huawei.com>
---
app/test/meson.build | 2 +
app/test/test_pflock_stress.c | 76 ++++++
app/test/test_rwlock_stress.c | 59 +++++
app/test/test_rwlock_stress_impl.h | 393 +++++++++++++++++++++++++++++
4 files changed, 530 insertions(+)
create mode 100644 app/test/test_pflock_stress.c
create mode 100644 app/test/test_rwlock_stress.c
create mode 100644 app/test/test_rwlock_stress_impl.h
diff --git a/app/test/meson.build b/app/test/meson.build
index 61024125a7..f85ad617ce 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -140,6 +140,7 @@ source_file_deps = {
'test_pdump.c': ['pdump'] + sample_packet_forward_deps,
'test_per_lcore.c': [],
'test_pflock.c': [],
+ 'test_pflock_stress.c': [],
'test_pie.c': ['sched'],
'test_pmd_af_packet.c': ['net_af_packet', 'ethdev', 'bus_vdev'],
'test_pmd_pcap.c': ['net_pcap', 'ethdev', 'bus_vdev'] + packet_burst_generator_deps,
@@ -178,6 +179,7 @@ source_file_deps = {
'test_ring_st_peek_stress_zc.c': ['ptr_compress'],
'test_ring_stress.c': ['ptr_compress'],
'test_rwlock.c': [],
+ 'test_rwlock_stress.c': [],
'test_sched.c': ['net', 'sched'],
'test_security.c': ['net', 'security'],
'test_security_inline_macsec.c': ['ethdev', 'security'],
diff --git a/app/test/test_pflock_stress.c b/app/test/test_pflock_stress.c
new file mode 100644
index 0000000000..cafc5defba
--- /dev/null
+++ b/app/test/test_pflock_stress.c
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Huawei Technologies Co., Ltd
+ */
+
+#include "test_rwlock_stress_impl.h"
+
+/* Pflock operation implementations */
+static void
+pflock_init_fn(struct rwlock_stress_lock *lock)
+{
+ rte_pflock_init(&lock->lock.pflock);
+}
+
+static void
+pflock_read_lock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_pflock_read_lock(&lock->lock.pflock);
+}
+
+static void
+pflock_read_unlock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_pflock_read_unlock(&lock->lock.pflock);
+}
+
+static void
+pflock_write_lock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_pflock_write_lock(&lock->lock.pflock);
+}
+
+static void
+pflock_write_unlock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_pflock_write_unlock(&lock->lock.pflock);
+}
+
+static void
+pflock_write_downgrade_fn(struct rwlock_stress_lock *lock)
+{
+ rte_pflock_write_downgrade(&lock->lock.pflock);
+}
+
+/* Pflock operations table */
+static const struct rwlock_ops pflock_ops = {
+ .name = "pflock",
+ .init = pflock_init_fn,
+ .read_lock = pflock_read_lock_fn,
+ .read_unlock = pflock_read_unlock_fn,
+ .write_lock = pflock_write_lock_fn,
+ .write_unlock = pflock_write_unlock_fn,
+ .write_downgrade = pflock_write_downgrade_fn,
+};
+
+static const struct test_descriptor pflock_specific_tests[] = {
+{
+ .name = "write_downgrade",
+ .num_readers_pct = 50,
+ .reader_delay_us = 0,
+ .writer_delay_us = 0,
+ .flags = DOWNGRADE_TEST,
+ },
+};
+
+static int
+run_pflock_tests(void)
+{
+ int ret = 0;
+ ret |= run_test_suite("PFLOCK Common Stress Tests", &pflock_ops,
+ tests, RTE_DIM(tests));
+ ret |= run_test_suite("PFLOCK Specific Stress Tests", &pflock_ops,
+ pflock_specific_tests, RTE_DIM(pflock_specific_tests));
+ return ret ? -1 : 0;
+}
+
+REGISTER_STRESS_TEST(pflock_stress_autotest, run_pflock_tests);
diff --git a/app/test/test_rwlock_stress.c b/app/test/test_rwlock_stress.c
new file mode 100644
index 0000000000..5d151f3f8f
--- /dev/null
+++ b/app/test/test_rwlock_stress.c
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Huawei Technologies Co., Ltd
+ */
+
+#include "test_rwlock_stress_impl.h"
+
+/* RWLock operation implementations */
+static void
+rwlock_init_fn(struct rwlock_stress_lock *lock)
+{
+ rte_rwlock_init(&lock->lock.rwlock);
+}
+
+static void
+rwlock_read_lock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_rwlock_read_lock(&lock->lock.rwlock);
+}
+
+static void
+rwlock_read_unlock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_rwlock_read_unlock(&lock->lock.rwlock);
+}
+
+static void
+rwlock_write_lock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_rwlock_write_lock(&lock->lock.rwlock);
+}
+
+static void
+rwlock_write_unlock_fn(struct rwlock_stress_lock *lock)
+{
+ rte_rwlock_write_unlock(&lock->lock.rwlock);
+}
+
+/* RWLock operations table */
+static const struct rwlock_ops rwlock_ops = {
+ .name = "rwlock",
+ .init = rwlock_init_fn,
+ .read_lock = rwlock_read_lock_fn,
+ .read_unlock = rwlock_read_unlock_fn,
+ .write_lock = rwlock_write_lock_fn,
+ .write_unlock = rwlock_write_unlock_fn,
+};
+
+static int
+run_rwlock_tests(void)
+{
+ int ret = 0;
+
+ ret |= run_test_suite("RWLOCK Stress Tests", &rwlock_ops, tests,
+ RTE_DIM(tests));
+
+ return ret ? -1 : 0;
+}
+
+REGISTER_STRESS_TEST(rwlock_stress_autotest, run_rwlock_tests);
diff --git a/app/test/test_rwlock_stress_impl.h b/app/test/test_rwlock_stress_impl.h
new file mode 100644
index 0000000000..d28ccd76e0
--- /dev/null
+++ b/app/test/test_rwlock_stress_impl.h
@@ -0,0 +1,393 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Huawei Technologies Co., Ltd
+ */
+
+#ifndef _TEST_RWLOCK_STRESS_H_
+#define _TEST_RWLOCK_STRESS_H_
+
+/**
+ * Generic reader-writer lock stress test.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <stdbool.h>
+
+#include <rte_lcore.h>
+#include <rte_cycles.h>
+#include <rte_atomic.h>
+#include <rte_launch.h>
+#include <rte_per_lcore.h>
+#include <rte_malloc.h>
+#include <rte_pflock.h>
+#include <rte_random.h>
+#include <rte_rwlock.h>
+
+#include "test.h"
+
+#define TEST_DURATION_SEC 5
+#define COUNTER_ARRAY_SIZE 1024
+#define DOWNGRADE_TEST 0x1 /* Will attempt to downgrade from write to read lock */
+#define DYNAMIC_ROLES 0x2 /* Threads can switch between reader/writer roles */
+
+struct rwlock_stress_lock;
+
+/**
+ * Lock operations interface.
+ */
+struct rwlock_ops {
+ const char *name;
+
+ void (*init)(struct rwlock_stress_lock *lock);
+ void (*read_lock)(struct rwlock_stress_lock *lock);
+ void (*read_unlock)(struct rwlock_stress_lock *lock);
+ void (*write_lock)(struct rwlock_stress_lock *lock);
+ void (*write_unlock)(struct rwlock_stress_lock *lock);
+ void (*write_downgrade)(struct rwlock_stress_lock *lock);
+};
+
+/**
+ * Generic lock structure.
+ */
+struct rwlock_stress_lock {
+ const struct rwlock_ops *ops;
+
+ union {
+ struct rte_pflock pflock;
+ rte_rwlock_t rwlock;
+ } lock;
+};
+
+/**
+ * Per-lcore statistics
+ */
+struct lcore_stats {
+ uint64_t reader_ops;
+ uint64_t writer_ops;
+ uint64_t local_counter;
+ uint64_t reader_errors;
+ uint64_t writer_errors;
+ uint64_t acquire_time;
+} __rte_cache_aligned;
+
+/**
+ * Test controls
+ */
+struct test_descriptor {
+ const char *name;
+ uint32_t num_readers_pct; /* Percentage of workers as readers (0-100) */
+ uint32_t reader_delay_us; /* Microseconds to delay in reader */
+ uint32_t writer_delay_us; /* Microseconds to delay in writer */
+ uint32_t flags; /* Specialist test behaviour */
+};
+
+/**
+ * Shared test state.
+ */
+struct rwlock_test_shared {
+ struct rwlock_stress_lock lock;
+ volatile uint64_t counter;
+ volatile uint64_t counter_array[COUNTER_ARRAY_SIZE];
+ volatile bool stop;
+ uint32_t num_readers;
+ uint32_t num_writers;
+ const struct test_descriptor *test;
+ struct lcore_stats stats[RTE_MAX_LCORE];
+} __rte_cache_aligned;
+
+/* Test descriptors array */
+static const struct test_descriptor tests[] = {
+ {
+ .name = "basic_reader_writer",
+ .num_readers_pct = 75,
+ .reader_delay_us = 0,
+ .writer_delay_us = 0,
+ },
+ {
+ .name = "long_hold",
+ .num_readers_pct = 67,
+ .reader_delay_us = 100,
+ .writer_delay_us = 100,
+ },
+ {
+ .name = "rapid_acquire_release",
+ .num_readers_pct = 67,
+ .reader_delay_us = 0,
+ .writer_delay_us = 0,
+ },
+ {
+ .name = "dynamic_roles",
+ .num_readers_pct = 75,
+ .reader_delay_us = 0,
+ .writer_delay_us = 0,
+ .flags = DYNAMIC_ROLES,
+ },
+};
+
+static inline bool
+should_be_writer(uint32_t num_readers, uint32_t flags)
+{
+ uint32_t total_lcores = rte_lcore_count();
+ if (total_lcores <= 1)
+ return true;
+
+ if (flags & DYNAMIC_ROLES) {
+ uint32_t readers_pct = (num_readers * 100) / (total_lcores - 1);
+ return (rte_rand_max(100) >= readers_pct);
+ }
+
+ unsigned int idx = rte_lcore_index(rte_lcore_id()) - 1;
+ return idx >= num_readers;
+}
+
+static void
+handle_error(struct rwlock_test_shared *s, unsigned int lcore_id,
+ bool write_lock, const char *func, int line)
+{
+ s->stop = true;
+ if (write_lock) {
+ s->stats[lcore_id].writer_errors++;
+ s->lock.ops->write_unlock(&s->lock);
+ } else {
+ s->stats[lcore_id].reader_errors++;
+ /* Don't unlock here as it's already unlocked by the calling function */
+ }
+ printf("ERROR: lcore:%u: %s:%d early termination\n", lcore_id, func, line);
+}
+
+static int
+handle_writer_work(struct rwlock_test_shared *s, unsigned int lcore_id,
+ const struct test_descriptor *test, uint64_t delta)
+{
+ s->lock.ops->write_lock(&s->lock);
+ uint64_t old_val = s->counter;
+ s->counter += delta;
+ s->stats[lcore_id].local_counter += delta;
+
+ /* Verify increment was atomic */
+ if (s->counter != old_val + delta) {
+ handle_error(s, lcore_id, true, __func__, __LINE__);
+ return -1;
+ }
+
+ /* Update all array elements */
+ for (uint32_t i = 0; i < COUNTER_ARRAY_SIZE; i++) {
+ s->counter_array[i] += delta;
+ if (s->counter_array[i] != s->counter) {
+ handle_error(s, lcore_id, true, __func__, __LINE__);
+ return -1;
+ }
+ }
+
+ if (test->flags & DOWNGRADE_TEST) {
+ /* Downgrade to read lock */
+ if (s->lock.ops->write_downgrade) {
+ s->lock.ops->write_downgrade(&s->lock);
+ /* Verify array consistency under read lock */
+ for (uint32_t i = 0; i < COUNTER_ARRAY_SIZE; i++) {
+ if (s->counter_array[i] != s->counter) {
+ handle_error(s, lcore_id, false, __func__, __LINE__);
+ return -1;
+ }
+ }
+ s->lock.ops->read_unlock(&s->lock);
+ }
+ } else {
+ if (test->writer_delay_us > 0)
+ rte_delay_us_sleep(test->writer_delay_us);
+ s->lock.ops->write_unlock(&s->lock);
+ }
+ s->stats[lcore_id].writer_ops++;
+ return 0;
+}
+
+static int
+handle_reader_work(struct rwlock_test_shared *s, unsigned int lcore_id,
+ const struct test_descriptor *test)
+{
+ uint64_t local_counter;
+
+ s->lock.ops->read_lock(&s->lock);
+ local_counter = s->counter;
+
+ /* Verify array consistency */
+ for (uint32_t i = 0; i < COUNTER_ARRAY_SIZE; i++) {
+ if (s->counter_array[i] != local_counter) {
+ handle_error(s, lcore_id, false, __func__, __LINE__);
+ return -1;
+ }
+ }
+
+ if (test->reader_delay_us > 0)
+ rte_delay_us_sleep(test->reader_delay_us);
+
+ /* Verify counter didn't change during read */
+ if (s->counter != local_counter) {
+ handle_error(s, lcore_id, false, __func__, __LINE__);
+ return -1;
+ }
+
+ s->lock.ops->read_unlock(&s->lock);
+ s->stats[lcore_id].reader_ops++;
+ return 0;
+}
+
+static int
+lcore_function(void *arg)
+{
+ struct rwlock_test_shared *s = arg;
+ unsigned int lcore_id = rte_lcore_id();
+ bool is_writer = should_be_writer(s->num_readers, s->test->flags);
+ const struct test_descriptor *test = s->test;
+
+ while (!s->stop) {
+ uint64_t start = rte_get_timer_cycles();
+ uint64_t delta = (rte_rand() % 64) + 1;
+ int ret;
+
+ if (is_writer)
+ ret = handle_writer_work(s, lcore_id, test, delta);
+ else
+ ret = handle_reader_work(s, lcore_id, test);
+
+ if (ret < 0)
+ continue;
+
+ /* Record max acquire time */
+ uint64_t wait_time = rte_get_timer_cycles() - start;
+ if (wait_time > s->stats[lcore_id].acquire_time)
+ s->stats[lcore_id].acquire_time = wait_time;
+ }
+
+ return 0;
+}
+
+static int
+verify(struct rwlock_test_shared *s)
+{
+ int ret = 0;
+ unsigned int lcore_id;
+ uint64_t total_reader_errors = 0;
+ uint64_t total_writer_errors = 0;
+ uint64_t sum_local_counters = 0;
+
+ /* Calculate errors and counters */
+ RTE_LCORE_FOREACH_WORKER(lcore_id) {
+ total_reader_errors += s->stats[lcore_id].reader_errors;
+ total_writer_errors += s->stats[lcore_id].writer_errors;
+ sum_local_counters += s->stats[lcore_id].local_counter;
+ }
+
+ /* Verify sum of per-lcore counters matches the shared counter */
+ if (s->counter != sum_local_counters) {
+ printf(" FAILED: shared counter=%" PRIu64
+ " sum of local counters=%" PRIu64 "\n",
+ s->counter, sum_local_counters);
+ ret = -1;
+ }
+
+ if (total_reader_errors) {
+ printf(" FAILED: reader errors=%" PRIu64 "\n",
+ total_reader_errors);
+ ret = -1;
+ }
+
+ if (total_writer_errors) {
+ printf(" FAILED: writer errors =%" PRIu64 "\n",
+ total_writer_errors);
+ ret = -1;
+ }
+
+ /* Verify array consistency */
+ for (uint32_t i = 0; i < COUNTER_ARRAY_SIZE; i++) {
+ if (s->counter_array[i] != s->counter) {
+ printf(" FAILED: counter_array[%u]=%" PRIu64 " counter=%" PRIu64 "\n",
+ i, s->counter_array[i], s->counter);
+ ret = -1;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int
+test_rwlock_stress_impl(const struct rwlock_ops *ops,
+ const struct test_descriptor *ind_test)
+{
+ struct rwlock_test_shared shared = {0};
+ uint64_t start_time, end_time;
+ uint64_t total_reader_ops = 0;
+ uint64_t total_writer_ops = 0;
+ uint64_t max_acquire_time = 0;
+ unsigned int lcore_id;
+ int ret = 0;
+
+ shared.lock.ops = ops;
+ shared.lock.ops->init(&shared.lock);
+ shared.test = ind_test;
+ shared.num_readers = (ind_test->num_readers_pct * (rte_lcore_count() - 1)) / 100;
+ shared.num_writers = (rte_lcore_count() - 1) - shared.num_readers;
+
+ printf(" %u readers, %u writers\n", shared.num_readers, shared.num_writers);
+
+ /* Launch workers */
+ RTE_LCORE_FOREACH_WORKER(lcore_id) {
+ rte_eal_remote_launch(lcore_function, &shared, lcore_id);
+ }
+
+ /* Run test for duration */
+ start_time = rte_get_timer_cycles();
+ rte_delay_ms(TEST_DURATION_SEC * 1000);
+
+ /* Stop workers and collect stats */
+ shared.stop = true;
+ RTE_LCORE_FOREACH_WORKER(lcore_id) {
+ rte_eal_wait_lcore(lcore_id);
+ if (shared.stats[lcore_id].acquire_time > max_acquire_time)
+ max_acquire_time = shared.stats[lcore_id].acquire_time;
+ total_reader_ops += shared.stats[lcore_id].reader_ops;
+ total_writer_ops += shared.stats[lcore_id].writer_ops;
+ }
+ end_time = rte_get_timer_cycles();
+
+ printf(" %"PRIu64" reader ops, %"PRIu64" writer ops,"
+ "total time: %.2f seconds\n",
+ total_reader_ops, total_writer_ops,
+ (double)(end_time - start_time) / rte_get_timer_hz());
+
+ ret = verify(&shared);
+ if (ret == 0) {
+ uint64_t hz = rte_get_timer_hz();
+ printf(" PASSED: All checks passed (max wait: %.2f us)\n",
+ (double)max_acquire_time * 1000000 / hz);
+ }
+ return ret;
+}
+
+/**
+ * Run a test suite with the given title and tests
+ */
+static int
+run_test_suite(const char *title, const struct rwlock_ops *ops,
+ const struct test_descriptor suite[], uint32_t count)
+{
+ uint32_t failed = 0;
+
+ printf("%s\n===================\n\n", title);
+ for (uint32_t i = 0; i < count; i++) {
+ printf("Test %u/%u: %s\n", i + 1, count, suite[i].name);
+ if (test_rwlock_stress_impl(ops, &suite[i]) < 0)
+ failed++;
+ printf("\n");
+ }
+ printf("===================\n");
+ printf("Results: %u/%u passed, %u failed\n", count - failed, count, failed);
+
+ return failed ? -1 : 0;
+}
+
+#endif /* _TEST_RWLOCK_STRESS_H_ */
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries
2026-06-10 9:11 [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Eimear Morrissey
2026-06-10 9:11 ` [PATCH 1/2] eal/pflock: add API to downgrade from wr to rd lock Eimear Morrissey
2026-06-10 9:11 ` [PATCH 2/2] app/test: add stress tests for rwlock and pflock Eimear Morrissey
@ 2026-06-10 15:59 ` Stephen Hemminger
2 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2026-06-10 15:59 UTC (permalink / raw)
To: Eimear Morrissey; +Cc: dev
On Wed, 10 Jun 2026 10:11:45 +0100
Eimear Morrissey <eimear.morrissey@huawei.com> wrote:
> Add new downgrade option for pflock. Add stress tests for this &
> by extension the rest of the pflock/rwlock libraries.
>
> Eimear Morrissey (1):
> app/test: add stress tests for rwlock and pflock
>
> Konstantin Ananyev (1):
> eal/pflock: add API to downgrade from wr to rd lock
>
> app/test/meson.build | 2 +
> app/test/test_pflock_stress.c | 76 ++++++
> app/test/test_rwlock_stress.c | 59 +++++
> app/test/test_rwlock_stress_impl.h | 393 +++++++++++++++++++++++++++++
> lib/eal/include/rte_pflock.h | 21 ++
> 5 files changed, 551 insertions(+)
> create mode 100644 app/test/test_pflock_stress.c
> create mode 100644 app/test/test_rwlock_stress.c
> create mode 100644 app/test/test_rwlock_stress_impl.h
>
Interesting idea, lots of feedback from AI. Mostly about the test.
Patch 1/2 (eal/pflock: add API to downgrade from wr to rd lock)
Warning: new public API is not marked __rte_experimental.
New APIs (including static inline) should carry the experimental tag
for at least one release per the ABI policy:
__rte_experimental
static inline void
rte_pflock_write_downgrade(rte_pflock_t *pf)
Warning: new EAL API added without a release notes entry.
Please add a note to doc/guides/rel_notes/release_26_07.rst.
Info: the hunk adds a double blank line before the #ifdef __cplusplus,
and the two atomic calls use different continuation indentation
(one tab vs two). checkpatch will complain about the blank lines.
Patch 2/2 (app/test: add stress tests for rwlock and pflock)
Error: read lock is leaked on reader error paths, hanging the test.
handle_error() with write_lock=false does not unlock, and its comment
claims the lock is "already unlocked by the calling function" -- but
handle_reader_work() calls it while still holding the read lock (both
the array-mismatch path and the counter-changed path), and the
DOWNGRADE_TEST failure path in handle_writer_work() likewise calls it
while holding the downgraded read lock. The leaked reader keeps rd.out
from ever matching, so any writer blocks forever in write_lock() and
rte_eal_wait_lcore() never returns: a detected failure becomes a hang
instead of a test failure. Simplest fix is to have callers unlock
before calling handle_error() and drop the unlock from it entirely;
that also fixes the downgrade path incrementing reader_errors for a
writer thread.
Error: stop flag uses volatile instead of atomics.
"volatile bool stop" is written by workers (handle_error) and the main
lcore and polled by all workers. volatile provides no ordering or
atomicity guarantee; use RTE_ATOMIC(bool) with
rte_atomic_load_explicit/rte_atomic_store_explicit as
test_ring_stress_impl.h does for wrk_cmd.
The volatile on counter and counter_array is unnecessary -- they are
only accessed under the lock, which already provides ordering -- and
it defeats compiler optimization of the 1024-element verify loops.
Warning: DYNAMIC_ROLES does not switch roles.
should_be_writer() is called once in lcore_function() before the loop,
so each thread's role is fixed for the whole run; the flag's stated
purpose ("Threads can switch between reader/writer roles") never
happens. Move the role decision inside the while loop when
DYNAMIC_ROLES is set.
Warning: should_be_writer() assumes the main lcore has index 0.
unsigned int idx = rte_lcore_index(rte_lcore_id()) - 1;
With --main-lcore set to a non-lowest core, a worker can have index 0,
so idx underflows to UINT_MAX and the reader/writer split no longer
matches num_readers/num_writers. Compute the worker's position by
iterating RTE_LCORE_FOREACH_WORKER or skip the main lcore's index
explicitly.
Warning: trailing alignment attribute placement.
} __rte_cache_aligned;
on struct lcore_stats and struct rwlock_test_shared must be written as
struct __rte_cache_aligned lcore_stats {
checkpatch enforces this (required for MSVC).
Info: "max wait" statistic does not measure lock wait time.
acquire_time spans the entire iteration including the verify loops and
the configured reader/writer delays, so for long_hold it reports the
delay, not contention. Either time only the lock call or rename it.
Info: missing space in the summary printf string concatenation:
"%"PRIu64" writer ops," "total time: ..."
prints "writer ops,total time". Also the first element of
pflock_specific_tests is mis-indented (opening brace at column 0).
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-10 15:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-10 9:11 [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Eimear Morrissey
2026-06-10 9:11 ` [PATCH 1/2] eal/pflock: add API to downgrade from wr to rd lock Eimear Morrissey
2026-06-10 9:11 ` [PATCH 2/2] app/test: add stress tests for rwlock and pflock Eimear Morrissey
2026-06-10 15:59 ` [PATCH 0/2] Pflock downgrade & stress tests for pflock/rwlock libraries Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox