Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-18 Thread Masami Hiramatsu
On Thu, 18 Feb 2021 09:36:36 +0100
Uladzislau Rezki  wrote:

> On Thu, Feb 18, 2021 at 02:03:07PM +0900, Masami Hiramatsu wrote:
> > On Wed, 17 Feb 2021 10:17:38 -0800
> > "Paul E. McKenney"  wrote:
> > 
> > > > > 1.Spawn ksoftirqd earlier.
> > > > > 
> > > > > 2.Suppress attempts to awaken ksoftirqd before it exists,
> > > > >   forcing all ksoftirq execution on the back of interrupts.
> > > > > 
> > > > > Uladzislau and I each produced patches for #1, and I produced a patch
> > > > > for #2.
> > > > > 
> > > > > The only other option I know of is to push the call to init_kprobes()
> > > > > later in the boot sequence, perhaps to its original subsys_initcall(),
> > > > > or maybe only as late as core_initcall().  I added Masami and Steve on
> > > > > CC for their thoughts on this.
> > > > > 
> > > > > Is there some other proper fix that I am missing?
> > > > 
> > > > Oh, I missed that the synchronize_rcu_tasks() will be involved the 
> > > > kprobes
> > > > in early stage. Does the problem only exist in the 
> > > > synchronize_rcu_tasks()
> > > > instead of synchronize_rcu()? If so I can just stop optimizer in early 
> > > > stage
> > > > because I just want to enable kprobes in early stage, but not optprobes.
> > > > 
> > > > Does the following patch help?
> > > 
> > > It does look to me like it would!  I clearly should have asked you about
> > > this a couple of months ago.  ;-)
> > > 
> > > The proof of the pudding would be whether the powerpc guys can apply
> > > this to v5.10-rc7 and have their kernel come up without hanging at boot.
> > 
> > Who could I ask for testing this patch, Uladzislau?
> > I think the test machine which enough slow or the kernel has much initcall
> > to run optimization thread while booting.
> > In my environment, I could not reproduce that issue because the optimizer
> > was sheduled after some tick passed. At that point, ksoftirqd has already
> > been initialized.
> > 
> From my end i did some simulation and had a look at your change. So the
> patch works on my setup. I see that optimization of kprobes is deferred
> and can be initiated only from the subsys_initcall() phase. So the sequence
> should be correct for v5.10-rc7:
> 
> 1. ksoftirq is setup early_initcall();

also kprobe framework is initialized in early_initcall() and
kprobes smoketest (register/unregister probes) executed.

> 2. rcu_spawn_tasks_* are setup in the core_initcall();

and kprobe events are setup in core_initcall_sync() (via trace_boot);

> 3. an optimization of kprobes are invoked from subsys_initcall().

   At this point, all kprobes which can be optimized will be actually
   optimized.


> For real test on power-pc you can ask Daniel Axtens  for 
> help. 

OK, I'll CC to Daniel.

Thank you,

-- 
Masami Hiramatsu 


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-18 Thread Uladzislau Rezki
On Thu, Feb 18, 2021 at 02:03:07PM +0900, Masami Hiramatsu wrote:
> On Wed, 17 Feb 2021 10:17:38 -0800
> "Paul E. McKenney"  wrote:
> 
> > > > 1.  Spawn ksoftirqd earlier.
> > > > 
> > > > 2.  Suppress attempts to awaken ksoftirqd before it exists,
> > > > forcing all ksoftirq execution on the back of interrupts.
> > > > 
> > > > Uladzislau and I each produced patches for #1, and I produced a patch
> > > > for #2.
> > > > 
> > > > The only other option I know of is to push the call to init_kprobes()
> > > > later in the boot sequence, perhaps to its original subsys_initcall(),
> > > > or maybe only as late as core_initcall().  I added Masami and Steve on
> > > > CC for their thoughts on this.
> > > > 
> > > > Is there some other proper fix that I am missing?
> > > 
> > > Oh, I missed that the synchronize_rcu_tasks() will be involved the kprobes
> > > in early stage. Does the problem only exist in the synchronize_rcu_tasks()
> > > instead of synchronize_rcu()? If so I can just stop optimizer in early 
> > > stage
> > > because I just want to enable kprobes in early stage, but not optprobes.
> > > 
> > > Does the following patch help?
> > 
> > It does look to me like it would!  I clearly should have asked you about
> > this a couple of months ago.  ;-)
> > 
> > The proof of the pudding would be whether the powerpc guys can apply
> > this to v5.10-rc7 and have their kernel come up without hanging at boot.
> 
> Who could I ask for testing this patch, Uladzislau?
> I think the test machine which enough slow or the kernel has much initcall
> to run optimization thread while booting.
> In my environment, I could not reproduce that issue because the optimizer
> was sheduled after some tick passed. At that point, ksoftirqd has already
> been initialized.
> 
>From my end i did some simulation and had a look at your change. So the
patch works on my setup. I see that optimization of kprobes is deferred
and can be initiated only from the subsys_initcall() phase. So the sequence
should be correct for v5.10-rc7:

1. ksoftirq is setup early_initcall();
2. rcu_spawn_tasks_* are setup in the core_initcall();
3. an optimization of kprobes are invoked from subsys_initcall().

For real test on power-pc you can ask Daniel Axtens  for help. 

--
Vlad Rezki


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-17 Thread Masami Hiramatsu
On Wed, 17 Feb 2021 10:17:38 -0800
"Paul E. McKenney"  wrote:

> > > 1.Spawn ksoftirqd earlier.
> > > 
> > > 2.Suppress attempts to awaken ksoftirqd before it exists,
> > >   forcing all ksoftirq execution on the back of interrupts.
> > > 
> > > Uladzislau and I each produced patches for #1, and I produced a patch
> > > for #2.
> > > 
> > > The only other option I know of is to push the call to init_kprobes()
> > > later in the boot sequence, perhaps to its original subsys_initcall(),
> > > or maybe only as late as core_initcall().  I added Masami and Steve on
> > > CC for their thoughts on this.
> > > 
> > > Is there some other proper fix that I am missing?
> > 
> > Oh, I missed that the synchronize_rcu_tasks() will be involved the kprobes
> > in early stage. Does the problem only exist in the synchronize_rcu_tasks()
> > instead of synchronize_rcu()? If so I can just stop optimizer in early stage
> > because I just want to enable kprobes in early stage, but not optprobes.
> > 
> > Does the following patch help?
> 
> It does look to me like it would!  I clearly should have asked you about
> this a couple of months ago.  ;-)
> 
> The proof of the pudding would be whether the powerpc guys can apply
> this to v5.10-rc7 and have their kernel come up without hanging at boot.

Who could I ask for testing this patch, Uladzislau?
I think the test machine which enough slow or the kernel has much initcall
to run optimization thread while booting.
In my environment, I could not reproduce that issue because the optimizer
was sheduled after some tick passed. At that point, ksoftirqd has already
been initialized.

Thank you,


-- 
Masami Hiramatsu 


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-17 Thread Paul E. McKenney
On Wed, Feb 17, 2021 at 11:47:59PM +0900, Masami Hiramatsu wrote:
> On Tue, 16 Feb 2021 09:30:03 -0800
> "Paul E. McKenney"  wrote:
> 
> > On Mon, Feb 15, 2021 at 12:28:26PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2021-02-13 08:45:54 [-0800], Paul E. McKenney wrote:
> > > > Glad you like it!  But let's see which (if any) of these patches solves
> > > > the problem for Sebastian.
> > > 
> > > Looking at that, is there any reason for doing this that can not be
> > > solved by moving the self-test a little later? Maybe once we reached at
> > > least SYSTEM_SCHEDULING?
> > 
> > One problem is that ksoftirqd and the kprobes use are early_initcall(),
> > so we cannot count on ksoftirqd being spawned when kprobes first uses
> > synchronize_rcu_tasks().  Moving the selftest later won't fix this
> > problem, but rather just paper it over.
> > 
> > > This happens now even before lockdep is up or the console is registered.
> > > So if something bad happens, you end up with a blank terminal.
> > 
> > I was getting a splat, but I could easily believe that there are
> > configurations where the hang is totally silent.  In other words, I do
> > agree that this needs a proper fix.  All we need do is work out an
> > agreeable value of "proper".  ;-)
> > 
> > > There is nothing else that early in the boot process that requires
> > > working softirq. The only exception to this is wait_task_inactive()
> > > which is used while starting a new thread (including the ksoftirqd)
> > > which is why it was moved to schedule_hrtimeout().
> > 
> > Moving kprobes initialization to early_initcall() [1] means that there
> > can be a call to synchronize_rcu_tasks() before the current spawning of
> > ksoftirqd.  Because synchronize_rcu_tasks() needs timers to work, it needs
> > softirq to work.  I know two straightforward ways to make that happen:
> > 
> > 1.  Spawn ksoftirqd earlier.
> > 
> > 2.  Suppress attempts to awaken ksoftirqd before it exists,
> > forcing all ksoftirq execution on the back of interrupts.
> > 
> > Uladzislau and I each produced patches for #1, and I produced a patch
> > for #2.
> > 
> > The only other option I know of is to push the call to init_kprobes()
> > later in the boot sequence, perhaps to its original subsys_initcall(),
> > or maybe only as late as core_initcall().  I added Masami and Steve on
> > CC for their thoughts on this.
> > 
> > Is there some other proper fix that I am missing?
> 
> Oh, I missed that the synchronize_rcu_tasks() will be involved the kprobes
> in early stage. Does the problem only exist in the synchronize_rcu_tasks()
> instead of synchronize_rcu()? If so I can just stop optimizer in early stage
> because I just want to enable kprobes in early stage, but not optprobes.
> 
> Does the following patch help?

It does look to me like it would!  I clearly should have asked you about
this a couple of months ago.  ;-)

The proof of the pudding would be whether the powerpc guys can apply
this to v5.10-rc7 and have their kernel come up without hanging at boot.

Thanx, Paul

> >From e5fafcda3ff918cd52619f795a3f22fb95c72b11 Mon Sep 17 00:00:00 2001
> From: Masami Hiramatsu 
> Date: Wed, 17 Feb 2021 23:35:20 +0900
> Subject: [PATCH] kprobes: Fix to delay the kprobes jump optimization
> 
> Since the kprobes jump optimization involves synchronize_rcu_tasks()
> which depends on the ksoftirqd, that can not be enabled at the
> early_initcall() boot stage. So this makes the kprobe optimization
> disabled in the early_initcall() and enables it in subsys_initcall().
> 
> Note that non-optimized kprobes is still available after
> early_initcall(). Only jump optimization is delayed.
> 
> Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> Signed-off-by: Masami Hiramatsu 
> ---
>  kernel/kprobes.c | 31 +--
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d5a3eb74a657..779d8322e307 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -861,7 +861,6 @@ static void try_to_optimize_kprobe(struct kprobe *p)
>   cpus_read_unlock();
>  }
>  
> -#ifdef CONFIG_SYSCTL
>  static void optimize_all_kprobes(void)
>  {
>   struct hlist_head *head;
> @@ -887,6 +886,7 @@ static void optimize_all_kprobes(void)
>   mutex_unlock(&kprobe_mutex);
>  }
>  
> +#ifdef CONFIG_SYSCTL
>  static void unoptimize_all_kprobes(void)
>  {
>   struct hlist_head *head;
> @@ -2497,18 +2497,14 @@ static int __init init_kprobes(void)
>   }
>   }
>  
> -#if defined(CONFIG_OPTPROBES)
> -#if defined(__ARCH_WANT_KPROBES_INSN_SLOT)
> - /* Init kprobe_optinsn_slots */
> - kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
> -#endif
> - /* By default, kprobes can be optimized */
> - kprobes_allow_optimization = true;
> -#endif
> -
>   /* By default, kprobes are armed */
>   kprobes_all_disarmed = false;
>  
> +#if de

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-17 Thread Masami Hiramatsu
On Tue, 16 Feb 2021 09:30:03 -0800
"Paul E. McKenney"  wrote:

