casaroli commented on code in PR #3936:
URL: https://github.com/apache/nuttx-apps/pull/3936#discussion_r4437001108
##########
testing/ostest/spinlock.c:
##########
@@ -38,90 +38,178 @@
* Preprocessor Definitions
****************************************************************************/
+/* Maximum number of test threads - typically one per CPU core */
+
#define MAX_THREAD_NUM (CONFIG_SMP_NCPUS)
-#define LOOP_TIMES (CONFIG_TEST_LOOP_SCALE * 100000)
+
+/* Number of lock/unlock iterations per thread */
+
+#define LOOP_TIMES (CONFIG_TEST_LOOP_SCALE * 10000)
Review Comment:
Master d40132fba renamed `TEST_LOOP_SCALE` to `TESTING_LOOP_SCALE`. After
the rebase `CONFIG_TEST_LOOP_SCALE` is undefined and spinlock.c does not build.
Fix: rebase and use `CONFIG_TESTING_LOOP_SCALE * 10000`.
##########
testing/ostest/spinlock.c:
##########
@@ -236,37 +334,118 @@ void run_test_thread(void *lock, FAR void
*(*thread_func)(FAR void *arg),
****************************************************************************/
/****************************************************************************
- * Name: spinlock_test
+ * Name: spinlock_test_thread_num
+ *
+ * Description:
+ * Run lock performance tests for a specific number of threads.
+ * This function tests three different lock types:
+ * - spinlock_t: Basic spinlock
+ * - rspinlock_t: Recursive spinlock
+ * - seqcount_t: Sequence counter lock
+ *
+ * Input Parameters:
+ * thread_num - Number of threads to use in the test
+ *
+ * Returned Value:
+ * None
+ *
****************************************************************************/
static void spinlock_test_thread_num(unsigned thread_num)
{
- aligned_data(64) union
+ union
{
spinlock_t spinlock;
rspinlock_t rspinlock;
seqcount_t seqcount;
} lock;
- printf("Start Lock test:\n");
printf("Thread num: %u, Loop times: %d\n\n", thread_num, LOOP_TIMES);
+ /* Test basic spinlock */
+
spin_lock_init(&lock.spinlock);
run_test_thread(&lock, spinlock_t_test_thread, thread_num, "spinlock");
+ /* Test read-write spinlock */
Review Comment:
`rspinlock_t` is the recursive spinlock, not a read-write spinlock.
Fix: write "Test recursive spinlock".
##########
testing/ostest/Kconfig:
##########
@@ -144,6 +155,16 @@ config TESTING_OSTEST_MULTIUSER
config TEST_LOOP_SCALE
int "Loop scale of spinlock test (N x 10,000)"
+ default 5 if MM_KASAN
default 100
+ ---help---
+ Number of iterations per spinlock test thread, expressed as
+ scale * 100000 (see LOOP_TIMES in spinlock.c). When KASAN is
Review Comment:
The help says `scale * 100000`, but `LOOP_TIMES` is `scale * 10000`
(spinlock.c:47).
Fix: write "scale * 10000".
##########
testing/ostest/spinlock.c:
##########
@@ -236,37 +334,118 @@ void run_test_thread(void *lock, FAR void
*(*thread_func)(FAR void *arg),
****************************************************************************/
/****************************************************************************
- * Name: spinlock_test
+ * Name: spinlock_test_thread_num
+ *
+ * Description:
+ * Run lock performance tests for a specific number of threads.
+ * This function tests three different lock types:
+ * - spinlock_t: Basic spinlock
+ * - rspinlock_t: Recursive spinlock
+ * - seqcount_t: Sequence counter lock
+ *
+ * Input Parameters:
+ * thread_num - Number of threads to use in the test
+ *
+ * Returned Value:
+ * None
+ *
****************************************************************************/
static void spinlock_test_thread_num(unsigned thread_num)
{
- aligned_data(64) union
+ union
{
spinlock_t spinlock;
rspinlock_t rspinlock;
seqcount_t seqcount;
} lock;
- printf("Start Lock test:\n");
printf("Thread num: %u, Loop times: %d\n\n", thread_num, LOOP_TIMES);
+ /* Test basic spinlock */
+
spin_lock_init(&lock.spinlock);
run_test_thread(&lock, spinlock_t_test_thread, thread_num, "spinlock");
+ /* Test read-write spinlock */
+
rspin_lock_init(&lock.rspinlock);
run_test_thread(&lock, rspinlock_t_test_thread, thread_num, "rspinlock");
+ /* Test sequence counter lock */
+
seqlock_init(&lock.seqcount);
run_test_thread(&lock, seqcount_t_test_thread, thread_num, "seqcount");
}
-void spinlock_test(void)
+/****************************************************************************
+ * Name: spinlock_test_thread
+ *
+ * Description:
+ * Main test thread function that runs tests with varying thread counts.
+ * This function iterates through different thread counts (1 to
+ * MAX_THREAD_NUM) to test lock performance under different concurrency
+ * levels.
+ *
+ * Input Parameters:
+ * arg - Unused argument
+ *
+ * Returned Value:
+ * Always returns NULL
+ *
+ ****************************************************************************/
+
+static void *spinlock_test_thread(void *arg)
{
unsigned tnr;
for (tnr = 1; tnr <= MAX_THREAD_NUM; tnr++)
{
spinlock_test_thread_num(tnr);
}
+
+ return NULL;
+}
+
+/****************************************************************************
+ * Name: spinlock_test
+ *
+ * Description:
+ * Entry point for the spinlock performance test suite.
+ * This function creates a test thread with reduced priority to run
+ * the lock performance tests. The tests measure throughput and
+ * execution time for different lock types under varying thread counts.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void spinlock_test(void)
+{
+ pthread_t tid;
+ pthread_attr_t attr;
+ struct sched_param sparam;
+
+ /* Initialize thread attributes */
+
+ ASSERT(0 == pthread_attr_init(&attr));
+
+ /* Set thread priority to default - 1 */
Review Comment:
The thread gets `CONFIG_TESTING_OSTEST_SPINLOCK_PRIORITY`, not "default -
1". The comments at lines 235 ("maximum priority") and 415 ("reduced priority")
are also wrong now.
Fix: say `CONFIG_TESTING_OSTEST_SPINLOCK_PRIORITY` at lines 235 and 437, and
remove "with reduced priority" at line 415.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
