[PATCH 0/3] printk: KUnit: Followup fixes for the new KUnit test

[PATCH 0/3] printk: KUnit: Followup fixes for the new KUnit test

[Petr Mladek]

Hi,

this patchset puts together some followup fixes for the new KUnit test
which were discussed on several locations.

1st patch:

  + adds a comment exaplaing why the test ignores pr_reserve() failures.

  + was proposed at https://lore.kernel.org/r/aFUiQESkXjFIGqez@pathway.suse.cz

  + Thomas Weißschuh added into v4 of the original patch but I have already
    comitted v3 in the meantime, see
    https://lore.kernel.org/r/20250620-printk-ringbuffer-test-v4-1-8df873f1f3e0@linutronix.de


2nd patch:

  + dynamically allocates a cpu bitmap to make the code safe even on systems
    with many CPUs.

  + v1 was set by Arnd Bergmann but it had some problems, see
    https://lore.kernel.org/r/20250620192554.2234184-1-arnd@kernel.org

  + This version just integreates the proposed fixes from
    https://lore.kernel.org/r/aFkuqaFn3BOvsPT-@pathway.suse.cz


3rd patch:

  + stores "size" instead on "len" in struct prbtest_rbdata so that
    is can be used to check code sanity by __counted_by(size).

  + fixes https://lore.kernel.org/r/eaea66b9-266a-46e7-980d-33f40ad4b215@sabinyo.mountain

  + it is based on the idea from Thomas Weißschuh, see
    20250626082605-c5fbbb88-f6cc-4659-bea0-a283cdb58e81@linutronix.de


Sigh, I should have asked people to send new patches. But this looked
easier and I wanted to clean the table.


Arnd Bergmann (1):
  printk: kunit: support offstack cpumask

Petr Mladek (2):
  printk: ringbuffer: Explain why the KUnit test ignores failed writes
  printk: kunit: Fix __counted_by() in struct prbtest_rbdata

 kernel/printk/printk_ringbuffer_kunit_test.c | 78 +++++++++++++-------
 1 file changed, 52 insertions(+), 26 deletions(-)

-- 
2.50.0

[PATCH 1/3] printk: ringbuffer: Explain why the KUnit test ignores failed writes

[Petr Mladek]

The KUnit test ignores prb_reserve() failures on purpose. It tries
to push the ringbuffer beyond limits.

Note that it is a know problem that writes might fail in this situation.
printk() tries to prevent this problem by:

  + allocating big enough data buffer, see log_buf_add_cpu().

  + allocating enough descriptors by using small enough average
    record, see PRB_AVGBITS.

  + storing the record with disabled interrupts, see vprintk_store().

Also the amount of printk() messages is always somehow bound in
practice. And they are serialized when they are printed from
many CPUs on purpose, for example, when printing backtraces.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk_ringbuffer_kunit_test.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
index 4081ae051d8e..217dcc14670c 100644
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -123,6 +123,19 @@ static int prbtest_writer(void *data)
 		/* specify the text sizes for reservation */
 		prb_rec_init_wr(&r, record_size);
 
+		/*
+		 * Reservation can fail if:
+		 *
+		 *      - No free descriptor is available.
+		 *      - The buffer is full, and the oldest record is reserved
+		 *        but not yet committed.
+		 *
+		 * It actually happens in this test because all CPUs are trying
+		 * to write an unbounded number of messages in a tight loop.
+		 * These failures are intentionally ignored because this test
+		 * focuses on races, ringbuffer consistency, and pushing system
+		 * usability limits.
+		 */
 		if (prb_reserve(&e, tr->test_data->ringbuffer, &r)) {
 			r.info->text_len = record_size;
 
-- 
2.50.0

Re: [PATCH 1/3] printk: ringbuffer: Explain why the KUnit test ignores failed writes

[John Ogness]

On 2025-07-02, Petr Mladek <pmladek@suse.com> wrote:
> The KUnit test ignores prb_reserve() failures on purpose. It tries
> to push the ringbuffer beyond limits.
>
> Note that it is a know problem that writes might fail in this situation.
> printk() tries to prevent this problem by:
>
>   + allocating big enough data buffer, see log_buf_add_cpu().
>
>   + allocating enough descriptors by using small enough average
>     record, see PRB_AVGBITS.
>
>   + storing the record with disabled interrupts, see vprintk_store().
>
> Also the amount of printk() messages is always somehow bound in
> practice. And they are serialized when they are printed from
> many CPUs on purpose, for example, when printing backtraces.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: John Ogness <john.ogness@linutronix.de>

[PATCH 2/3] 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.

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>
---
 kernel/printk/printk_ringbuffer_kunit_test.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
index 217dcc14670c..0c3030fde8c2 100644
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -216,6 +216,7 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
 	return 0;
 }
 
+KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
 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)
@@ -240,8 +241,13 @@ 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;
+	int err;
+
+	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
+	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
+	KUNIT_ASSERT_EQ(test, err, 0);
 
 	cpus_read_lock();
 	/*
@@ -250,15 +256,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);
@@ -271,7 +277,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

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Nathan Chancellor]

On Wed, Jul 02, 2025 at 11:51:56AM +0200, 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.
> 
> 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>
> ---
>  kernel/printk/printk_ringbuffer_kunit_test.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
> index 217dcc14670c..0c3030fde8c2 100644
> --- a/kernel/printk/printk_ringbuffer_kunit_test.c
> +++ b/kernel/printk/printk_ringbuffer_kunit_test.c
> @@ -216,6 +216,7 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
>  	return 0;
>  }
>  
> +KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);

This appears to break the build for me when CONFIG_CPUMASK_OFFSTACK is not
set, like when enabling this test on top of x86_64 defconfig:

  In file included from kernel/printk/printk_ringbuffer_kunit_test.c:14:
  kernel/printk/printk_ringbuffer_kunit_test.c: In function 'prbtest_cpumask_cleanup':
  include/kunit/resource.h:409:32: error: cast specifies array type
    409 |                 arg_type arg = (arg_type)in;                    \
        |                                ^
  kernel/printk/printk_ringbuffer_kunit_test.c:226:1: note: in expansion of macro 'KUNIT_DEFINE_ACTION_WRAPPER'
    226 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Clang's error might be a little clearer with the "aka" note it provides.

  kernel/printk/printk_ringbuffer_kunit_test.c:226:1: error: used type 'cpumask_var_t' (aka 'struct cpumask[1]') where arithmetic or pointer type is required
    226 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/kunit/resource.h:409:18: note: expanded from macro 'KUNIT_DEFINE_ACTION_WRAPPER'
    409 |                 arg_type arg = (arg_type)in;                    \
        |                                ^         ~~

>  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)
> @@ -240,8 +241,13 @@ 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;
> +	int err;
> +
> +	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
> +	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
> +	KUNIT_ASSERT_EQ(test, err, 0);
>  
>  	cpus_read_lock();
>  	/*
> @@ -250,15 +256,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);
> @@ -271,7 +277,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
> 

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Petr Mladek]

On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
> On Wed, Jul 02, 2025 at 11:51:56AM +0200, 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.
> > 
> > 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>
> > ---
> >  kernel/printk/printk_ringbuffer_kunit_test.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
> > index 217dcc14670c..0c3030fde8c2 100644
> > --- a/kernel/printk/printk_ringbuffer_kunit_test.c
> > +++ b/kernel/printk/printk_ringbuffer_kunit_test.c
> > @@ -216,6 +216,7 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
> >  	return 0;
> >  }
> >  
> > +KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
> 
> This appears to break the build for me when CONFIG_CPUMASK_OFFSTACK is not
> set, like when enabling this test on top of x86_64 defconfig:
> 
>   In file included from kernel/printk/printk_ringbuffer_kunit_test.c:14:
>   kernel/printk/printk_ringbuffer_kunit_test.c: In function 'prbtest_cpumask_cleanup':
>   include/kunit/resource.h:409:32: error: cast specifies array type
>     409 |                 arg_type arg = (arg_type)in;                    \
>         |                                ^
>   kernel/printk/printk_ringbuffer_kunit_test.c:226:1: note: in expansion of macro 'KUNIT_DEFINE_ACTION_WRAPPER'
>     226 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
>         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Clang's error might be a little clearer with the "aka" note it provides.
> 
>   kernel/printk/printk_ringbuffer_kunit_test.c:226:1: error: used type 'cpumask_var_t' (aka 'struct cpumask[1]') where arithmetic or pointer type is required
>     226 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
>         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   include/kunit/resource.h:409:18: note: expanded from macro 'KUNIT_DEFINE_ACTION_WRAPPER'
>     409 |                 arg_type arg = (arg_type)in;                    \
>         |                                ^         ~~
> 
> >  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)

Thanks a lot for the nice report.

The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:

#ifdef CONFIG_CPUMASK_OFFSTACK
typedef struct cpumask *cpumask_var_t;
#else
typedef struct cpumask cpumask_var_t[1];
#endif /* CONFIG_CPUMASK_OFFSTACK */