> On Mon, Feb 15, 2021 at 12:28:26PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2021-02-13 08:45:54 [-0800], Paul E. McKenney wrote:
> > > Glad you like it!  But let's see which (if any) of these patches solves
> > > the problem for Sebastian.
> > 
> > Looking at that, is there any reason for doing this that can not be
> > solved by moving the self-test a little later? Maybe once we reached at
> > least SYSTEM_SCHEDULING?
> 
> One problem is that ksoftirqd and the kprobes use are early_initcall(),
> so we cannot count on ksoftirqd being spawned when kprobes first uses
> synchronize_rcu_tasks().  Moving the selftest later won't fix this
> problem, but rather just paper it over.
> 
> > This happens now even before lockdep is up or the console is registered.
> > So if something bad happens, you end up with a blank terminal.
> 
> I was getting a splat, but I could easily believe that there are
> configurations where the hang is totally silent.  In other words, I do
> agree that this needs a proper fix.  All we need do is work out an
> agreeable value of "proper".  ;-)
> 
> > There is nothing else that early in the boot process that requires
> > working softirq. The only exception to this is wait_task_inactive()
> > which is used while starting a new thread (including the ksoftirqd)
> > which is why it was moved to schedule_hrtimeout().
> 
> Moving kprobes initialization to early_initcall() [1] means that there
> can be a call to synchronize_rcu_tasks() before the current spawning of
> ksoftirqd.  Because synchronize_rcu_tasks() needs timers to work, it needs
> softirq to work.  I know two straightforward ways to make that happen:
> 
> 1.Spawn ksoftirqd earlier.
> 
> 2.Suppress attempts to awaken ksoftirqd before it exists,
>   forcing all ksoftirq execution on the back of interrupts.
> 
> Uladzislau and I each produced patches for #1, and I produced a patch
> for #2.
> 
> The only other option I know of is to push the call to init_kprobes()
> later in the boot sequence, perhaps to its original subsys_initcall(),
> or maybe only as late as core_initcall().  I added Masami and Steve on
> CC for their thoughts on this.
> 
> Is there some other proper fix that I am missing?

Oh, I missed that the synchronize_rcu_tasks() will be involved the kprobes
in early stage. Does the problem only exist in the synchronize_rcu_tasks()
instead of synchronize_rcu()? If so I can just stop optimizer in early stage
because I just want to enable kprobes in early stage, but not optprobes.

Does the following patch help?

>From e5fafcda3ff918cd52619f795a3f22fb95c72b11 Mon Sep 17 00:00:00 2001
From: Masami Hiramatsu 
Date: Wed, 17 Feb 2021 23:35:20 +0900
Subject: [PATCH] kprobes: Fix to delay the kprobes jump optimization

Since the kprobes jump optimization involves synchronize_rcu_tasks()
which depends on the ksoftirqd, that can not be enabled at the
early_initcall() boot stage. So this makes the kprobe optimization
disabled in the early_initcall() and enables it in subsys_initcall().

Note that non-optimized kprobes is still available after
early_initcall(). Only jump optimization is delayed.

Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
Signed-off-by: Masami Hiramatsu 
---
 kernel/kprobes.c | 31 +--
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d5a3eb74a657..779d8322e307 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -861,7 +861,6 @@ static void try_to_optimize_kprobe(struct kprobe *p)
cpus_read_unlock();
 }
 
-#ifdef CONFIG_SYSCTL
 static void optimize_all_kprobes(void)
 {
struct hlist_head *head;
@@ -887,6 +886,7 @@ static void optimize_all_kprobes(void)
mutex_unlock(&kprobe_mutex);
 }
 
+#ifdef CONFIG_SYSCTL
 static void unoptimize_all_kprobes(void)
 {
struct hlist_head *head;
@@ -2497,18 +2497,14 @@ static int __init init_kprobes(void)
}
}
 
-#if defined(CONFIG_OPTPROBES)
-#if defined(__ARCH_WANT_KPROBES_INSN_SLOT)
-   /* Init kprobe_optinsn_slots */
-   kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
-#endif
-   /* By default, kprobes can be optimized */
-   kprobes_allow_optimization = true;
-#endif
-
/* By default, kprobes are armed */
kprobes_all_disarmed = false;
 
+#if defined(CONFIG_OPTPROBES) && defined(__ARCH_WANT_KPROBES_INSN_SLOT)
+   /* Init kprobe_optinsn_slots for allocation */
+   kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
+#endif
+
err = arch_init_kprobes();
if (!err)
err = register_die_notifier(&kprobe_exceptions_nb);
@@ -2523,6 +2519,21 @@ static int __init init_kprobes(void)
 }
 early_initcall(init_kprobes);
 
