[PATCH v3] printk: kunit: support offstack cpumask

[PATCH v3] printk: kunit: support offstack cpumask

[Petr Mladek]

From: Arnd Bergmann <arnd@arndb.de>

For large values of CONFIG_NR_CPUS, the newly added kunit test fails
to build:

kernel/printk/printk_ringbuffer_kunit_test.c: In function 'test_readerwriter':
kernel/printk/printk_ringbuffer_kunit_test.c:279:1: error: the frame size of 1432 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]

Change this to use cpumask_var_t and allocate it dynamically when
CONFIG_CPUMASK_OFFSTACK is set.

alloc_cpumask_var() and free_cpumask_var() are not called when
CONFIG_CPUMASK_OFFSTACK is not set. It would require an explicit cast
in the clean up action, leaking the stack address to the kthread doing
the clean up, or another workaround. And both function do nothing
do nothing in this case, anyway. It looks likes the least evil solution.

Fixes: 5ea2bcdfbf46 ("printk: ringbuffer: Add KUnit test")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[pmladek@suse.com: Correctly handle allocation failures and freeing using KUnit test API.]
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
This is actually 3rd version of the patch.

Changes against [v2]:

  + Fix compilation without CONFIG_CPUMASK_OFFSTACK

Changes against [v1]

  + Use KUnit API for handling allocation failure and freeing.

[v2] https://lore.kernel.org/r/20250702095157.110916-3-pmladek@suse.com
[v1] https://lore.kernel.org/r/20250620192554.2234184-1-arnd@kernel.org

The patch applies on the top of printk/linux.git,
branch rework/ringbuffer-kunit-test.

 kernel/printk/printk_ringbuffer_kunit_test.c | 35 ++++++++++++++++----
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
index e67e1815f4c8..21000d7d7a32 100644
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -223,6 +223,27 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
 	return 0;
 }
 
+/*
+ * A cast would be needed for the clean up action when the cpumask was on stack.
+ * Also it would leak the stack address to the cleanup thread.
+ * And alloc_cpu_mask() and free_cpumask_var() would do nothing anyway.
+ */
+#ifdef CONFIG_CPUMASK_OFFSTACK
+KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
+
+static void prbtest_alloc_cpumask(struct kunit *test, cpumask_var_t *mask)
+{
+	int err;
+
+	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(mask, GFP_KERNEL));
+	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, *mask);
+	KUNIT_ASSERT_EQ(test, err, 0);
+}
+#else
+static inline
+void prbtest_alloc_cpumask(struct kunit *test, cpumask_var_t *mask) {}
+#endif
+
 KUNIT_DEFINE_ACTION_WRAPPER(prbtest_kthread_cleanup, kthread_stop, struct task_struct *);
 
 static void prbtest_add_kthread_cleanup(struct kunit *test, struct task_struct *kthread)
@@ -247,9 +268,11 @@ static void test_readerwriter(struct kunit *test)
 	struct prbtest_thread_data *thread_data;
 	struct prbtest_data *test_data;
 	struct task_struct *thread;
-	cpumask_t test_cpus;
+	cpumask_var_t test_cpus;
 	int cpu, reader_cpu;
 
+	prbtest_alloc_cpumask(test, &test_cpus);
+
 	cpus_read_lock();
 	/*
 	 * Failure of KUNIT_ASSERT() kills the current task
@@ -257,15 +280,15 @@ static void test_readerwriter(struct kunit *test)
 	 * Instead use a snapshot of the online CPUs.
 	 * If they change during test execution it is unfortunate but not a grave error.
 	 */
-	cpumask_copy(&test_cpus, cpu_online_mask);
+	cpumask_copy(test_cpus, cpu_online_mask);
 	cpus_read_unlock();
 
 	/* One CPU is for the reader, all others are writers */
-	reader_cpu = cpumask_first(&test_cpus);
-	if (cpumask_weight(&test_cpus) == 1)
+	reader_cpu = cpumask_first(test_cpus);
+	if (cpumask_weight(test_cpus) == 1)
 		kunit_warn(test, "more than one CPU is recommended");
 	else
-		cpumask_clear_cpu(reader_cpu, &test_cpus);
+		cpumask_clear_cpu(reader_cpu, test_cpus);
 
 	/* KUnit test can get restarted more times. */
 	prbtest_prb_reinit(&test_rb);
@@ -278,7 +301,7 @@ static void test_readerwriter(struct kunit *test)
 
 	kunit_info(test, "running for %lu ms\n", runtime_ms);
 
