DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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