+#if defined(CONFIG_OPTPROBES)
+static int __init init_optprobes(void)
+{
+   /*
+* Enable kprobe optimization - this kicks the optimizer which
+* depend

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-16 Thread Paul E. McKenney
On Mon, Feb 15, 2021 at 12:28:26PM +0100, Sebastian Andrzej Siewior wrote:
> On 2021-02-13 08:45:54 [-0800], Paul E. McKenney wrote:
> > Glad you like it!  But let's see which (if any) of these patches solves
> > the problem for Sebastian.
> 
> Looking at that, is there any reason for doing this that can not be
> solved by moving the self-test a little later? Maybe once we reached at
> least SYSTEM_SCHEDULING?

One problem is that ksoftirqd and the kprobes use are early_initcall(),
so we cannot count on ksoftirqd being spawned when kprobes first uses
synchronize_rcu_tasks().  Moving the selftest later won't fix this
problem, but rather just paper it over.

> This happens now even before lockdep is up or the console is registered.
> So if something bad happens, you end up with a blank terminal.

I was getting a splat, but I could easily believe that there are
configurations where the hang is totally silent.  In other words, I do
agree that this needs a proper fix.  All we need do is work out an
agreeable value of "proper".  ;-)

> There is nothing else that early in the boot process that requires
> working softirq. The only exception to this is wait_task_inactive()
> which is used while starting a new thread (including the ksoftirqd)
> which is why it was moved to schedule_hrtimeout().

Moving kprobes initialization to early_initcall() [1] means that there
can be a call to synchronize_rcu_tasks() before the current spawning of
ksoftirqd.  Because synchronize_rcu_tasks() needs timers to work, it needs
softirq to work.  I know two straightforward ways to make that happen:

1.  Spawn ksoftirqd earlier.

2.  Suppress attempts to awaken ksoftirqd before it exists,
forcing all ksoftirq execution on the back of interrupts.

Uladzislau and I each produced patches for #1, and I produced a patch
for #2.

The only other option I know of is to push the call to init_kprobes()
later in the boot sequence, perhaps to its original subsys_initcall(),
or maybe only as late as core_initcall().  I added Masami and Steve on
CC for their thoughts on this.

Is there some other proper fix that I am missing?

Thanx, Paul

[1] 36dadef23fcc ("kprobes: Init kprobes in early_initcall")


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-15 Thread Sebastian Andrzej Siewior
On 2021-02-13 08:45:54 [-0800], Paul E. McKenney wrote:
> Glad you like it!  But let's see which (if any) of these patches solves
> the problem for Sebastian.

Looking at that, is there any reason for doing this that can not be
solved by moving the self-test a little later? Maybe once we reached at
least SYSTEM_SCHEDULING?
This happens now even before lockdep is up or the console is registered.
So if something bad happens, you end up with a blank terminal.

There is nothing else that early in the boot process that requires
working softirq. The only exception to this is wait_task_inactive()
which is used while starting a new thread (including the ksoftirqd)
which is why it was moved to schedule_hrtimeout().

>   Thanx, Paul

Sebastian


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-13 Thread Uladzislau Rezki
On Sat, Feb 13, 2021 at 08:45:54AM -0800, Paul E. McKenney wrote:
> On Sat, Feb 13, 2021 at 12:30:30PM +0100, Uladzislau Rezki wrote:
> > On Fri, Feb 12, 2021 at 04:43:28PM -0800, Paul E. McKenney wrote:
> > > On Fri, Feb 12, 2021 at 04:37:09PM -0800, Paul E. McKenney wrote:
> > > > On Fri, Feb 12, 2021 at 03:48:51PM -0800, Paul E. McKenney wrote:
> > > > > On Fri, Feb 12, 2021 at 10:12:07PM +0100, Uladzislau Rezki wrote:
> > > > > > On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior 
> > > > > > wrote:
> > > > > > > On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > > > > > > > Add self tests for checking of RCU-tasks API functionality.
> > > > > > > > It covers:
> > > > > > > > - wait API functions;
> > > > > > > > - invoking/completion call_rcu_tasks*().
> > > > > > > > 
> > > > > > > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is 
> > > > > > > > set.
> > > > > > > 
> > > > > > > I just bisected to this commit. By booting with `threadirqs' I 
> > > > > > > end up
> > > > > > > with:
> > > > > > > [0.176533] Running RCU-tasks wait API self tests
> > > > > > > 
> > > > > > > No stall warning or so.
> > > > > > > It boots again with:
> > > > > > > 
> > > > > > > diff --git a/init/main.c b/init/main.c
> > > > > > > --- a/init/main.c
> > > > > > > +++ b/init/main.c
> > > > > > > @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
> > > > > > >   fput(file);
> > > > > > >  }
> > > > > > >  
> > > > > > > +void rcu_tasks_initiate_self_tests(void);
> > > > > > >  static noinline void __init kernel_init_freeable(void)
> > > > > > >  {
> > > > > > >   /*
> > > > > > > @@ -1514,6 +1515,7 @@ static noinline void __init 
> > > > > > > kernel_init_freeable(void)
> > > > > > >  
> > > > > > >   rcu_init_tasks_generic();
> > > > > > >   do_pre_smp_initcalls();
> > > > > > > + rcu_tasks_initiate_self_tests();
> > > > > > >   lockup_detector_init();
> > > > > > >  
> > > > > > >   smp_init();
> > > > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > > > --- a/kernel/rcu/tasks.h
> > > > > > > +++ b/kernel/rcu/tasks.h
> > > > > > > @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct 
> > > > > > > rcu_head *rhp)
> > > > > > >   rttd->notrun = true;
> > > > > > >  }
> > > > > > >  
> > > > > > > -static void rcu_tasks_initiate_self_tests(void)
> > > > > > > +void rcu_tasks_initiate_self_tests(void)
> > > > > > >  {
> > > > > > >   pr_info("Running RCU-tasks wait API self tests\n");
> > > > > > >  #ifdef CONFIG_TASKS_RCU
> > > > > > > @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
> > > > > > >  #endif
> > > > > > >  
> > > > > > >   // Run the self-tests.
> > > > > > > - rcu_tasks_initiate_self_tests();
> > > > > > >  }
> > > > > > >  
> > > > > > >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > > > > > > 
> > > > > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > > > 
> > > > > Apologies for the hassle!  My testing clearly missed this combination
> > > > > of CONFIG_PROVE_RCU=y and threadirqs=1.  :-(
> > > > > 
> > > > > But at least I can easily reproduce this hang as follows:
> > > > > 
> > > > > tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 
> > > > > --configs "TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y 
> > > > > CONFIG_PROVE_LOCKING=y" --bootargs "threadirqs=1" --trust-make
> > > > > 
> > > > > Sadly, I cannot take your patch because that simply papers over the
> > > > > fact that early boot use of synchronize_rcu_tasks() is broken in this
> > > > > particular configuration, which will likely eventually bite others now
> > > > > that init_kprobes() has been moved earlier in boot:
> > > > > 
> > > > > 1b04fa990026 ("rcu-tasks: Move RCU-tasks initialization to before 
> > > > > early_initcall()")
> > > > > Link: 
> > > > > https://lore.kernel.org/rcu/87eekfh80a@dja-thinkpad.axtens.net/
> > > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > > 
> > > > > > > Sebastian
> > > > > > >
> > > > > > We should be able to use call_rcu_tasks() in the *initcall() 
> > > > > > callbacks.
> > > > > > The problem is that, ksoftirqd threads are not spawned by the time 
> > > > > > when
> > > > > > an rcu_init_tasks_generic() is invoked:
> > > > > > 
> > > > > > diff --git a/init/main.c b/init/main.c
> > > > > > index c68d784376ca..e6106bb12b2d 100644
> > > > > > --- a/init/main.c
> > > > > > +++ b/init/main.c
> > > > > > @@ -954,7 +954,6 @@ asmlinkage __visible void __init 
> > > > > > __no_sanitize_address start_kernel(void)
> > > > > > rcu_init_nohz();
> > > > > > init_timers();
> > > > > > hrtimers_init();
> > > > > > -   softirq_init();
> > > > > > timekeeping_init();
> > > > > >  
> > > > > > /*
> > > > > > @@ -1512,6 +1511,7 @@ static noinline void __init 
> > > > > > kernel_init_freeable(void)
> > > > > >  
> > > > > > init_mm_internals();
> > > > > >  
> > > > > > +   softirq_init();
> > > > > > rcu_init_tasks_generic();
> > 

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-13 Thread Paul E. McKenney
On Sat, Feb 13, 2021 at 12:30:30PM +0100, Uladzislau Rezki wrote:
> On Fri, Feb 12, 2021 at 04:43:28PM -0800, Paul E. McKenney wrote:
> > On Fri, Feb 12, 2021 at 04:37:09PM -0800, Paul E. McKenney wrote:
> > > On Fri, Feb 12, 2021 at 03:48:51PM -0800, Paul E. McKenney wrote:
> > > > On Fri, Feb 12, 2021 at 10:12:07PM +0100, Uladzislau Rezki wrote:
> > > > > On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior 
> > > > > wrote:
> > > > > > On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > > > > > > Add self tests for checking of RCU-tasks API functionality.
> > > > > > > It covers:
> > > > > > > - wait API functions;
> > > > > > > - invoking/completion call_rcu_tasks*().
> > > > > > > 
> > > > > > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > > > > > 
> > > > > > I just bisected to this commit. By booting with `threadirqs' I end 
> > > > > > up
> > > > > > with:
> > > > > > [0.176533] Running RCU-tasks wait API self tests
> > > > > > 
> > > > > > No stall warning or so.
> > > > > > It boots again with:
> > > > > > 
> > > > > > diff --git a/init/main.c b/init/main.c
> > > > > > --- a/init/main.c
> > > > > > +++ b/init/main.c
> > > > > > @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
> > > > > > fput(file);
> > > > > >  }
> > > > > >  
> > > > > > +void rcu_tasks_initiate_self_tests(void);
> > > > > >  static noinline void __init kernel_init_freeable(void)
> > > > > >  {
> > > > > > /*
> > > > > > @@ -1514,6 +1515,7 @@ static noinline void __init 
> > > > > > kernel_init_freeable(void)
> > > > > >  
> > > > > > rcu_init_tasks_generic();
> > > > > > do_pre_smp_initcalls();
> > > > > > +   rcu_tasks_initiate_self_tests();
> > > > > > lockup_detector_init();
> > > > > >  
> > > > > > smp_init();
> > > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > > --- a/kernel/rcu/tasks.h
> > > > > > +++ b/kernel/rcu/tasks.h
> > > > > > @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct 
> > > > > > rcu_head *rhp)
> > > > > > rttd->notrun = true;
> > > > > >  }
> > > > > >  
> > > > > > -static void rcu_tasks_initiate_self_tests(void)
> > > > > > +void rcu_tasks_initiate_self_tests(void)
> > > > > >  {
> > > > > > pr_info("Running RCU-tasks wait API self tests\n");
> > > > > >  #ifdef CONFIG_TASKS_RCU
> > > > > > @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
> > > > > >  #endif
> > > > > >  
> > > > > > // Run the self-tests.
> > > > > > -   rcu_tasks_initiate_self_tests();
> > > > > >  }
> > > > > >  
> > > > > >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > > > > > 
> > > > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > > 
> > > > Apologies for the hassle!  My testing clearly missed this combination
> > > > of CONFIG_PROVE_RCU=y and threadirqs=1.  :-(
> > > > 
> > > > But at least I can easily reproduce this hang as follows:
> > > > 
> > > > tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 
> > > > --configs "TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y 
> > > > CONFIG_PROVE_LOCKING=y" --bootargs "threadirqs=1" --trust-make
> > > > 
> > > > Sadly, I cannot take your patch because that simply papers over the
> > > > fact that early boot use of synchronize_rcu_tasks() is broken in this
> > > > particular configuration, which will likely eventually bite others now
> > > > that init_kprobes() has been moved earlier in boot:
> > > > 
> > > > 1b04fa990026 ("rcu-tasks: Move RCU-tasks initialization to before 
> > > > early_initcall()")
> > > > Link: 
> > > > https://lore.kernel.org/rcu/87eekfh80a@dja-thinkpad.axtens.net/
> > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > 
> > > > > > Sebastian
> > > > > >
> > > > > We should be able to use call_rcu_tasks() in the *initcall() 
> > > > > callbacks.
> > > > > The problem is that, ksoftirqd threads are not spawned by the time 
> > > > > when
> > > > > an rcu_init_tasks_generic() is invoked:
> > > > > 
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > index c68d784376ca..e6106bb12b2d 100644
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -954,7 +954,6 @@ asmlinkage __visible void __init 
> > > > > __no_sanitize_address start_kernel(void)
> > > > >   rcu_init_nohz();
> > > > >   init_timers();
> > > > >   hrtimers_init();
> > > > > - softirq_init();
> > > > >   timekeeping_init();
> > > > >  
> > > > >   /*
> > > > > @@ -1512,6 +1511,7 @@ static noinline void __init 
> > > > > kernel_init_freeable(void)
> > > > >  
> > > > >   init_mm_internals();
> > > > >  
> > > > > + softirq_init();
> > > > >   rcu_init_tasks_generic();
> > > > >   do_pre_smp_initcalls();
> > > > >   lockup_detector_init();
> > > > > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > > > > index 9d71046ea247..cafa55c496d0 100644
> > > > > --- a/kernel/softirq.c
> > > > > +++ b/kernel/softirq.c
> > > > > @@ -630,6 +6

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-13 Thread Uladzislau Rezki
On Fri, Feb 12, 2021 at 04:43:28PM -0800, Paul E. McKenney wrote:
> On Fri, Feb 12, 2021 at 04:37:09PM -0800, Paul E. McKenney wrote:
> > On Fri, Feb 12, 2021 at 03:48:51PM -0800, Paul E. McKenney wrote:
> > > On Fri, Feb 12, 2021 at 10:12:07PM +0100, Uladzislau Rezki wrote:
> > > > On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior 
> > > > wrote:
> > > > > On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > > > > > Add self tests for checking of RCU-tasks API functionality.
> > > > > > It covers:
> > > > > > - wait API functions;
> > > > > > - invoking/completion call_rcu_tasks*().
> > > > > > 
> > > > > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > > > > 
> > > > > I just bisected to this commit. By booting with `threadirqs' I end up
> > > > > with:
> > > > > [0.176533] Running RCU-tasks wait API self tests
> > > > > 
> > > > > No stall warning or so.
> > > > > It boots again with:
> > > > > 
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
> > > > >   fput(file);
> > > > >  }
> > > > >  
> > > > > +void rcu_tasks_initiate_self_tests(void);
> > > > >  static noinline void __init kernel_init_freeable(void)
> > > > >  {
> > > > >   /*
> > > > > @@ -1514,6 +1515,7 @@ static noinline void __init 
> > > > > kernel_init_freeable(void)
> > > > >  
> > > > >   rcu_init_tasks_generic();
> > > > >   do_pre_smp_initcalls();
> > > > > + rcu_tasks_initiate_self_tests();
> > > > >   lockup_detector_init();
> > > > >  
> > > > >   smp_init();
> > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > --- a/kernel/rcu/tasks.h
> > > > > +++ b/kernel/rcu/tasks.h
> > > > > @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct 
> > > > > rcu_head *rhp)
> > > > >   rttd->notrun = true;
> > > > >  }
> > > > >  
> > > > > -static void rcu_tasks_initiate_self_tests(void)
> > > > > +void rcu_tasks_initiate_self_tests(void)
> > > > >  {
> > > > >   pr_info("Running RCU-tasks wait API self tests\n");
> > > > >  #ifdef CONFIG_TASKS_RCU
> > > > > @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
> > > > >  #endif
> > > > >  
> > > > >   // Run the self-tests.
> > > > > - rcu_tasks_initiate_self_tests();
> > > > >  }
> > > > >  
> > > > >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > > > > 
> > > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > 
> > > Apologies for the hassle!  My testing clearly missed this combination
> > > of CONFIG_PROVE_RCU=y and threadirqs=1.  :-(
> > > 
> > > But at least I can easily reproduce this hang as follows:
> > > 
> > > tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 
> > > --configs "TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y 
> > > CONFIG_PROVE_LOCKING=y" --bootargs "threadirqs=1" --trust-make
> > > 
> > > Sadly, I cannot take your patch because that simply papers over the
> > > fact that early boot use of synchronize_rcu_tasks() is broken in this
> > > particular configuration, which will likely eventually bite others now
> > > that init_kprobes() has been moved earlier in boot:
> > > 
> > > 1b04fa990026 ("rcu-tasks: Move RCU-tasks initialization to before 
> > > early_initcall()")
> > > Link: https://lore.kernel.org/rcu/87eekfh80a@dja-thinkpad.axtens.net/
> > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > 
> > > > > Sebastian
> > > > >
> > > > We should be able to use call_rcu_tasks() in the *initcall() callbacks.
> > > > The problem is that, ksoftirqd threads are not spawned by the time when
> > > > an rcu_init_tasks_generic() is invoked:
> > > > 
> > > > diff --git a/init/main.c b/init/main.c
> > > > index c68d784376ca..e6106bb12b2d 100644
> > > > --- a/init/main.c
> > > > +++ b/init/main.c
> > > > @@ -954,7 +954,6 @@ asmlinkage __visible void __init 
> > > > __no_sanitize_address start_kernel(void)
> > > > rcu_init_nohz();
> > > > init_timers();
> > > > hrtimers_init();
> > > > -   softirq_init();
> > > > timekeeping_init();
> > > >  
> > > > /*
> > > > @@ -1512,6 +1511,7 @@ static noinline void __init 
> > > > kernel_init_freeable(void)
> > > >  
> > > > init_mm_internals();
> > > >  
> > > > +   softirq_init();
> > > > rcu_init_tasks_generic();
> > > > do_pre_smp_initcalls();
> > > > lockup_detector_init();
> > > > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > > > index 9d71046ea247..cafa55c496d0 100644
> > > > --- a/kernel/softirq.c
> > > > +++ b/kernel/softirq.c
> > > > @@ -630,6 +630,7 @@ void __init softirq_init(void)
> > > > &per_cpu(tasklet_hi_vec, cpu).head;
> > > > }
> > > >  
> > > > +   spawn_ksoftirqd();
> > > 
> > > We need a forward reference to allow this to build, but with that added,
> > > my test case passes.  Good show!
> > >

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-12 Thread Paul E. McKenney
On Fri, Feb 12, 2021 at 04:37:09PM -0800, Paul E. McKenney wrote:
> On Fri, Feb 12, 2021 at 03:48:51PM -0800, Paul E. McKenney wrote:
> > On Fri, Feb 12, 2021 at 10:12:07PM +0100, Uladzislau Rezki wrote:
> > > On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior wrote:
> > > > On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > > > > Add self tests for checking of RCU-tasks API functionality.
> > > > > It covers:
> > > > > - wait API functions;
> > > > > - invoking/completion call_rcu_tasks*().
> > > > > 
> > > > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > > > 
> > > > I just bisected to this commit. By booting with `threadirqs' I end up
> > > > with:
> > > > [0.176533] Running RCU-tasks wait API self tests
> > > > 
> > > > No stall warning or so.
> > > > It boots again with:
> > > > 
> > > > diff --git a/init/main.c b/init/main.c
> > > > --- a/init/main.c
> > > > +++ b/init/main.c
> > > > @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
> > > > fput(file);
> > > >  }
> > > >  
> > > > +void rcu_tasks_initiate_self_tests(void);
> > > >  static noinline void __init kernel_init_freeable(void)
> > > >  {
> > > > /*
> > > > @@ -1514,6 +1515,7 @@ static noinline void __init 
> > > > kernel_init_freeable(void)
> > > >  
> > > > rcu_init_tasks_generic();
> > > > do_pre_smp_initcalls();
> > > > +   rcu_tasks_initiate_self_tests();
> > > > lockup_detector_init();
> > > >  
> > > > smp_init();
> > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > --- a/kernel/rcu/tasks.h
> > > > +++ b/kernel/rcu/tasks.h
> > > > @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct 
> > > > rcu_head *rhp)
> > > > rttd->notrun = true;
> > > >  }
> > > >  
> > > > -static void rcu_tasks_initiate_self_tests(void)
> > > > +void rcu_tasks_initiate_self_tests(void)
> > > >  {
> > > > pr_info("Running RCU-tasks wait API self tests\n");
> > > >  #ifdef CONFIG_TASKS_RCU
> > > > @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
> > > >  #endif
> > > >  
> > > > // Run the self-tests.
> > > > -   rcu_tasks_initiate_self_tests();
> > > >  }
> > > >  
> > > >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > > > 
> > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > 
> > Apologies for the hassle!  My testing clearly missed this combination
> > of CONFIG_PROVE_RCU=y and threadirqs=1.  :-(
> > 
> > But at least I can easily reproduce this hang as follows:
> > 
> > tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 
> > --configs "TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y 
> > CONFIG_PROVE_LOCKING=y" --bootargs "threadirqs=1" --trust-make
> > 
> > Sadly, I cannot take your patch because that simply papers over the
> > fact that early boot use of synchronize_rcu_tasks() is broken in this
> > particular configuration, which will likely eventually bite others now
> > that init_kprobes() has been moved earlier in boot:
> > 
> > 1b04fa990026 ("rcu-tasks: Move RCU-tasks initialization to before 
> > early_initcall()")
> > Link: https://lore.kernel.org/rcu/87eekfh80a@dja-thinkpad.axtens.net/
> > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > 
> > > > Sebastian
> > > >
> > > We should be able to use call_rcu_tasks() in the *initcall() callbacks.
> > > The problem is that, ksoftirqd threads are not spawned by the time when
> > > an rcu_init_tasks_generic() is invoked:
> > > 
> > > diff --git a/init/main.c b/init/main.c
> > > index c68d784376ca..e6106bb12b2d 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -954,7 +954,6 @@ asmlinkage __visible void __init 
> > > __no_sanitize_address start_kernel(void)
> > >   rcu_init_nohz();
> > >   init_timers();
> > >   hrtimers_init();
> > > - softirq_init();
> > >   timekeeping_init();
> > >  
> > >   /*
> > > @@ -1512,6 +1511,7 @@ static noinline void __init 
> > > kernel_init_freeable(void)
> > >  
> > >   init_mm_internals();
> > >  
> > > + softirq_init();
> > >   rcu_init_tasks_generic();
> > >   do_pre_smp_initcalls();
> > >   lockup_detector_init();
> > > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > > index 9d71046ea247..cafa55c496d0 100644
> > > --- a/kernel/softirq.c
> > > +++ b/kernel/softirq.c
> > > @@ -630,6 +630,7 @@ void __init softirq_init(void)
> > >   &per_cpu(tasklet_hi_vec, cpu).head;
> > >   }
> > >  
> > > + spawn_ksoftirqd();
> > 
> > We need a forward reference to allow this to build, but with that added,
> > my test case passes.  Good show!
> > 
> > >   open_softirq(TASKLET_SOFTIRQ, tasklet_action);
> > >   open_softirq(HI_SOFTIRQ, tasklet_hi_action);
> > >  }
> > > @@ -732,7 +733,6 @@ static __init int spawn_ksoftirqd(void)
> > >  
> > >   return 0;
> > >  }
> > > -early_initcall(spawn_ksoftirqd);
> > >  
> > >  /*
> > >   * [ These __weak aliases are kept in a separate compilation unit, so 
> > > that
> > > 
> > > Any

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-12 Thread Paul E. McKenney
On Fri, Feb 12, 2021 at 03:48:51PM -0800, Paul E. McKenney wrote:
> On Fri, Feb 12, 2021 at 10:12:07PM +0100, Uladzislau Rezki wrote:
> > On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > > > Add self tests for checking of RCU-tasks API functionality.
> > > > It covers:
> > > > - wait API functions;
> > > > - invoking/completion call_rcu_tasks*().
> > > > 
> > > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > > 
> > > I just bisected to this commit. By booting with `threadirqs' I end up
> > > with:
> > > [0.176533] Running RCU-tasks wait API self tests
> > > 
> > > No stall warning or so.
> > > It boots again with:
> > > 
> > > diff --git a/init/main.c b/init/main.c
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
> > >   fput(file);
> > >  }
> > >  
> > > +void rcu_tasks_initiate_self_tests(void);
> > >  static noinline void __init kernel_init_freeable(void)
> > >  {
> > >   /*
> > > @@ -1514,6 +1515,7 @@ static noinline void __init 
> > > kernel_init_freeable(void)
> > >  
> > >   rcu_init_tasks_generic();
> > >   do_pre_smp_initcalls();
> > > + rcu_tasks_initiate_self_tests();
> > >   lockup_detector_init();
> > >  
> > >   smp_init();
> > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > --- a/kernel/rcu/tasks.h
> > > +++ b/kernel/rcu/tasks.h
> > > @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct rcu_head 
> > > *rhp)
> > >   rttd->notrun = true;
> > >  }
> > >  
> > > -static void rcu_tasks_initiate_self_tests(void)
> > > +void rcu_tasks_initiate_self_tests(void)
> > >  {
> > >   pr_info("Running RCU-tasks wait API self tests\n");
> > >  #ifdef CONFIG_TASKS_RCU
> > > @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
> > >  #endif
> > >  
> > >   // Run the self-tests.
> > > - rcu_tasks_initiate_self_tests();
> > >  }
> > >  
> > >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > > 
> > > > Signed-off-by: Uladzislau Rezki (Sony) 
> 
> Apologies for the hassle!  My testing clearly missed this combination
> of CONFIG_PROVE_RCU=y and threadirqs=1.  :-(
> 
> But at least I can easily reproduce this hang as follows:
> 
> tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 
> --configs "TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y 
> CONFIG_PROVE_LOCKING=y" --bootargs "threadirqs=1" --trust-make
> 
> Sadly, I cannot take your patch because that simply papers over the
> fact that early boot use of synchronize_rcu_tasks() is broken in this
> particular configuration, which will likely eventually bite others now
> that init_kprobes() has been moved earlier in boot:
> 
> 1b04fa990026 ("rcu-tasks: Move RCU-tasks initialization to before 
> early_initcall()")
> Link: https://lore.kernel.org/rcu/87eekfh80a@dja-thinkpad.axtens.net/
> Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> 
> > > Sebastian
> > >
> > We should be able to use call_rcu_tasks() in the *initcall() callbacks.
> > The problem is that, ksoftirqd threads are not spawned by the time when
> > an rcu_init_tasks_generic() is invoked:
> > 
> > diff --git a/init/main.c b/init/main.c
> > index c68d784376ca..e6106bb12b2d 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -954,7 +954,6 @@ asmlinkage __visible void __init __no_sanitize_address 
> > start_kernel(void)
> > rcu_init_nohz();
> > init_timers();
> > hrtimers_init();
> > -   softirq_init();
> > timekeeping_init();
> >  
> > /*
> > @@ -1512,6 +1511,7 @@ static noinline void __init kernel_init_freeable(void)
> >  
> > init_mm_internals();
> >  
> > +   softirq_init();
> > rcu_init_tasks_generic();
> > do_pre_smp_initcalls();
> > lockup_detector_init();
> > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > index 9d71046ea247..cafa55c496d0 100644
> > --- a/kernel/softirq.c
> > +++ b/kernel/softirq.c
> > @@ -630,6 +630,7 @@ void __init softirq_init(void)
> > &per_cpu(tasklet_hi_vec, cpu).head;
> > }
> >  
> > +   spawn_ksoftirqd();
> 
> We need a forward reference to allow this to build, but with that added,
> my test case passes.  Good show!
> 
> > open_softirq(TASKLET_SOFTIRQ, tasklet_action);
> > open_softirq(HI_SOFTIRQ, tasklet_hi_action);
> >  }
> > @@ -732,7 +733,6 @@ static __init int spawn_ksoftirqd(void)
> >  
> > return 0;
> >  }
> > -early_initcall(spawn_ksoftirqd);
> >  
> >  /*
> >   * [ These __weak aliases are kept in a separate compilation unit, so that
> > 
> > Any thoughts?
> 
> One likely problem is that there are almost certainly parts of the kernel
> that need softirq_init() to stay roughly where it is.  So, is it possible
> to leave softirq_init() where it is, and to arrange for spawn_ksoftirqd()
> to be invoked just before rcu_init_tasks_generic() is called?

This still seems worth trying (and doing so is next on my list), but just

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-12 Thread Paul E. McKenney
On Fri, Feb 12, 2021 at 10:12:07PM +0100, Uladzislau Rezki wrote:
> On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > > Add self tests for checking of RCU-tasks API functionality.
> > > It covers:
> > > - wait API functions;
> > > - invoking/completion call_rcu_tasks*().
> > > 
> > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > 
> > I just bisected to this commit. By booting with `threadirqs' I end up
> > with:
> > [0.176533] Running RCU-tasks wait API self tests
> > 
> > No stall warning or so.
> > It boots again with:
> > 
> > diff --git a/init/main.c b/init/main.c
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
> > fput(file);
> >  }
> >  
> > +void rcu_tasks_initiate_self_tests(void);
> >  static noinline void __init kernel_init_freeable(void)
> >  {
> > /*
> > @@ -1514,6 +1515,7 @@ static noinline void __init kernel_init_freeable(void)
> >  
> > rcu_init_tasks_generic();
> > do_pre_smp_initcalls();
> > +   rcu_tasks_initiate_self_tests();
> > lockup_detector_init();
> >  
> > smp_init();
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct rcu_head 
> > *rhp)
> > rttd->notrun = true;
> >  }
> >  
> > -static void rcu_tasks_initiate_self_tests(void)
> > +void rcu_tasks_initiate_self_tests(void)
> >  {
> > pr_info("Running RCU-tasks wait API self tests\n");
> >  #ifdef CONFIG_TASKS_RCU
> > @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
> >  #endif
> >  
> > // Run the self-tests.
> > -   rcu_tasks_initiate_self_tests();
> >  }
> >  
> >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > 
> > > Signed-off-by: Uladzislau Rezki (Sony) 

Apologies for the hassle!  My testing clearly missed this combination
of CONFIG_PROVE_RCU=y and threadirqs=1.  :-(

But at least I can easily reproduce this hang as follows:

tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 --configs 
"TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y" 
--bootargs "threadirqs=1" --trust-make

Sadly, I cannot take your patch because that simply papers over the
fact that early boot use of synchronize_rcu_tasks() is broken in this
particular configuration, which will likely eventually bite others now
that init_kprobes() has been moved earlier in boot:

1b04fa990026 ("rcu-tasks: Move RCU-tasks initialization to before 
early_initcall()")
Link: https://lore.kernel.org/rcu/87eekfh80a@dja-thinkpad.axtens.net/
Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")

> > Sebastian
> >
> We should be able to use call_rcu_tasks() in the *initcall() callbacks.
> The problem is that, ksoftirqd threads are not spawned by the time when
> an rcu_init_tasks_generic() is invoked:
> 
> diff --git a/init/main.c b/init/main.c
> index c68d784376ca..e6106bb12b2d 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -954,7 +954,6 @@ asmlinkage __visible void __init __no_sanitize_address 
> start_kernel(void)
>   rcu_init_nohz();
>   init_timers();
>   hrtimers_init();
> - softirq_init();
>   timekeeping_init();
>  
>   /*
> @@ -1512,6 +1511,7 @@ static noinline void __init kernel_init_freeable(void)
>  
>   init_mm_internals();
>  
> + softirq_init();
>   rcu_init_tasks_generic();
>   do_pre_smp_initcalls();
>   lockup_detector_init();
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 9d71046ea247..cafa55c496d0 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -630,6 +630,7 @@ void __init softirq_init(void)
>   &per_cpu(tasklet_hi_vec, cpu).head;
>   }
>  
> + spawn_ksoftirqd();

We need a forward reference to allow this to build, but with that added,
my test case passes.  Good show!

>   open_softirq(TASKLET_SOFTIRQ, tasklet_action);
>   open_softirq(HI_SOFTIRQ, tasklet_hi_action);
>  }
> @@ -732,7 +733,6 @@ static __init int spawn_ksoftirqd(void)
>  
>   return 0;
>  }
> -early_initcall(spawn_ksoftirqd);
>  
>  /*
>   * [ These __weak aliases are kept in a separate compilation unit, so that
> 
> Any thoughts?

One likely problem is that there are almost certainly parts of the kernel
that need softirq_init() to stay roughly where it is.  So, is it possible
to leave softirq_init() where it is, and to arrange for spawn_ksoftirqd()
to be invoked just before rcu_init_tasks_generic() is called?

For my part, I will look into what is required to make Tasks RCU do
without softirq during boot, for example, by looking carefully at where
in boot RCU grace periods are unconditionally expedited.  Just in case
adjusting softirq has unforeseen side effects.

Thanx, Paul


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-12 Thread Uladzislau Rezki
On Fri, Feb 12, 2021 at 08:20:59PM +0100, Sebastian Andrzej Siewior wrote:
> On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> > Add self tests for checking of RCU-tasks API functionality.
> > It covers:
> > - wait API functions;
> > - invoking/completion call_rcu_tasks*().
> > 
> > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> 
> I just bisected to this commit. By booting with `threadirqs' I end up
> with:
> [0.176533] Running RCU-tasks wait API self tests
> 
> No stall warning or so.
> It boots again with:
> 
> diff --git a/init/main.c b/init/main.c
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
>   fput(file);
>  }
>  
> +void rcu_tasks_initiate_self_tests(void);
>  static noinline void __init kernel_init_freeable(void)
>  {
>   /*
> @@ -1514,6 +1515,7 @@ static noinline void __init kernel_init_freeable(void)
>  
>   rcu_init_tasks_generic();
>   do_pre_smp_initcalls();
> + rcu_tasks_initiate_self_tests();
>   lockup_detector_init();
>  
>   smp_init();
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct rcu_head 
> *rhp)
>   rttd->notrun = true;
>  }
>  
> -static void rcu_tasks_initiate_self_tests(void)
> +void rcu_tasks_initiate_self_tests(void)
>  {
>   pr_info("Running RCU-tasks wait API self tests\n");
>  #ifdef CONFIG_TASKS_RCU
> @@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
>  #endif
>  
>   // Run the self-tests.
> - rcu_tasks_initiate_self_tests();
>  }
>  
>  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> 
> > Signed-off-by: Uladzislau Rezki (Sony) 
> 
> Sebastian
>
We should be able to use call_rcu_tasks() in the *initcall() callbacks.
The problem is that, ksoftirqd threads are not spawned by the time when
an rcu_init_tasks_generic() is invoked:

diff --git a/init/main.c b/init/main.c
index c68d784376ca..e6106bb12b2d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -954,7 +954,6 @@ asmlinkage __visible void __init __no_sanitize_address 
start_kernel(void)
rcu_init_nohz();
init_timers();
hrtimers_init();
-   softirq_init();
timekeeping_init();
 
/*
@@ -1512,6 +1511,7 @@ static noinline void __init kernel_init_freeable(void)
 
init_mm_internals();
 
+   softirq_init();
rcu_init_tasks_generic();
do_pre_smp_initcalls();
lockup_detector_init();
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 9d71046ea247..cafa55c496d0 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -630,6 +630,7 @@ void __init softirq_init(void)
&per_cpu(tasklet_hi_vec, cpu).head;
}
 
+   spawn_ksoftirqd();
open_softirq(TASKLET_SOFTIRQ, tasklet_action);
open_softirq(HI_SOFTIRQ, tasklet_hi_action);
 }
@@ -732,7 +733,6 @@ static __init int spawn_ksoftirqd(void)
 
return 0;
 }
-early_initcall(spawn_ksoftirqd);
 
 /*
  * [ These __weak aliases are kept in a separate compilation unit, so that

Any thoughts?

--
Vlad Rezki


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2021-02-12 Thread Sebastian Andrzej Siewior
On 2020-12-09 21:27:32 [+0100], Uladzislau Rezki (Sony) wrote:
> Add self tests for checking of RCU-tasks API functionality.
> It covers:
> - wait API functions;
> - invoking/completion call_rcu_tasks*().
> 
> Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.

I just bisected to this commit. By booting with `threadirqs' I end up
with:
[0.176533] Running RCU-tasks wait API self tests

No stall warning or so.
It boots again with:

diff --git a/init/main.c b/init/main.c
--- a/init/main.c
+++ b/init/main.c
@@ -1489,6 +1489,7 @@ void __init console_on_rootfs(void)
fput(file);
 }
 
+void rcu_tasks_initiate_self_tests(void);
 static noinline void __init kernel_init_freeable(void)
 {
/*
@@ -1514,6 +1515,7 @@ static noinline void __init kernel_init_freeable(void)
 
rcu_init_tasks_generic();
do_pre_smp_initcalls();
+   rcu_tasks_initiate_self_tests();
lockup_detector_init();
 
smp_init();
diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1266,7 +1266,7 @@ static void test_rcu_tasks_callback(struct rcu_head *rhp)
rttd->notrun = true;
 }
 
-static void rcu_tasks_initiate_self_tests(void)
+void rcu_tasks_initiate_self_tests(void)
 {
pr_info("Running RCU-tasks wait API self tests\n");
 #ifdef CONFIG_TASKS_RCU
@@ -1322,7 +1322,6 @@ void __init rcu_init_tasks_generic(void)
 #endif
 
// Run the self-tests.
-   rcu_tasks_initiate_self_tests();
 }
 
 #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */

> Signed-off-by: Uladzislau Rezki (Sony) 

Sebastian


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Uladzislau Rezki
On Mon, Dec 21, 2020 at 12:45:13PM -0800, Paul E. McKenney wrote:
> On Mon, Dec 21, 2020 at 08:48:48PM +0100, Uladzislau Rezki wrote:
> > On Mon, Dec 21, 2020 at 11:29:06AM -0800, Paul E. McKenney wrote:
> > > On Mon, Dec 21, 2020 at 07:45:39PM +0100, Uladzislau Rezki wrote:
> > > > On Mon, Dec 21, 2020 at 09:18:05AM -0800, Paul E. McKenney wrote:
> > > > > On Mon, Dec 21, 2020 at 04:38:09PM +0100, Uladzislau Rezki wrote:
> > > > > > On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> > > > > > > On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > > > > 
> > > > > [ . . . ]
> > > > > 
> > > > > > > > 2.20.1
> > > > > > > 
> > > > > > > Again, much improved!
> > > > > > > 
> > > > > > See below the v3 version. I hope i fixed all comments :)
> > > > > > 
> > > > > > >From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 
> > > > > > >2001
> > > > > > From: "Uladzislau Rezki (Sony)" 
> > > > > > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > > > > > Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
> > > > > > 
> > > > > > This commit adds self tests for early-boot use of RCU-tasks grace 
> > > > > > periods.
> > > > > > It tests all three variants (Rude, Tasks, and Tasks Trace) and 
> > > > > > covers
> > > > > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous 
> > > > > > (e.g.,
> > > > > > call_rcu_tasks()) grace-period APIs.
> > > > > > 
> > > > > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > > > > 
> > > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > > > 
> > > > > Much better!
> > > > > 
> > > > > I pulled this in, but made one small additional change.  Please let me
> > > > > know if this is problematic.
> > > > > 
> > > > >   Thanx, Paul
> > > > > 
> > > > > 
> > > > > 
> > > > > commit 93372198b5c9efdfd288aa3b3ee41c1f90866886
> > > > > Author: Uladzislau Rezki (Sony) 
> > > > > Date:   Wed Dec 9 21:27:32 2020 +0100
> > > > > 
> > > > > rcu-tasks: Add RCU-tasks self tests
> > > > > 
> > > > > This commit adds self tests for early-boot use of RCU-tasks grace 
> > > > > periods.
> > > > > It tests all three variants (Rude, Tasks, and Tasks Trace) and 
> > > > > covers
> > > > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous 
> > > > > (e.g.,
> > > > > call_rcu_tasks()) grace-period APIs.
> > > > > 
> > > > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > > > 
> > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > > > Signed-off-by: Paul E. McKenney 
> > > > > 
> > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > index 3660755..35a2cd5 100644
> > > > > --- a/kernel/rcu/tasks.h
> > > > > +++ b/kernel/rcu/tasks.h
> > > > > @@ -1224,6 +1224,40 @@ void show_rcu_tasks_gp_kthreads(void)
> > > > >  }
> > > > >  #endif /* #ifndef CONFIG_TINY_RCU */
> > > > >  
> > > > > +struct rcu_tasks_test_desc {
> > > > > + struct rcu_head rh;
> > > > > + const char *name;
> > > > > + bool notrun;
> > > > > +};
> > > > > +
> > > > > +static struct rcu_tasks_test_desc tests[] = {
> > > > > + {
> > > > > + .name = "call_rcu_tasks()",
> > > > > + /* If not defined, the test is skipped. */
> > > > > + .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
> > > > > + },
> > > > > + {
> > > > > + .name = "call_rcu_tasks_rude()",
> > > > > + /* If not defined, the test is skipped. */
> > > > > + .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
> > > > > + },
> > > > > + {
> > > > > + .name = "call_rcu_tasks_trace()",
> > > > > + /* If not defined, the test is skipped. */
> > > > > + .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
> > > > > + }
> > > > > +};
> > > > > +
> > > > > +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> > > > > +{
> > > > > + struct rcu_tasks_test_desc *rttd =
> > > > > + container_of(rhp, struct rcu_tasks_test_desc, rh);
> > > > > +
> > > > > + pr_info("Callback from %s invoked.\n", rttd->name);
> > > > That is fine! We can output the name instead of executed counter.
> > > > Doing so makes it completely clear who triggers the callback.
> > > 
> > > And we also need to make it not trigger when CONFIG_PROVE_RCU=n.
> > > While in the area, we might as well leave anything that is needed only
> > > by CONFIG_PROVE_RCU=y undefined when CONFIG_PROVE_RCU=n.
> > > 
> > > How about the following?
> > > 
> > >   Thanx, Paul
> > > 
> > > 
> > > 
> > > commit f7a1ac0d3504e0518745da7f98573c1b13587f3e
> > > Author: Uladzislau Rezki (Sony) 
> > > Date:   Wed Dec 9 21:27:32 2020 +0100
> > > 
> > > rcu-tasks: Add RCU-ta

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Paul E. McKenney
On Mon, Dec 21, 2020 at 08:48:48PM +0100, Uladzislau Rezki wrote:
> On Mon, Dec 21, 2020 at 11:29:06AM -0800, Paul E. McKenney wrote:
> > On Mon, Dec 21, 2020 at 07:45:39PM +0100, Uladzislau Rezki wrote:
> > > On Mon, Dec 21, 2020 at 09:18:05AM -0800, Paul E. McKenney wrote:
> > > > On Mon, Dec 21, 2020 at 04:38:09PM +0100, Uladzislau Rezki wrote:
> > > > > On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> > > > > > On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > > > 
> > > > [ . . . ]
> > > > 
> > > > > > > 2.20.1
> > > > > > 
> > > > > > Again, much improved!
> > > > > > 
> > > > > See below the v3 version. I hope i fixed all comments :)
> > > > > 
> > > > > >From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 
> > > > > >2001
> > > > > From: "Uladzislau Rezki (Sony)" 
> > > > > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > > > > Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
> > > > > 
> > > > > This commit adds self tests for early-boot use of RCU-tasks grace 
> > > > > periods.
> > > > > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > > > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous 
> > > > > (e.g.,
> > > > > call_rcu_tasks()) grace-period APIs.
> > > > > 
> > > > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > > > 
> > > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > > 
> > > > Much better!
> > > > 
> > > > I pulled this in, but made one small additional change.  Please let me
> > > > know if this is problematic.
> > > > 
> > > > Thanx, Paul
> > > > 
> > > > 
> > > > 
> > > > commit 93372198b5c9efdfd288aa3b3ee41c1f90866886
> > > > Author: Uladzislau Rezki (Sony) 
> > > > Date:   Wed Dec 9 21:27:32 2020 +0100
> > > > 
> > > > rcu-tasks: Add RCU-tasks self tests
> > > > 
> > > > This commit adds self tests for early-boot use of RCU-tasks grace 
> > > > periods.
> > > > It tests all three variants (Rude, Tasks, and Tasks Trace) and 
> > > > covers
> > > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous 
> > > > (e.g.,
> > > > call_rcu_tasks()) grace-period APIs.
> > > > 
> > > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > > 
> > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > > Signed-off-by: Paul E. McKenney 
> > > > 
> > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > index 3660755..35a2cd5 100644
> > > > --- a/kernel/rcu/tasks.h
> > > > +++ b/kernel/rcu/tasks.h
> > > > @@ -1224,6 +1224,40 @@ void show_rcu_tasks_gp_kthreads(void)
> > > >  }
> > > >  #endif /* #ifndef CONFIG_TINY_RCU */
> > > >  
> > > > +struct rcu_tasks_test_desc {
> > > > +   struct rcu_head rh;
> > > > +   const char *name;
> > > > +   bool notrun;
> > > > +};
> > > > +
> > > > +static struct rcu_tasks_test_desc tests[] = {
> > > > +   {
> > > > +   .name = "call_rcu_tasks()",
> > > > +   /* If not defined, the test is skipped. */
> > > > +   .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
> > > > +   },
> > > > +   {
> > > > +   .name = "call_rcu_tasks_rude()",
> > > > +   /* If not defined, the test is skipped. */
> > > > +   .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
> > > > +   },
> > > > +   {
> > > > +   .name = "call_rcu_tasks_trace()",
> > > > +   /* If not defined, the test is skipped. */
> > > > +   .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
> > > > +   }
> > > > +};
> > > > +
> > > > +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> > > > +{
> > > > +   struct rcu_tasks_test_desc *rttd =
> > > > +   container_of(rhp, struct rcu_tasks_test_desc, rh);
> > > > +
> > > > +   pr_info("Callback from %s invoked.\n", rttd->name);
> > > That is fine! We can output the name instead of executed counter.
> > > Doing so makes it completely clear who triggers the callback.
> > 
> > And we also need to make it not trigger when CONFIG_PROVE_RCU=n.
> > While in the area, we might as well leave anything that is needed only
> > by CONFIG_PROVE_RCU=y undefined when CONFIG_PROVE_RCU=n.
> > 
> > How about the following?
> > 
> > Thanx, Paul
> > 
> > 
> > 
> > commit f7a1ac0d3504e0518745da7f98573c1b13587f3e
> > Author: Uladzislau Rezki (Sony) 
> > Date:   Wed Dec 9 21:27:32 2020 +0100
> > 
> > rcu-tasks: Add RCU-tasks self tests
> > 
> > This commit adds self tests for early-boot use of RCU-tasks grace 
> > periods.
> > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Uladzislau Rezki
On Mon, Dec 21, 2020 at 11:29:06AM -0800, Paul E. McKenney wrote:
> On Mon, Dec 21, 2020 at 07:45:39PM +0100, Uladzislau Rezki wrote:
> > On Mon, Dec 21, 2020 at 09:18:05AM -0800, Paul E. McKenney wrote:
> > > On Mon, Dec 21, 2020 at 04:38:09PM +0100, Uladzislau Rezki wrote:
> > > > On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> > > > > On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > > 
> > > [ . . . ]
> > > 
> > > > > > 2.20.1
> > > > > 
> > > > > Again, much improved!
> > > > > 
> > > > See below the v3 version. I hope i fixed all comments :)
> > > > 
> > > > >From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 2001
> > > > From: "Uladzislau Rezki (Sony)" 
> > > > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > > > Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
> > > > 
> > > > This commit adds self tests for early-boot use of RCU-tasks grace 
> > > > periods.
> > > > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> > > > call_rcu_tasks()) grace-period APIs.
> > > > 
> > > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > > 
> > > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > 
> > > Much better!
> > > 
> > > I pulled this in, but made one small additional change.  Please let me
> > > know if this is problematic.
> > > 
> > >   Thanx, Paul
> > > 
> > > 
> > > 
> > > commit 93372198b5c9efdfd288aa3b3ee41c1f90866886
> > > Author: Uladzislau Rezki (Sony) 
> > > Date:   Wed Dec 9 21:27:32 2020 +0100
> > > 
> > > rcu-tasks: Add RCU-tasks self tests
> > > 
> > > This commit adds self tests for early-boot use of RCU-tasks grace 
> > > periods.
> > > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous 
> > > (e.g.,
> > > call_rcu_tasks()) grace-period APIs.
> > > 
> > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > 
> > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > Signed-off-by: Paul E. McKenney 
> > > 
> > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > index 3660755..35a2cd5 100644
> > > --- a/kernel/rcu/tasks.h
> > > +++ b/kernel/rcu/tasks.h
> > > @@ -1224,6 +1224,40 @@ void show_rcu_tasks_gp_kthreads(void)
> > >  }
> > >  #endif /* #ifndef CONFIG_TINY_RCU */
> > >  
> > > +struct rcu_tasks_test_desc {
> > > + struct rcu_head rh;
> > > + const char *name;
> > > + bool notrun;
> > > +};
> > > +
> > > +static struct rcu_tasks_test_desc tests[] = {
> > > + {
> > > + .name = "call_rcu_tasks()",
> > > + /* If not defined, the test is skipped. */
> > > + .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
> > > + },
> > > + {
> > > + .name = "call_rcu_tasks_rude()",
> > > + /* If not defined, the test is skipped. */
> > > + .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
> > > + },
> > > + {
> > > + .name = "call_rcu_tasks_trace()",
> > > + /* If not defined, the test is skipped. */
> > > + .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
> > > + }
> > > +};
> > > +
> > > +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> > > +{
> > > + struct rcu_tasks_test_desc *rttd =
> > > + container_of(rhp, struct rcu_tasks_test_desc, rh);
> > > +
> > > + pr_info("Callback from %s invoked.\n", rttd->name);
> > That is fine! We can output the name instead of executed counter.
> > Doing so makes it completely clear who triggers the callback.
> 
> And we also need to make it not trigger when CONFIG_PROVE_RCU=n.
> While in the area, we might as well leave anything that is needed only
> by CONFIG_PROVE_RCU=y undefined when CONFIG_PROVE_RCU=n.
> 
> How about the following?
> 
>   Thanx, Paul
> 
> 
> 
> commit f7a1ac0d3504e0518745da7f98573c1b13587f3e
> Author: Uladzislau Rezki (Sony) 
> Date:   Wed Dec 9 21:27:32 2020 +0100
> 
> rcu-tasks: Add RCU-tasks self tests
> 
> This commit adds self tests for early-boot use of RCU-tasks grace periods.
> It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> call_rcu_tasks()) grace-period APIs.
> 
> Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> 
> Signed-off-by: Uladzislau Rezki (Sony) 
> [ paulmck: Handle CONFIG_PROVE_RCU=n and identify test cases' callbacks. ]
> Signed-off-by: Paul E. McKenney 
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 3660755..af7c194 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1224,6 +1224,82 @@

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Paul E. McKenney
On Mon, Dec 21, 2020 at 07:45:39PM +0100, Uladzislau Rezki wrote:
> On Mon, Dec 21, 2020 at 09:18:05AM -0800, Paul E. McKenney wrote:
> > On Mon, Dec 21, 2020 at 04:38:09PM +0100, Uladzislau Rezki wrote:
> > > On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> > > > On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > 
> > [ . . . ]
> > 
> > > > > 2.20.1
> > > > 
> > > > Again, much improved!
> > > > 
> > > See below the v3 version. I hope i fixed all comments :)
> > > 
> > > >From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 2001
> > > From: "Uladzislau Rezki (Sony)" 
> > > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > > Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
> > > 
> > > This commit adds self tests for early-boot use of RCU-tasks grace periods.
> > > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> > > call_rcu_tasks()) grace-period APIs.
> > > 
> > > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > > 
> > > Signed-off-by: Uladzislau Rezki (Sony) 
> > 
> > Much better!
> > 
> > I pulled this in, but made one small additional change.  Please let me
> > know if this is problematic.
> > 
> > Thanx, Paul
> > 
> > 
> > 
> > commit 93372198b5c9efdfd288aa3b3ee41c1f90866886
> > Author: Uladzislau Rezki (Sony) 
> > Date:   Wed Dec 9 21:27:32 2020 +0100
> > 
> > rcu-tasks: Add RCU-tasks self tests
> > 
> > This commit adds self tests for early-boot use of RCU-tasks grace 
> > periods.
> > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> > call_rcu_tasks()) grace-period APIs.
> > 
> > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) 
> > Signed-off-by: Paul E. McKenney 
> > 
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index 3660755..35a2cd5 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -1224,6 +1224,40 @@ void show_rcu_tasks_gp_kthreads(void)
> >  }
> >  #endif /* #ifndef CONFIG_TINY_RCU */
> >  
> > +struct rcu_tasks_test_desc {
> > +   struct rcu_head rh;
> > +   const char *name;
> > +   bool notrun;
> > +};
> > +
> > +static struct rcu_tasks_test_desc tests[] = {
> > +   {
> > +   .name = "call_rcu_tasks()",
> > +   /* If not defined, the test is skipped. */
> > +   .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
> > +   },
> > +   {
> > +   .name = "call_rcu_tasks_rude()",
> > +   /* If not defined, the test is skipped. */
> > +   .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
> > +   },
> > +   {
> > +   .name = "call_rcu_tasks_trace()",
> > +   /* If not defined, the test is skipped. */
> > +   .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
> > +   }
> > +};
> > +
> > +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> > +{
> > +   struct rcu_tasks_test_desc *rttd =
> > +   container_of(rhp, struct rcu_tasks_test_desc, rh);
> > +
> > +   pr_info("Callback from %s invoked.\n", rttd->name);
> That is fine! We can output the name instead of executed counter.
> Doing so makes it completely clear who triggers the callback.

And we also need to make it not trigger when CONFIG_PROVE_RCU=n.
While in the area, we might as well leave anything that is needed only
by CONFIG_PROVE_RCU=y undefined when CONFIG_PROVE_RCU=n.

How about the following?

Thanx, Paul



commit f7a1ac0d3504e0518745da7f98573c1b13587f3e
Author: Uladzislau Rezki (Sony) 
Date:   Wed Dec 9 21:27:32 2020 +0100

rcu-tasks: Add RCU-tasks self tests

This commit adds self tests for early-boot use of RCU-tasks grace periods.
It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
call_rcu_tasks()) grace-period APIs.

Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.