And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
is a pointer.

I am going to solve this by adding a wrapper over free_cpumask_var()
which would work with a pointer to cpumask_var_t.

It has another catch, though. It seems that the automatic clean up is
done after test_readerwriter() finishes. Therefore the variable
@test_cpus can't be on stack.

Anyway, the following seems to work with both CONFIG_CPUMASK_OFFSTACK
enabled and disabled:

--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -223,7 +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, cpumask_var_t);
+/*
+ * Add a custom wrapper around free_cpumask_var() to be used by
+ * KUNIT_DEFINE_ACTION_WRAPPER(). It allows to pass @mask using
+ * a pointer even when CONFIG_CPUMASK_OFFSTACK is disabled.
+ */
+static void prbtest_free_cpumask_var(cpumask_var_t *mask)
+{
+	free_cpumask_var(*mask);
+}
+
+KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, prbtest_free_cpumask_var, cpumask_var_t *);
 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)
@@ -244,16 +254,19 @@ static void test_readerwriter(struct kunit *test)
 {
 	/* Equivalent to CONFIG_LOG_BUF_SHIFT=13 */
 	DEFINE_PRINTKRB(test_rb, 8, 5);
-
+	/*
+	 * @test_cpus can't be on stack. THe pointer to this variable is passed
+	 * to an automatic clean up action, see prbtest_free_cpumask_var().
+	 */
+	static cpumask_var_t test_cpus;
 	struct prbtest_thread_data *thread_data;
 	struct prbtest_data *test_data;
 	struct task_struct *thread;
-	cpumask_var_t test_cpus;
 	int cpu, reader_cpu;
 	int err;
 
 	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
-	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
+	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, &test_cpus);
 	KUNIT_ASSERT_EQ(test, err, 0);
 
 	cpus_read_lock();

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Arnd Bergmann]

On Tue, Jul 8, 2025, at 16:24, Petr Mladek wrote:
> On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
>> On Wed, Jul 02, 2025 at 11:51:56AM +0200, Petr Mladek wrote:
>
> Thanks a lot for the nice report.
>
> The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:
>
> #ifdef CONFIG_CPUMASK_OFFSTACK
> typedef struct cpumask *cpumask_var_t;
> #else
> typedef struct cpumask cpumask_var_t[1];
> #endif /* CONFIG_CPUMASK_OFFSTACK */
>
> And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
> is a pointer.
>
> I am going to solve this by adding a wrapper over free_cpumask_var()
> which would work with a pointer to cpumask_var_t.

I'm not familiar enough with the cleanup mechanism of kunit,
but can't you just move the mask allocation outside of
test_readerwriter()?

> + */
> +static void prbtest_free_cpumask_var(cpumask_var_t *mask)
> +{
> +	free_cpumask_var(*mask);
> +}

Or you could pass this as a cpumask_t pointer instead,
which should do the right thing without the indirection.
> 
>  	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
> -	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
> +	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, &test_cpus);

In my original version, I did not have the
KUNIT_ASSERT_TRUE() here, which seems sufficient since this
is not what you are testing at all, and in normal systems
this would just be a stack variable.

    Arnd

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Petr Mladek]

On Tue 2025-07-08 16:48:47, Arnd Bergmann wrote:
> On Tue, Jul 8, 2025, at 16:24, Petr Mladek wrote:
> > On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
> >> On Wed, Jul 02, 2025 at 11:51:56AM +0200, Petr Mladek wrote:
> >
> > Thanks a lot for the nice report.
> >
> > The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:
> >
> > #ifdef CONFIG_CPUMASK_OFFSTACK
> > typedef struct cpumask *cpumask_var_t;
> > #else
> > typedef struct cpumask cpumask_var_t[1];
> > #endif /* CONFIG_CPUMASK_OFFSTACK */
> >
> > And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
> > is a pointer.
> >
> > I am going to solve this by adding a wrapper over free_cpumask_var()
> > which would work with a pointer to cpumask_var_t.
> 
> I'm not familiar enough with the cleanup mechanism of kunit,
> but can't you just move the mask allocation outside of
> test_readerwriter()?