-	for_each_cpu(cpu, &test_cpus) {
+	for_each_cpu(cpu, test_cpus) {
 		thread_data = kunit_kmalloc(test, sizeof(*thread_data), GFP_KERNEL);
 		KUNIT_ASSERT_NOT_NULL(test, thread_data);
 		thread_data->test_data = test_data;
-- 
2.50.0

[PATCH v3] printk: kunit: support offstack cpumask

[Petr Mladek]

From: Arnd Bergmann <arnd@arndb.de>

For large values of CONFIG_NR_CPUS, the newly added kunit test fails
to build:

kernel/printk/printk_ringbuffer_kunit_test.c: In function 'test_readerwriter':
kernel/printk/printk_ringbuffer_kunit_test.c:279:1: error: the frame size of 1432 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]

Change this to use cpumask_var_t and allocate it dynamically when
CONFIG_CPUMASK_OFFSTACK is set.

The variable has to be released via a KUnit action wrapper so that it is
freed when the test fails and gets aborted. The parameter type is hardcoded
to "struct cpumask *" because the macro KUNIT_DEFINE_ACTION_WRAPPER()
does not accept an array. But the function does nothing when
CONFIG_CPUMASK_OFFSTACK is not set anyway.

Fixes: 5ea2bcdfbf46 ("printk: ringbuffer: Add KUnit test")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[pmladek@suse.com: Correctly handle allocation failures and freeing using KUnit test API.]
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
This patch applies on top of "rework/ringbuffer-kunit-test" branch
in printk/linux.git.

Changes against v2 [2]:

  - Hardcode the pointer type to "struct cpumask *" when defining
    prbtest_cpumask_cleanup() action to avoid warning
    when CONFIG_CPUMASK_OFFSTACK is not set.

Changes against v1 [1]:

   - Abort the test when the cpumask allocation fails.
   - Free the cpumask when the tests exits.

[1] https://lore.kernel.org/all/20250620192554.2234184-1-arnd@kernel.org
[2] https://lore.kernel.org/all/20250702095157.110916-3-pmladek@suse.com

 kernel/printk/printk_ringbuffer_kunit_test.c | 24 +++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
index e67e1815f4c8..2282348e869a 100644
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -223,8 +223,17 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
 	return 0;
 }
 
+KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, struct cpumask *);
 KUNIT_DEFINE_ACTION_WRAPPER(prbtest_kthread_cleanup, kthread_stop, struct task_struct *);
 
+static void prbtest_add_cpumask_cleanup(struct kunit *test, cpumask_var_t mask)
+{
+	int err;
+
+	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, mask);
+	KUNIT_ASSERT_EQ(test, err, 0);
+}
+
 static void prbtest_add_kthread_cleanup(struct kunit *test, struct task_struct *kthread)
 {
 	int err;
@@ -247,9 +256,12 @@ static void test_readerwriter(struct kunit *test)
 	struct prbtest_thread_data *thread_data;
 	struct prbtest_data *test_data;
 	struct task_struct *thread;
-	cpumask_t test_cpus;
+	cpumask_var_t test_cpus;
 	int cpu, reader_cpu;
 
+	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
+	prbtest_add_cpumask_cleanup(test, test_cpus);
+
 	cpus_read_lock();
 	/*
 	 * Failure of KUNIT_ASSERT() kills the current task
@@ -257,15 +269,15 @@ static void test_readerwriter(struct kunit *test)
 	 * Instead use a snapshot of the online CPUs.
 	 * If they change during test execution it is unfortunate but not a grave error.
 	 */
-	cpumask_copy(&test_cpus, cpu_online_mask);
+	cpumask_copy(test_cpus, cpu_online_mask);
 	cpus_read_unlock();
 
 	/* One CPU is for the reader, all others are writers */
-	reader_cpu = cpumask_first(&test_cpus);
-	if (cpumask_weight(&test_cpus) == 1)
+	reader_cpu = cpumask_first(test_cpus);
+	if (cpumask_weight(test_cpus) == 1)
 		kunit_warn(test, "more than one CPU is recommended");
 	else
-		cpumask_clear_cpu(reader_cpu, &test_cpus);
+		cpumask_clear_cpu(reader_cpu, test_cpus);
 
 	/* KUnit test can get restarted more times. */
 	prbtest_prb_reinit(&test_rb);
@@ -278,7 +290,7 @@ static void test_readerwriter(struct kunit *test)
 
 	kunit_info(test, "running for %lu ms\n", runtime_ms);
 
-	for_each_cpu(cpu, &test_cpus) {
+	for_each_cpu(cpu, test_cpus) {
 		thread_data = kunit_kmalloc(test, sizeof(*thread_data), GFP_KERNEL);
 		KUNIT_ASSERT_NOT_NULL(test, thread_data);
 		thread_data->test_data = test_data;
-- 
2.50.1

Re: [PATCH v3] printk: kunit: support offstack cpumask

[Petr Mladek]

On Tue 2025-09-02 16:03:26, Petr Mladek wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> For large values of CONFIG_NR_CPUS, the newly added kunit test fails
> to build:
> 
> kernel/printk/printk_ringbuffer_kunit_test.c: In function 'test_readerwriter':
> kernel/printk/printk_ringbuffer_kunit_test.c:279:1: error: the frame size of 1432 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> 
> Change this to use cpumask_var_t and allocate it dynamically when
> CONFIG_CPUMASK_OFFSTACK is set.
> 
> The variable has to be released via a KUnit action wrapper so that it is
> freed when the test fails and gets aborted. The parameter type is hardcoded
> to "struct cpumask *" because the macro KUNIT_DEFINE_ACTION_WRAPPER()
> does not accept an array. But the function does nothing when
> CONFIG_CPUMASK_OFFSTACK is not set anyway.
> 
> Fixes: 5ea2bcdfbf46 ("printk: ringbuffer: Add KUnit test")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> [pmladek@suse.com: Correctly handle allocation failures and freeing using KUnit test API.]
> Signed-off-by: Petr Mladek <pmladek@suse.com>

JFYI, this patch have been committed into printk/linux.git,
branch rework/ringbuffer-kunit-test.

It worked, I did it according to the feedback. Let's give
it some spin in linux-next before the next merge window.

Feel free to ask me to revert it, ...

Best Regards,
Petr