Signed-off-by: Uladzislau Rezki (Sony) 
[ paulmck: Handle CONFIG_PROVE_RCU=n and identify test cases' callbacks. ]
Signed-off-by: Paul E. McKenney 

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 3660755..af7c194 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1224,6 +1224,82 @@ void show_rcu_tasks_gp_kthreads(void)
 }
 #endif /* #ifndef CONFIG_TINY_RCU */
 
+#ifdef CONFIG_PROVE_RCU
+struct rcu_tasks_test_desc {
+   struct rcu_head rh;
+   const char *name;
+   bool notrun;
+};
+
+static struct rcu_tasks_test_desc tests[] = {
+   {
+  

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Uladzislau Rezki
On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > > Add self tests for checking of RCU-tasks API functionality.
> > > It covers:
> > > - wait API functions;
> > > - invoking/completion call_rcu_tasks*().
> > > 
> > > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > > 
> > > Signed-off-by: Uladzislau Rezki (Sony) 
> > > ---
> > >  kernel/rcu/tasks.h | 44 
> > >  1 file changed, 44 insertions(+)
> > > 
> > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > index 67a162949763..9407772780c1 100644
> > > --- a/kernel/rcu/tasks.h
> > > +++ b/kernel/rcu/tasks.h
> > > @@ -1225,6 +1225,16 @@ void show_rcu_tasks_gp_kthreads(void)
> > >  }
> > >  #endif /* #ifndef CONFIG_TINY_RCU */
> > >  
> > > +static struct rcu_head rhp;
> > > +static int rcu_execurted_test_counter;
> > > +static int rcu_run_test_counter;
> > > +
> > > +static void test_rcu_tasks_callback(struct rcu_head *r)
> > > +{
> > > + pr_info("RCU-tasks test callback executed %d\n",
> > > + ++rcu_execurted_test_counter);
> > > +}
> > > +
> > >  void __init rcu_init_tasks_generic(void)
> > >  {
> > >  #ifdef CONFIG_TASKS_RCU
> > > @@ -1238,7 +1248,41 @@ void __init rcu_init_tasks_generic(void)
> > >  #ifdef CONFIG_TASKS_TRACE_RCU
> > >   rcu_spawn_tasks_trace_kthread();
> > >  #endif
> > > +
> > > + if (IS_ENABLED(CONFIG_PROVE_RCU)) {
> > > + pr_info("Running RCU-tasks wait API self tests\n");
> > > +#ifdef CONFIG_TASKS_RCU
> > > + rcu_run_test_counter++;
> > > + call_rcu_tasks(&rhp, test_rcu_tasks_callback);
> > > + synchronize_rcu_tasks();
> > > +#endif
> > > +
> > > +#ifdef CONFIG_TASKS_RUDE_RCU
> > > + rcu_run_test_counter++;
> > > + call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> > > + synchronize_rcu_tasks_rude();
> > > +#endif
> > > +
> > > +#ifdef CONFIG_TASKS_TRACE_RCU
> > > + rcu_run_test_counter++;
> > > + call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> > > + synchronize_rcu_tasks_trace();
> > > +#endif
> > > + }
> > > +}
> > > +
> > > +static int rcu_tasks_verify_self_tests(void)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (rcu_run_test_counter != rcu_execurted_test_counter) {
> > > + WARN_ON(1);
> > > + ret = -1;
> > > + }
> > > +
> > > + return ret;
> > >  }
> > > +late_initcall(rcu_tasks_verify_self_tests);
> > >  
> > >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> > >  static inline void rcu_tasks_bootup_oddness(void) {}
> > Please find a v2 of the patch that is in question. First version
> > uses the same rhp for all RCU flavors what is wrong. Initially
> > i had three different one per one flavor. But for some reason
> > end up with only one.
> > 
> > 
> > >From e7c6096af5a7916f29c0b4b05e1644b3b3a6c589 Mon Sep 17 00:00:00 2001
> > From: "Uladzislau Rezki (Sony)" 
> > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > Subject: [PATCH v2 1/1] rcu-tasks: Add RCU-tasks self tests
> > 
> > This commit adds self tests for early-boot use of RCU-tasks grace periods.
> > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> > call_rcu_tasks()) grace-period APIs.
> > 
> > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) 
> 
> Much improved, thank you!  A few more comments below.
> 
> > ---
> >  kernel/rcu/tasks.h | 69 ++
> >  1 file changed, 69 insertions(+)
> > 
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index 36607551f966..7478d912734a 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -1224,6 +1224,35 @@ void show_rcu_tasks_gp_kthreads(void)
> >  }
> >  #endif /* #ifndef CONFIG_TINY_RCU */
> > 
> > +struct test_desc {
> 
> Please use something like "struct rcu_tasks_test_desc" to help the poor
> people who might need to grep for this.  Feel free to shorten it, but
> please make it descriptive and thus more likely to stay unique.
> 
> > +   struct rcu_head rh;
> > +   const char *name;
> > +   bool run;
> 
> If you make this "bool notrun" you don't need to initialize.
> 
> > +};
> > +
> > +static struct test_desc tests[] = {
> > +   { .name = "call_rcu_tasks()" },
> > +   { .name = "call_rcu_rude()"  },
> > +   { .name = "call_rcu_trace()" },
> > +};
> > +
> > +static int rcu_executed_test_counter;
> > +
> > +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> > +{
> 
>   struct rcu_tasks_test_desc *rttdp;
> 
> > +   int i;
> > +
> > +   pr_info("RCU-tasks test callback executed %d\n",
> > +   ++rcu_executed_test_counter);
> 
>   rttdp = container_of(rhp, rh, struct rcu_tasks_test_desc);
>   rttdp->notrun = true;
> 
> Or I suppose:
> 
>   container_of(rhp, rh, s

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Uladzislau Rezki
On Mon, Dec 21, 2020 at 09:18:05AM -0800, Paul E. McKenney wrote:
> On Mon, Dec 21, 2020 at 04:38:09PM +0100, Uladzislau Rezki wrote:
> > On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> > > On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> 
> [ . . . ]
> 
> > > > 2.20.1
> > > 
> > > Again, much improved!
> > > 
> > See below the v3 version. I hope i fixed all comments :)
> > 
> > >From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 2001
> > From: "Uladzislau Rezki (Sony)" 
> > Date: Wed, 9 Dec 2020 21:27:32 +0100
> > Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
> > 
> > This commit adds self tests for early-boot use of RCU-tasks grace periods.
> > It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> > both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> > call_rcu_tasks()) grace-period APIs.
> > 
> > Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) 
> 
> Much better!
> 
> I pulled this in, but made one small additional change.  Please let me
> know if this is problematic.
> 
>   Thanx, Paul
> 
> 
> 
> commit 93372198b5c9efdfd288aa3b3ee41c1f90866886
> Author: Uladzislau Rezki (Sony) 
> Date:   Wed Dec 9 21:27:32 2020 +0100
> 
> rcu-tasks: Add RCU-tasks self tests
> 
> This commit adds self tests for early-boot use of RCU-tasks grace periods.
> It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> call_rcu_tasks()) grace-period APIs.
> 
> Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> 
> Signed-off-by: Uladzislau Rezki (Sony) 
> Signed-off-by: Paul E. McKenney 
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 3660755..35a2cd5 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1224,6 +1224,40 @@ void show_rcu_tasks_gp_kthreads(void)
>  }
>  #endif /* #ifndef CONFIG_TINY_RCU */
>  
> +struct rcu_tasks_test_desc {
> + struct rcu_head rh;
> + const char *name;
> + bool notrun;
> +};
> +
> +static struct rcu_tasks_test_desc tests[] = {
> + {
> + .name = "call_rcu_tasks()",
> + /* If not defined, the test is skipped. */
> + .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
> + },
> + {
> + .name = "call_rcu_tasks_rude()",
> + /* If not defined, the test is skipped. */
> + .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
> + },
> + {
> + .name = "call_rcu_tasks_trace()",
> + /* If not defined, the test is skipped. */
> + .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
> + }
> +};
> +
> +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> +{
> + struct rcu_tasks_test_desc *rttd =
> + container_of(rhp, struct rcu_tasks_test_desc, rh);
> +
> + pr_info("Callback from %s invoked.\n", rttd->name);
That is fine! We can output the name instead of executed counter.
Doing so makes it completely clear who triggers the callback.

--
Vlad Rezki


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-21 Thread Paul E. McKenney
On Mon, Dec 21, 2020 at 04:38:09PM +0100, Uladzislau Rezki wrote:
> On Wed, Dec 16, 2020 at 03:29:55PM -0800, Paul E. McKenney wrote:
> > On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:

[ . . . ]

> > > 2.20.1
> > 
> > Again, much improved!
> > 
> See below the v3 version. I hope i fixed all comments :)
> 
> >From 06f7adfd84cbb1994d0e2693ee9dcdfd272a9bd0 Mon Sep 17 00:00:00 2001
> From: "Uladzislau Rezki (Sony)" 
> Date: Wed, 9 Dec 2020 21:27:32 +0100
> Subject: [PATCH v3 1/1] rcu-tasks: Add RCU-tasks self tests
> 
> This commit adds self tests for early-boot use of RCU-tasks grace periods.
> It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> call_rcu_tasks()) grace-period APIs.
> 
> Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> 
> Signed-off-by: Uladzislau Rezki (Sony) 