The only solution would be global variable.

test_readerwriter() is the top-level function passed
to KUnit framework via:

	KUNIT_CASE_SLOW(test_readerwriter),

And it seems that the clean is even done in a separate process.

I see the following:

   KUNIT_CASE_SLOW() sets .run_case()

The callback is called via via:

  + kunit_try_run_case()
   +  kunit_run_case_internal()
     + test_case->run_case()

And kunit_try_run_case() is called via:

    + kunit_run_case_catch_errors()
      + kunit_try_catch_run()
	+ kthread_create()
	     -> kunit_try_run_case() in the new thread

The clean up is called from the same kunit_run_case_catch_errors()
in another thread

      + kunit_try_catch_run()
	+ kthread_create()
	    -> kunit_try_run_case_cleanup() in another new thread

> > + */
> > +static void prbtest_free_cpumask_var(cpumask_var_t *mask)
> > +{
> > +	free_cpumask_var(*mask);
> > +}
> 
> Or you could pass this as a cpumask_t pointer instead,
> which should do the right thing without the indirection.

Nice trick. I am going to try it.

> >  	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
> > -	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
> > +	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, &test_cpus);
> 
> In my original version, I did not have the
> KUNIT_ASSERT_TRUE() here, which seems sufficient since this
> is not what you are testing at all, and in normal systems
> this would just be a stack variable.

I think that KUNIT_ASSERT is standard handling of any problem in the
test. At least, I see KUNIT_ASSERT*() after any *malloc*() in
the code samples in Documentation/dev-tools/kunit/usage.rst

Best Regards,
Petr

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Thomas Weißschuh]

On Wed, Jul 09, 2025 at 01:36:14PM +0200, Petr Mladek wrote:
> On Tue 2025-07-08 16:48:47, Arnd Bergmann wrote:
> > On Tue, Jul 8, 2025, at 16:24, Petr Mladek wrote:
> > > On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
> > >> On Wed, Jul 02, 2025 at 11:51:56AM +0200, Petr Mladek wrote:
> > >
> > > Thanks a lot for the nice report.
> > >
> > > The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:
> > >
> > > #ifdef CONFIG_CPUMASK_OFFSTACK
> > > typedef struct cpumask *cpumask_var_t;
> > > #else
> > > typedef struct cpumask cpumask_var_t[1];
> > > #endif /* CONFIG_CPUMASK_OFFSTACK */
> > >
> > > And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
> > > is a pointer.
> > >
> > > I am going to solve this by adding a wrapper over free_cpumask_var()
> > > which would work with a pointer to cpumask_var_t.
> > 
> > I'm not familiar enough with the cleanup mechanism of kunit,
> > but can't you just move the mask allocation outside of
> > test_readerwriter()?
> 
> The only solution would be global variable.

When the cpumask is allocated on the stack, free_cpumask_var() is a no-op.
So while the stack address would be leaked to another thread,
it should be fine as nothing is ever done with it.
For more clarity it could also be gated explicitly:

if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {
	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
	KUNIT_ASSERT_EQ(test, err, 0);
}

<snip>

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Petr Mladek]

On Wed 2025-07-09 14:53:29, Thomas Weißschuh wrote:
> On Wed, Jul 09, 2025 at 01:36:14PM +0200, Petr Mladek wrote:
> > On Tue 2025-07-08 16:48:47, Arnd Bergmann wrote:
> > > On Tue, Jul 8, 2025, at 16:24, Petr Mladek wrote:
> > > > On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
> > > >> On Wed, Jul 02, 2025 at 11:51:56AM +0200, Petr Mladek wrote:
> > > >
> > > > Thanks a lot for the nice report.
> > > >
> > > > The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:
> > > >
> > > > #ifdef CONFIG_CPUMASK_OFFSTACK
> > > > typedef struct cpumask *cpumask_var_t;
> > > > #else
> > > > typedef struct cpumask cpumask_var_t[1];
> > > > #endif /* CONFIG_CPUMASK_OFFSTACK */
> > > >
> > > > And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
> > > > is a pointer.
> > > >
> > > > I am going to solve this by adding a wrapper over free_cpumask_var()
> > > > which would work with a pointer to cpumask_var_t.
> > > 
> > > I'm not familiar enough with the cleanup mechanism of kunit,
> > > but can't you just move the mask allocation outside of
> > > test_readerwriter()?
> > 
> > The only solution would be global variable.
> 
> When the cpumask is allocated on the stack, free_cpumask_var() is a no-op.
> So while the stack address would be leaked to another thread,
> it should be fine as nothing is ever done with it.
> For more clarity it could also be gated explicitly:
> 
> if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {
> 	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
> 	KUNIT_ASSERT_EQ(test, err, 0);
> }

It is likely a matter of taste but I like this idea. It looks better
than passing an invalid pointer and hope nobody would do anything
with it.

The only problem is that

    if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {

did not prevented the compiler warning. I guess that the code was still
compiled and later just optimized out.

So, I am going to use #ifdef instead. I create a function:

/*
 * 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

which will be called in test_readerwriter().

It seems to work, ..., sigh. I did not expect so many troubles with
a tiny detail.

Best Regards,
Petr

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Arnd Bergmann]

On Thu, Jul 10, 2025, at 15:51, Petr Mladek wrote:
> On Wed 2025-07-09 14:53:29, Thomas Weißschuh wrote:
>> On Wed, Jul 09, 2025 at 01:36:14PM +0200, Petr Mladek wrote:
>> if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {
>> 	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
>> 	KUNIT_ASSERT_EQ(test, err, 0);
>> }
>
> It is likely a matter of taste but I like this idea. It looks better
> than passing an invalid pointer and hope nobody would do anything
> with it.
>
> The only problem is that
>
>     if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {
>
> did not prevented the compiler warning. I guess that the code was still
> compiled and later just optimized out.

Right, gcc does some of the warnings after dead code eliminations
and some before. clang tries to do all warnings before eliminating
dead code, so you still lose.

> /*
>  * 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
>
> which will be called in test_readerwriter().

Looks fine to me

> It seems to work, ..., sigh. I did not expect so many troubles with
> a tiny detail.

I wonder if just making the cpumask_t 'static' would still be
simpler, given that there are no concurrent callers.

    Arnd

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[Petr Mladek]

On Tue 2025-07-08 16:48:47, Arnd Bergmann wrote:
> On Tue, Jul 8, 2025, at 16:24, Petr Mladek wrote:
> > On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
> >> On Wed, Jul 02, 2025 at 11:51:56AM +0200, Petr Mladek wrote:
> >
> > Thanks a lot for the nice report.
> >
> > The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:
> >
> > #ifdef CONFIG_CPUMASK_OFFSTACK
> > typedef struct cpumask *cpumask_var_t;
> > #else
> > typedef struct cpumask cpumask_var_t[1];
> > #endif /* CONFIG_CPUMASK_OFFSTACK */
> >
> > And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
> > is a pointer.
> >
> > I am going to solve this by adding a wrapper over free_cpumask_var()
> > which would work with a pointer to cpumask_var_t.
> 
> I'm not familiar enough with the cleanup mechanism of kunit,
> but can't you just move the mask allocation outside of
> test_readerwriter()?
> 
> > + */
> > +static void prbtest_free_cpumask_var(cpumask_var_t *mask)
> > +{
> > +	free_cpumask_var(*mask);
> > +}
> 
> Or you could pass this as a cpumask_t pointer instead,
> which should do the right thing without the indirection.

It was actually enough to use this in the KUNIT_DEFINE_ACTION_WRAPPER()
definition. It removed the warning about passing an array type.
And I needed neither a wrapper not #ifdef.

I am going to send an updated version of the patch. I am sorry
that it took so long. This patch has fallen though cracks because
of vacation and other urgent tasks.

Best Regards,
Petr

Re: [PATCH 2/3] printk: kunit: support offstack cpumask

[kernel test robot]

Hi Petr,

kernel test robot noticed the following build errors:

[auto build test ERROR on linux-next/master]
[cannot apply to linus/master v6.16-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Petr-Mladek/printk-ringbuffer-Explain-why-the-KUnit-test-ignores-failed-writes/20250702-175422
base:   linux-next/master
patch link:    https://lore.kernel.org/r/20250702095157.110916-3-pmladek%40suse.com
patch subject: [PATCH 2/3] printk: kunit: support offstack cpumask
config: riscv-randconfig-001-20250703 (https://download.01.org/0day-ci/archive/20250703/202507032226.1sEv2EJM-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250703/202507032226.1sEv2EJM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507032226.1sEv2EJM-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/printk/printk_ringbuffer_kunit_test.c:14:
   kernel/printk/printk_ringbuffer_kunit_test.c: In function 'prbtest_cpumask_cleanup':
>> include/kunit/resource.h:409:32: error: cast specifies array type
     409 |                 arg_type arg = (arg_type)in;                    \
         |                                ^
   kernel/printk/printk_ringbuffer_kunit_test.c:219:1: note: in expansion of macro 'KUNIT_DEFINE_ACTION_WRAPPER'
     219 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from printk_ringbuffer_kunit_test.c:14:
   printk_ringbuffer_kunit_test.c: In function 'prbtest_cpumask_cleanup':
>> include/kunit/resource.h:409:32: error: cast specifies array type
     409 |                 arg_type arg = (arg_type)in;                    \
         |                                ^
   printk_ringbuffer_kunit_test.c:219:1: note: in expansion of macro 'KUNIT_DEFINE_ACTION_WRAPPER'
     219 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +409 include/kunit/resource.h

b9dce8a1ed3efe David Gow 2023-05-25  392  
56778b49c9a2cb David Gow 2023-11-28  393  /**
56778b49c9a2cb David Gow 2023-11-28  394   * KUNIT_DEFINE_ACTION_WRAPPER() - Wrap a function for use as a deferred action.
56778b49c9a2cb David Gow 2023-11-28  395   *
56778b49c9a2cb David Gow 2023-11-28  396   * @wrapper: The name of the new wrapper function define.
56778b49c9a2cb David Gow 2023-11-28  397   * @orig: The original function to wrap.
56778b49c9a2cb David Gow 2023-11-28  398   * @arg_type: The type of the argument accepted by @orig.
56778b49c9a2cb David Gow 2023-11-28  399   *
56778b49c9a2cb David Gow 2023-11-28  400   * Defines a wrapper for a function which accepts a single, pointer-sized
56778b49c9a2cb David Gow 2023-11-28  401   * argument. This wrapper can then be passed to kunit_add_action() and
56778b49c9a2cb David Gow 2023-11-28  402   * similar. This should be used in preference to casting a function
56778b49c9a2cb David Gow 2023-11-28  403   * directly to kunit_action_t, as casting function pointers will break
56778b49c9a2cb David Gow 2023-11-28  404   * control flow integrity (CFI), leading to crashes.
56778b49c9a2cb David Gow 2023-11-28  405   */
56778b49c9a2cb David Gow 2023-11-28  406  #define KUNIT_DEFINE_ACTION_WRAPPER(wrapper, orig, arg_type)	\
56778b49c9a2cb David Gow 2023-11-28  407  	static void wrapper(void *in)				\
56778b49c9a2cb David Gow 2023-11-28  408  	{							\
56778b49c9a2cb David Gow 2023-11-28 @409  		arg_type arg = (arg_type)in;			\
56778b49c9a2cb David Gow 2023-11-28  410  		orig(arg);					\
56778b49c9a2cb David Gow 2023-11-28  411  	}
56778b49c9a2cb David Gow 2023-11-28  412  
56778b49c9a2cb David Gow 2023-11-28  413  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 3/3] printk: kunit: Fix __counted_by() in struct prbtest_rbdata

[Petr Mladek]

__counted_by() has to point to a variable which defines the size
of the related array. The code must never access the array
beyond this limit.

struct prbtest_rbdata currently stores the length of the string.
And the code access the array beyond the limit when writing
or reading the trailing '\0'.

Store the size of the string, including the trailing '\0' if
we wanted to keep __counted_by().

Consistently use "_size" suffix when the trailing '\0' is counted.
Note that MAX_RBDATA_TEXT_SIZE was originally used to limit
the text length.

When touching the code, make sure that @text_size produced by
get_random_u32_inclusive() stays within the limits.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/eaea66b9-266a-46e7-980d-33f40ad4b215@sabinyo.mountain
Suggested-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk_ringbuffer_kunit_test.c | 47 +++++++++++---------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
index 0c3030fde8c2..088fe4d8c9b6 100644
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -52,13 +52,12 @@ module_param(runtime_ms, ulong, 0400);
 
 /* test data structure */
 struct prbtest_rbdata {
-	unsigned int len;
-	char text[] __counted_by(len);
+	unsigned int size;
+	char text[] __counted_by(size);
 };
 
-#define MAX_RBDATA_TEXT_SIZE 0x7f
-/* +1 for terminator. */
-#define MAX_PRB_RECORD_SIZE (sizeof(struct prbtest_rbdata) + MAX_RBDATA_TEXT_SIZE + 1)
+#define MAX_RBDATA_TEXT_SIZE 0x80
+#define MAX_PRB_RECORD_SIZE (sizeof(struct prbtest_rbdata) + MAX_RBDATA_TEXT_SIZE)
 
 struct prbtest_data {
 	struct kunit *test;
@@ -74,25 +73,29 @@ struct prbtest_thread_data {
 
 static void prbtest_fail_record(struct kunit *test, const struct prbtest_rbdata *dat, u64 seq)
 {
-	KUNIT_FAIL(test, "BAD RECORD: seq=%llu len=%u text=%.*s\n",
-		   seq, dat->len,
-		   dat->len <= MAX_RBDATA_TEXT_SIZE ? dat->len : -1,
-		   dat->len <= MAX_RBDATA_TEXT_SIZE ? dat->text : "<invalid>");
+	unsigned int len;
+
+	len = dat->size - 1;
+
+	KUNIT_FAIL(test, "BAD RECORD: seq=%llu size=%u text=%.*s\n",
+		   seq, dat->size,
+		   len < MAX_RBDATA_TEXT_SIZE ? len : -1,
+		   len < MAX_RBDATA_TEXT_SIZE ? dat->text : "<invalid>");
 }
 
 static bool prbtest_check_data(const struct prbtest_rbdata *dat)
 {
 	unsigned int len;
 
-	/* Sane length? */
-	if (dat->len < 1 || dat->len > MAX_RBDATA_TEXT_SIZE)
+	/* Sane size? At least one character + trailing '\0' */
+	if (dat->size < 2 || dat->size > MAX_RBDATA_TEXT_SIZE)
 		return false;
 
-	if (dat->text[dat->len] != '\0')
+	len = dat->size - 1;
+	if (dat->text[len] != '\0')
 		return false;
 
 	/* String repeats with the same character? */
-	len = dat->len;
 	while (len--) {
 		if (dat->text[len] != dat->text[0])
 			return false;
@@ -114,10 +117,14 @@ static int prbtest_writer(void *data)
 	kunit_info(tr->test_data->test, "start thread %03lu (writer)\n", tr->num);
 
 	for (;;) {
-		/* ensure at least 1 character */
-		text_size = get_random_u32_inclusive(1, MAX_RBDATA_TEXT_SIZE);
-		/* +1 for terminator. */
-		record_size = sizeof(struct prbtest_rbdata) + text_size + 1;
+		/* ensure at least 1 character + trailing '\0' */
+		text_size = get_random_u32_inclusive(2, MAX_RBDATA_TEXT_SIZE);
+		if (WARN_ON_ONCE(text_size < 2))
+			text_size = 2;
+		if (WARN_ON_ONCE(text_size > MAX_RBDATA_TEXT_SIZE))
+			text_size = MAX_RBDATA_TEXT_SIZE;
+
+		record_size = sizeof(struct prbtest_rbdata) + text_size;
 		WARN_ON_ONCE(record_size > MAX_PRB_RECORD_SIZE);
 
 		/* specify the text sizes for reservation */
@@ -140,9 +147,9 @@ static int prbtest_writer(void *data)
 			r.info->text_len = record_size;
 
 			dat = (struct prbtest_rbdata *)r.text_buf;
-			dat->len = text_size;
-			memset(dat->text, text_id, text_size);
-			dat->text[text_size] = 0;
+			dat->size = text_size;
+			memset(dat->text, text_id, text_size - 1);
+			dat->text[text_size - 1] = '\0';
 
 			prb_commit(&e);
 
-- 
2.50.0

Re: [PATCH 3/3] printk: kunit: Fix __counted_by() in struct prbtest_rbdata

[John Ogness]

On 2025-07-02, Petr Mladek <pmladek@suse.com> wrote:
> diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
> index 0c3030fde8c2..088fe4d8c9b6 100644
> --- a/kernel/printk/printk_ringbuffer_kunit_test.c
> +++ b/kernel/printk/printk_ringbuffer_kunit_test.c
> @@ -114,10 +117,14 @@ static int prbtest_writer(void *data)
>  	kunit_info(tr->test_data->test, "start thread %03lu (writer)\n", tr->num);
>  
>  	for (;;) {
> -		/* ensure at least 1 character */
> -		text_size = get_random_u32_inclusive(1, MAX_RBDATA_TEXT_SIZE);
> -		/* +1 for terminator. */
> -		record_size = sizeof(struct prbtest_rbdata) + text_size + 1;
> +		/* ensure at least 1 character + trailing '\0' */
> +		text_size = get_random_u32_inclusive(2, MAX_RBDATA_TEXT_SIZE);
> +		if (WARN_ON_ONCE(text_size < 2))
> +			text_size = 2;
> +		if (WARN_ON_ONCE(text_size > MAX_RBDATA_TEXT_SIZE))
> +			text_size = MAX_RBDATA_TEXT_SIZE;

IMHO the WARN_ON_ONCE's are excessive. It is allowed to expect that
get_random_u32_inclusive() works correctly. But I am fine with it.

Reviewed-by: John Ogness <john.ogness@linutronix.de>

Re: [PATCH 0/3] printk: KUnit: Followup fixes for the new KUnit test

[Thomas Weißschuh]

On Wed, Jul 02, 2025 at 11:51:54AM +0200, Petr Mladek wrote:
> Hi,
> 
> this patchset puts together some followup fixes for the new KUnit test
> which were discussed on several locations.

<snip>

> Arnd Bergmann (1):
>   printk: kunit: support offstack cpumask
> 
> Petr Mladek (2):
>   printk: ringbuffer: Explain why the KUnit test ignores failed writes
>   printk: kunit: Fix __counted_by() in struct prbtest_rbdata
> 
>  kernel/printk/printk_ringbuffer_kunit_test.c | 78 +++++++++++++-------
>  1 file changed, 52 insertions(+), 26 deletions(-)

For the series:

Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Re: [PATCH 0/3] printk: KUnit: Followup fixes for the new KUnit test

[Petr Mladek]

On Wed 2025-07-02 11:51:54, Petr Mladek wrote:
> Hi,
> 
> this patchset puts together some followup fixes for the new KUnit test
> which were discussed on several locations.
> 
> 1st patch:
> 
>   + adds a comment exaplaing why the test ignores pr_reserve() failures.
> 
>   + was proposed at https://lore.kernel.org/r/aFUiQESkXjFIGqez@pathway.suse.cz
> 
>   + Thomas Weißschuh added into v4 of the original patch but I have already
>     comitted v3 in the meantime, see
>     https://lore.kernel.org/r/20250620-printk-ringbuffer-test-v4-1-8df873f1f3e0@linutronix.de
> 
> 
> 2nd patch:
> 
>   + dynamically allocates a cpu bitmap to make the code safe even on systems
>     with many CPUs.
> 
>   + v1 was set by Arnd Bergmann but it had some problems, see
>     https://lore.kernel.org/r/20250620192554.2234184-1-arnd@kernel.org
> 
>   + This version just integreates the proposed fixes from
>     https://lore.kernel.org/r/aFkuqaFn3BOvsPT-@pathway.suse.cz
> 
> 
> 3rd patch:
> 
>   + stores "size" instead on "len" in struct prbtest_rbdata so that
>     is can be used to check code sanity by __counted_by(size).
> 
>   + fixes https://lore.kernel.org/r/eaea66b9-266a-46e7-980d-33f40ad4b215@sabinyo.mountain
> 
>   + it is based on the idea from Thomas Weißschuh, see
>     20250626082605-c5fbbb88-f6cc-4659-bea0-a283cdb58e81@linutronix.de


JFYI, the 1st and 3rd patch has been committed into printk/linux.git,
branch rework/ringbuffer-kunit-test.

These two patches were reviewed by Thomas and John and were accepted.

The 2nd patch is independent and has an issue. I am going to send
an update separately.

Best Regards,
Petr