Much better!

I pulled this in, but made one small additional change.  Please let me
know if this is problematic.

Thanx, Paul



commit 93372198b5c9efdfd288aa3b3ee41c1f90866886
Author: Uladzislau Rezki (Sony) 
Date:   Wed Dec 9 21:27:32 2020 +0100

rcu-tasks: Add RCU-tasks self tests

This commit adds self tests for early-boot use of RCU-tasks grace periods.
It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
call_rcu_tasks()) grace-period APIs.

Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.

Signed-off-by: Uladzislau Rezki (Sony) 
Signed-off-by: Paul E. McKenney 

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 3660755..35a2cd5 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1224,6 +1224,40 @@ void show_rcu_tasks_gp_kthreads(void)
 }
 #endif /* #ifndef CONFIG_TINY_RCU */
 
+struct rcu_tasks_test_desc {
+   struct rcu_head rh;
+   const char *name;
+   bool notrun;
+};
+
+static struct rcu_tasks_test_desc tests[] = {
+   {
+   .name = "call_rcu_tasks()",
+   /* If not defined, the test is skipped. */
+   .notrun = !IS_ENABLED(CONFIG_TASKS_RCU),
+   },
+   {
+   .name = "call_rcu_tasks_rude()",
+   /* If not defined, the test is skipped. */
+   .notrun = !IS_ENABLED(CONFIG_TASKS_RUDE_RCU),
+   },
+   {
+   .name = "call_rcu_tasks_trace()",
+   /* If not defined, the test is skipped. */
+   .notrun = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU)
+   }
+};
+
+static void test_rcu_tasks_callback(struct rcu_head *rhp)
+{
+   struct rcu_tasks_test_desc *rttd =
+   container_of(rhp, struct rcu_tasks_test_desc, rh);
+
+   pr_info("Callback from %s invoked.\n", rttd->name);
+
+   rttd->notrun = true;
+}
+
 void __init rcu_init_tasks_generic(void)
 {
 #ifdef CONFIG_TASKS_RCU
@@ -1237,7 +1271,45 @@ void __init rcu_init_tasks_generic(void)
 #ifdef CONFIG_TASKS_TRACE_RCU
rcu_spawn_tasks_trace_kthread();
 #endif
+
+   // Run the self-tests.
+   if (IS_ENABLED(CONFIG_PROVE_RCU)) {
+   pr_info("Running RCU-tasks wait API self tests\n");
+#ifdef CONFIG_TASKS_RCU
+   synchronize_rcu_tasks();
+   call_rcu_tasks(&tests[0].rh, test_rcu_tasks_callback);
+#endif
+
+#ifdef CONFIG_TASKS_RUDE_RCU
+   synchronize_rcu_tasks_rude();
+   call_rcu_tasks_rude(&tests[1].rh, test_rcu_tasks_callback);
+#endif
+
+#ifdef CONFIG_TASKS_TRACE_RCU
+   synchronize_rcu_tasks_trace();
+   call_rcu_tasks_trace(&tests[2].rh, test_rcu_tasks_callback);
+#endif
+   }
+}
+
+static int rcu_tasks_verify_self_tests(void)
+{
+   int ret = 0;
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(tests); i++) {
+   if (!tests[i].notrun) { // still hanging.
+   pr_err("%s has been failed.\n", tests[i].name);
+   ret = -1;
+   }
+   }
+
+   if (ret)
+   WARN_ON(1);
+
+   return ret;
 }
+late_initcall(rcu_tasks_verify_self_tests);
 
 #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
 static inline void rcu_tasks_bootup_oddness(void) {}


Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-16 Thread Paul E. McKenney
On Wed, Dec 16, 2020 at 04:49:59PM +0100, Uladzislau Rezki wrote:
> > Add self tests for checking of RCU-tasks API functionality.
> > It covers:
> > - wait API functions;
> > - invoking/completion call_rcu_tasks*().
> > 
> > Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) 
> > ---
> >  kernel/rcu/tasks.h | 44 
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index 67a162949763..9407772780c1 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -1225,6 +1225,16 @@ void show_rcu_tasks_gp_kthreads(void)
> >  }
> >  #endif /* #ifndef CONFIG_TINY_RCU */
> >  
> > +static struct rcu_head rhp;
> > +static int rcu_execurted_test_counter;
> > +static int rcu_run_test_counter;
> > +
> > +static void test_rcu_tasks_callback(struct rcu_head *r)
> > +{
> > +   pr_info("RCU-tasks test callback executed %d\n",
> > +   ++rcu_execurted_test_counter);
> > +}
> > +
> >  void __init rcu_init_tasks_generic(void)
> >  {
> >  #ifdef CONFIG_TASKS_RCU
> > @@ -1238,7 +1248,41 @@ void __init rcu_init_tasks_generic(void)
> >  #ifdef CONFIG_TASKS_TRACE_RCU
> > rcu_spawn_tasks_trace_kthread();
> >  #endif
> > +
> > +   if (IS_ENABLED(CONFIG_PROVE_RCU)) {
> > +   pr_info("Running RCU-tasks wait API self tests\n");
> > +#ifdef CONFIG_TASKS_RCU
> > +   rcu_run_test_counter++;
> > +   call_rcu_tasks(&rhp, test_rcu_tasks_callback);
> > +   synchronize_rcu_tasks();
> > +#endif
> > +
> > +#ifdef CONFIG_TASKS_RUDE_RCU
> > +   rcu_run_test_counter++;
> > +   call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> > +   synchronize_rcu_tasks_rude();
> > +#endif
> > +
> > +#ifdef CONFIG_TASKS_TRACE_RCU
> > +   rcu_run_test_counter++;
> > +   call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> > +   synchronize_rcu_tasks_trace();
> > +#endif
> > +   }
> > +}
> > +
> > +static int rcu_tasks_verify_self_tests(void)
> > +{
> > +   int ret = 0;
> > +
> > +   if (rcu_run_test_counter != rcu_execurted_test_counter) {
> > +   WARN_ON(1);
> > +   ret = -1;
> > +   }
> > +
> > +   return ret;
> >  }
> > +late_initcall(rcu_tasks_verify_self_tests);
> >  
> >  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
> >  static inline void rcu_tasks_bootup_oddness(void) {}
> Please find a v2 of the patch that is in question. First version
> uses the same rhp for all RCU flavors what is wrong. Initially
> i had three different one per one flavor. But for some reason
> end up with only one.
> 
> 
> >From e7c6096af5a7916f29c0b4b05e1644b3b3a6c589 Mon Sep 17 00:00:00 2001
> From: "Uladzislau Rezki (Sony)" 
> Date: Wed, 9 Dec 2020 21:27:32 +0100
> Subject: [PATCH v2 1/1] rcu-tasks: Add RCU-tasks self tests
> 
> This commit adds self tests for early-boot use of RCU-tasks grace periods.
> It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
> both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
> call_rcu_tasks()) grace-period APIs.
> 
> Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.
> 
> Signed-off-by: Uladzislau Rezki (Sony) 

Much improved, thank you!  A few more comments below.

> ---
>  kernel/rcu/tasks.h | 69 ++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 36607551f966..7478d912734a 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1224,6 +1224,35 @@ void show_rcu_tasks_gp_kthreads(void)
>  }
>  #endif /* #ifndef CONFIG_TINY_RCU */
> 
> +struct test_desc {

Please use something like "struct rcu_tasks_test_desc" to help the poor
people who might need to grep for this.  Feel free to shorten it, but
please make it descriptive and thus more likely to stay unique.

> +   struct rcu_head rh;
> +   const char *name;
> +   bool run;

If you make this "bool notrun" you don't need to initialize.

> +};
> +
> +static struct test_desc tests[] = {
> +   { .name = "call_rcu_tasks()" },
> +   { .name = "call_rcu_rude()"  },
> +   { .name = "call_rcu_trace()" },
> +};
> +
> +static int rcu_executed_test_counter;
> +
> +static void test_rcu_tasks_callback(struct rcu_head *rhp)
> +{

struct rcu_tasks_test_desc *rttdp;

> +   int i;
> +
> +   pr_info("RCU-tasks test callback executed %d\n",
> +   ++rcu_executed_test_counter);

rttdp = container_of(rhp, rh, struct rcu_tasks_test_desc);
rttdp->notrun = true;

Or I suppose:

container_of(rhp, rh, struct rcu_tasks_test_desc)->notrun = true;

Then the loop below can go away.

> +
> +   for (i = 0; i < ARRAY_SIZE(tests); i++) {
> +   if (rhp == &tests[i].rh) {
> +   tests[i].run = false;
> +   break;
> +   }
> +   }
> +}
> +
>  void __i

Re: [PATCH 2/2] rcu-tasks: add RCU-tasks self tests

2020-12-16 Thread Uladzislau Rezki
> Add self tests for checking of RCU-tasks API functionality.
> It covers:
> - wait API functions;
> - invoking/completion call_rcu_tasks*().
> 
> Self-tests are run when CONFIG_PROVE_RCU kernel parameter is set.
> 
> Signed-off-by: Uladzislau Rezki (Sony) 
> ---
>  kernel/rcu/tasks.h | 44 
>  1 file changed, 44 insertions(+)
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 67a162949763..9407772780c1 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1225,6 +1225,16 @@ void show_rcu_tasks_gp_kthreads(void)
>  }
>  #endif /* #ifndef CONFIG_TINY_RCU */
>  
> +static struct rcu_head rhp;
> +static int rcu_execurted_test_counter;
> +static int rcu_run_test_counter;
> +
> +static void test_rcu_tasks_callback(struct rcu_head *r)
> +{
> + pr_info("RCU-tasks test callback executed %d\n",
> + ++rcu_execurted_test_counter);
> +}
> +
>  void __init rcu_init_tasks_generic(void)
>  {
>  #ifdef CONFIG_TASKS_RCU
> @@ -1238,7 +1248,41 @@ void __init rcu_init_tasks_generic(void)
>  #ifdef CONFIG_TASKS_TRACE_RCU
>   rcu_spawn_tasks_trace_kthread();
>  #endif
> +
> + if (IS_ENABLED(CONFIG_PROVE_RCU)) {
> + pr_info("Running RCU-tasks wait API self tests\n");
> +#ifdef CONFIG_TASKS_RCU
> + rcu_run_test_counter++;
> + call_rcu_tasks(&rhp, test_rcu_tasks_callback);
> + synchronize_rcu_tasks();
> +#endif
> +
> +#ifdef CONFIG_TASKS_RUDE_RCU
> + rcu_run_test_counter++;
> + call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> + synchronize_rcu_tasks_rude();
> +#endif
> +
> +#ifdef CONFIG_TASKS_TRACE_RCU
> + rcu_run_test_counter++;
> + call_rcu_tasks_trace(&rhp, test_rcu_tasks_callback);
> + synchronize_rcu_tasks_trace();
> +#endif
> + }
> +}
> +
> +static int rcu_tasks_verify_self_tests(void)
> +{
> + int ret = 0;
> +
> + if (rcu_run_test_counter != rcu_execurted_test_counter) {
> + WARN_ON(1);
> + ret = -1;
> + }
> +
> + return ret;
>  }
> +late_initcall(rcu_tasks_verify_self_tests);
>  
>  #else /* #ifdef CONFIG_TASKS_RCU_GENERIC */
>  static inline void rcu_tasks_bootup_oddness(void) {}
Please find a v2 of the patch that is in question. First version
uses the same rhp for all RCU flavors what is wrong. Initially
i had three different one per one flavor. But for some reason
end up with only one.


>From e7c6096af5a7916f29c0b4b05e1644b3b3a6c589 Mon Sep 17 00:00:00 2001
From: "Uladzislau Rezki (Sony)" 
Date: Wed, 9 Dec 2020 21:27:32 +0100
Subject: [PATCH v2 1/1] rcu-tasks: Add RCU-tasks self tests

This commit adds self tests for early-boot use of RCU-tasks grace periods.
It tests all three variants (Rude, Tasks, and Tasks Trace) and covers
both synchronous (e.g., synchronize_rcu_tasks()) and asynchronous (e.g.,
call_rcu_tasks()) grace-period APIs.

Self-tests are run only in kernels built with CONFIG_PROVE_RCU=y.

Signed-off-by: Uladzislau Rezki (Sony) 
---
 kernel/rcu/tasks.h | 69 ++
 1 file changed, 69 insertions(+)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 36607551f966..7478d912734a 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1224,6 +1224,35 @@ void show_rcu_tasks_gp_kthreads(void)
 }
 #endif /* #ifndef CONFIG_TINY_RCU */

+struct test_desc {
+   struct rcu_head rh;
+   const char *name;
+   bool run;
+};
+
+static struct test_desc tests[] = {
+   { .name = "call_rcu_tasks()" },
+   { .name = "call_rcu_rude()"  },
+   { .name = "call_rcu_trace()" },
+};
+
+static int rcu_executed_test_counter;
+
+static void test_rcu_tasks_callback(struct rcu_head *rhp)
+{
+   int i;
+
+   pr_info("RCU-tasks test callback executed %d\n",
+   ++rcu_executed_test_counter);
+
+   for (i = 0; i < ARRAY_SIZE(tests); i++) {
+   if (rhp == &tests[i].rh) {
+   tests[i].run = false;
+   break;
+   }
+   }
+}
+
 void __init rcu_init_tasks_generic(void)
 {
 #ifdef CONFIG_TASKS_RCU
@@ -1237,7 +1266,47 @@ void __init rcu_init_tasks_generic(void)
 #ifdef CONFIG_TASKS_TRACE_RCU
rcu_spawn_tasks_trace_kthread();
 #endif
+
+   // Run the self-tests.
+   if (IS_ENABLED(CONFIG_PROVE_RCU)) {
+   pr_info("Running RCU-tasks wait API self tests\n");
+#ifdef CONFIG_TASKS_RCU
+   tests[0].run = true;
+   call_rcu_tasks(&tests[0].rh, test_rcu_tasks_callback);
+   synchronize_rcu_tasks();
+#endif
+
+#ifdef CONFIG_TASKS_RUDE_RCU
+   tests[1].run = true;
+   call_rcu_tasks_rude(&tests[1].rh, test_rcu_tasks_callback);
+   synchronize_rcu_tasks_rude();
+#endif
+
+#ifdef CONFIG_TASKS_TRACE_RCU
+   tests[2].run = true;
+   call_rcu_tasks_trace(&tests[2].rh, test_rcu_tasks_callback);
+