[PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Uladzislau Rezki (Sony)]

To stress and test a single argument of kfree_rcu() call, we
should to have a special coverage for it. We used to have it
in the test-suite related to vmalloc stressing. The reason is
the rcuscale is a correct place for RCU related things.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 kernel/rcu/rcuscale.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 06491d5530db..e17745a155f9 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -94,6 +94,7 @@ torture_param(bool, shutdown, RCUSCALE_SHUTDOWN,
 torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
 torture_param(int, writer_holdoff, 0, "Holdoff (us) between GPs, zero to disable");
 torture_param(int, kfree_rcu_test, 0, "Do we run a kfree_rcu() scale test?");
+torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
 torture_param(int, kfree_mult, 1, "Multiple of kfree_obj size to allocate.");
 
 static char *scale_type = "rcu";
@@ -667,10 +668,14 @@ kfree_scale_thread(void *arg)
 
 		for (i = 0; i < kfree_alloc_num; i++) {
 			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
+
 			if (!alloc_ptr)
 				return -ENOMEM;
 
-			kfree_rcu(alloc_ptr, rh);
+			if (kfree_rcu_test_single)
+				kfree_rcu(alloc_ptr);
+			else
+				kfree_rcu(alloc_ptr, rh);
 		}
 
 		cond_resched();
-- 
2.20.1

[PATCH 2/2] kvfree_rcu: Use same set of flags as for single-argument

[Uladzislau Rezki (Sony)]

Running an rcuscale stress-suite can lead to "Out of memory"
of a system. This can happen under high memory pressure with
a small amount of physical memory.

For example a KVM test configuration with 64 CPUs and 512 megabytes
can lead to of memory after running rcuscale with below parameters:

../kvm.sh --torture rcuscale --allcpus --duration 10 --kconfig CONFIG_NR_CPUS=64 \
--bootargs "rcuscale.kfree_rcu_test=1 rcuscale.kfree_nthreads=16 rcuscale.holdoff=20 \
  rcuscale.kfree_loops=10000 torture.disable_onoff_at_boot" --trust-make

<snip>
[   12.054448] kworker/1:1H invoked oom-killer: gfp_mask=0x2cc0(GFP_KERNEL|__GFP_NOWARN), order=0, oom_score_adj=0
[   12.055303] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
[   12.055416] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
[   12.056485] Workqueue: events_highpri fill_page_cache_func
[   12.056485] Call Trace:
[   12.056485]  dump_stack+0x57/0x6a
[   12.056485]  dump_header+0x4c/0x30a
[   12.056485]  ? del_timer_sync+0x20/0x30
[   12.056485]  out_of_memory.cold.47+0xa/0x7e
[   12.056485]  __alloc_pages_slowpath.constprop.123+0x82f/0xc00
[   12.056485]  __alloc_pages_nodemask+0x289/0x2c0
[   12.056485]  __get_free_pages+0x8/0x30
[   12.056485]  fill_page_cache_func+0x39/0xb0
[   12.056485]  process_one_work+0x1ed/0x3b0
[   12.056485]  ? process_one_work+0x3b0/0x3b0
[   12.060485]  worker_thread+0x28/0x3c0
[   12.060485]  ? process_one_work+0x3b0/0x3b0
[   12.060485]  kthread+0x138/0x160
[   12.060485]  ? kthread_park+0x80/0x80
[   12.060485]  ret_from_fork+0x22/0x30
[   12.062156] Mem-Info:
[   12.062350] active_anon:0 inactive_anon:0 isolated_anon:0
[   12.062350]  active_file:0 inactive_file:0 isolated_file:0
[   12.062350]  unevictable:0 dirty:0 writeback:0
[   12.062350]  slab_reclaimable:2797 slab_unreclaimable:80920
[   12.062350]  mapped:1 shmem:2 pagetables:8 bounce:0
[   12.062350]  free:10488 free_pcp:1227 free_cma:0
...
[   12.101610] Out of memory and no killable processes...
[   12.102042] Kernel panic - not syncing: System is deadlocked on memory
[   12.102583] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
[   12.102600] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
<snip>

Having a fallback mechanism we should not go with "GFP_KERNEL | __GFP_NOWARN"
that implies a "hard" page request involving OOM killer. Replace such set with
the same as the one used for a single argument.

Thus it will follow same rules:
    a) minimize a fallback hitting;
    b) avoid of OOM invoking;
    c) do a light-wait page request;
    d) avoid of dipping into the emergency reserves.

With this change an rcuscale and the parameters which are in question
never runs into "Kernel panic".

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 kernel/rcu/tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 1e862120db9e..2c9cf4df942c 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3435,7 +3435,7 @@ static void fill_page_cache_func(struct work_struct *work)
 
 	for (i = 0; i < rcu_min_cached_objs; i++) {
 		bnode = (struct kvfree_rcu_bulk_data *)
-			__get_free_page(GFP_KERNEL | __GFP_NOWARN);
+			__get_free_page(GFP_KERNEL | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
 
 		if (bnode) {
 			raw_spin_lock_irqsave(&krcp->lock, flags);
-- 
2.20.1

Re: [PATCH 2/2] kvfree_rcu: Use same set of flags as for single-argument

[Paul E. McKenney]

On Fri, Jan 29, 2021 at 09:05:05PM +0100, Uladzislau Rezki (Sony) wrote:
> Running an rcuscale stress-suite can lead to "Out of memory"
> of a system. This can happen under high memory pressure with
> a small amount of physical memory.
> 
> For example a KVM test configuration with 64 CPUs and 512 megabytes
> can lead to of memory after running rcuscale with below parameters:
> 
> ../kvm.sh --torture rcuscale --allcpus --duration 10 --kconfig CONFIG_NR_CPUS=64 \
> --bootargs "rcuscale.kfree_rcu_test=1 rcuscale.kfree_nthreads=16 rcuscale.holdoff=20 \
>   rcuscale.kfree_loops=10000 torture.disable_onoff_at_boot" --trust-make
> 
> <snip>
> [   12.054448] kworker/1:1H invoked oom-killer: gfp_mask=0x2cc0(GFP_KERNEL|__GFP_NOWARN), order=0, oom_score_adj=0
> [   12.055303] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
> [   12.055416] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
> [   12.056485] Workqueue: events_highpri fill_page_cache_func
> [   12.056485] Call Trace:
> [   12.056485]  dump_stack+0x57/0x6a
> [   12.056485]  dump_header+0x4c/0x30a
> [   12.056485]  ? del_timer_sync+0x20/0x30
> [   12.056485]  out_of_memory.cold.47+0xa/0x7e
> [   12.056485]  __alloc_pages_slowpath.constprop.123+0x82f/0xc00
> [   12.056485]  __alloc_pages_nodemask+0x289/0x2c0
> [   12.056485]  __get_free_pages+0x8/0x30
> [   12.056485]  fill_page_cache_func+0x39/0xb0
> [   12.056485]  process_one_work+0x1ed/0x3b0
> [   12.056485]  ? process_one_work+0x3b0/0x3b0
> [   12.060485]  worker_thread+0x28/0x3c0
> [   12.060485]  ? process_one_work+0x3b0/0x3b0
> [   12.060485]  kthread+0x138/0x160
> [   12.060485]  ? kthread_park+0x80/0x80
> [   12.060485]  ret_from_fork+0x22/0x30
> [   12.062156] Mem-Info:
> [   12.062350] active_anon:0 inactive_anon:0 isolated_anon:0
> [   12.062350]  active_file:0 inactive_file:0 isolated_file:0
> [   12.062350]  unevictable:0 dirty:0 writeback:0
> [   12.062350]  slab_reclaimable:2797 slab_unreclaimable:80920
> [   12.062350]  mapped:1 shmem:2 pagetables:8 bounce:0
> [   12.062350]  free:10488 free_pcp:1227 free_cma:0
> ...
> [   12.101610] Out of memory and no killable processes...
> [   12.102042] Kernel panic - not syncing: System is deadlocked on memory
> [   12.102583] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
> [   12.102600] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
> <snip>
> 
> Having a fallback mechanism we should not go with "GFP_KERNEL | __GFP_NOWARN"
> that implies a "hard" page request involving OOM killer. Replace such set with
> the same as the one used for a single argument.
> 
> Thus it will follow same rules:
>     a) minimize a fallback hitting;
>     b) avoid of OOM invoking;
>     c) do a light-wait page request;
>     d) avoid of dipping into the emergency reserves.
> 
> With this change an rcuscale and the parameters which are in question
> never runs into "Kernel panic".
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>

I did have some misgivings about this one, but after a closer look at
the GFP flags you suggest along with offlist discussions it looks like
what needs to happen.  So thank you for persisting!  ;-)

I did the usual wordsmithing as shown below, so please check to make
sure that I did not mess anything up.

							Thanx, Paul

------------------------------------------------------------------------

commit f60c420f9de536e7fc397f0ad8d6677d782d0141
Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
Date:   Fri Jan 29 21:05:05 2021 +0100

    kvfree_rcu: Use same set of GFP flags as does single-argument
    
    Running an rcuscale stress-suite can lead to "Out of memory" of a
    system. This can happen under high memory pressure with a small amount
    of physical memory.
    
    For example, a KVM test configuration with 64 CPUs and 512 megabytes
    can result in OOM when running rcuscale with below parameters:
    
    ../kvm.sh --torture rcuscale --allcpus --duration 10 --kconfig CONFIG_NR_CPUS=64 \
    --bootargs "rcuscale.kfree_rcu_test=1 rcuscale.kfree_nthreads=16 rcuscale.holdoff=20 \
      rcuscale.kfree_loops=10000 torture.disable_onoff_at_boot" --trust-make
    
    <snip>
    [   12.054448] kworker/1:1H invoked oom-killer: gfp_mask=0x2cc0(GFP_KERNEL|__GFP_NOWARN), order=0, oom_score_adj=0
    [   12.055303] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
    [   12.055416] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
    [   12.056485] Workqueue: events_highpri fill_page_cache_func
    [   12.056485] Call Trace:
    [   12.056485]  dump_stack+0x57/0x6a
    [   12.056485]  dump_header+0x4c/0x30a
    [   12.056485]  ? del_timer_sync+0x20/0x30
    [   12.056485]  out_of_memory.cold.47+0xa/0x7e
    [   12.056485]  __alloc_pages_slowpath.constprop.123+0x82f/0xc00
    [   12.056485]  __alloc_pages_nodemask+0x289/0x2c0
    [   12.056485]  __get_free_pages+0x8/0x30
    [   12.056485]  fill_page_cache_func+0x39/0xb0
    [   12.056485]  process_one_work+0x1ed/0x3b0
    [   12.056485]  ? process_one_work+0x3b0/0x3b0
    [   12.060485]  worker_thread+0x28/0x3c0
    [   12.060485]  ? process_one_work+0x3b0/0x3b0
    [   12.060485]  kthread+0x138/0x160
    [   12.060485]  ? kthread_park+0x80/0x80
    [   12.060485]  ret_from_fork+0x22/0x30
    [   12.062156] Mem-Info:
    [   12.062350] active_anon:0 inactive_anon:0 isolated_anon:0
    [   12.062350]  active_file:0 inactive_file:0 isolated_file:0
    [   12.062350]  unevictable:0 dirty:0 writeback:0
    [   12.062350]  slab_reclaimable:2797 slab_unreclaimable:80920
    [   12.062350]  mapped:1 shmem:2 pagetables:8 bounce:0
    [   12.062350]  free:10488 free_pcp:1227 free_cma:0
    ...
    [   12.101610] Out of memory and no killable processes...
    [   12.102042] Kernel panic - not syncing: System is deadlocked on memory
    [   12.102583] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
    [   12.102600] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
    <snip>
    
    Because kvfree_rcu() has a fallback path, memory allocation failure is
    not the end of the world.  Furthermore, the added overhead of aggressive
    GFP settings must be balanced against the overhead of the fallback path,
    which is a cache miss for double-argument kvfree_rcu() and a call to
    synchronize_rcu() for single-argument kvfree_rcu().  The current choice
    of GFP_KERNEL|__GFP_NOWARN can result in longer latencies than a call
    to synchronize_rcu(), so less-tenacious GFP flags would be helpful.
    
    Here is the tradeoff that must be balanced:
        a) Minimize use of the fallback path,
        b) Avoid pushing the system into OOM,
        c) Bound allocation latency to that of synchronize_rcu(), and
        d) Leave the emergency reserves to use cases lacking fallbacks.
    
    This commit therefore changes GFP flags from GFP_KERNEL|__GFP_NOWARN to
    GFP_KERNEL|__GFP_NORETRY|__GFP_NOMEMALLOC|__GFP_NOWARN.  This combination
    leaves the emergency reserves alone and can initiate reclaim, but will
    not invoke the OOM killer.
    
    Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 1e86212..2c9cf4d 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3435,7 +3435,7 @@ static void fill_page_cache_func(struct work_struct *work)
 
 	for (i = 0; i < rcu_min_cached_objs; i++) {
 		bnode = (struct kvfree_rcu_bulk_data *)
-			__get_free_page(GFP_KERNEL | __GFP_NOWARN);
+			__get_free_page(GFP_KERNEL | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
 
 		if (bnode) {
 			raw_spin_lock_irqsave(&krcp->lock, flags);

Re: [PATCH 2/2] kvfree_rcu: Use same set of flags as for single-argument

[Uladzislau Rezki]

On Thu, Feb 04, 2021 at 02:04:27PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 29, 2021 at 09:05:05PM +0100, Uladzislau Rezki (Sony) wrote:
> > Running an rcuscale stress-suite can lead to "Out of memory"
> > of a system. This can happen under high memory pressure with
> > a small amount of physical memory.
> > 
> > For example a KVM test configuration with 64 CPUs and 512 megabytes
> > can lead to of memory after running rcuscale with below parameters:
> > 
> > ../kvm.sh --torture rcuscale --allcpus --duration 10 --kconfig CONFIG_NR_CPUS=64 \
> > --bootargs "rcuscale.kfree_rcu_test=1 rcuscale.kfree_nthreads=16 rcuscale.holdoff=20 \
> >   rcuscale.kfree_loops=10000 torture.disable_onoff_at_boot" --trust-make
> > 
> > <snip>
> > [   12.054448] kworker/1:1H invoked oom-killer: gfp_mask=0x2cc0(GFP_KERNEL|__GFP_NOWARN), order=0, oom_score_adj=0
> > [   12.055303] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
> > [   12.055416] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
> > [   12.056485] Workqueue: events_highpri fill_page_cache_func
> > [   12.056485] Call Trace:
> > [   12.056485]  dump_stack+0x57/0x6a
> > [   12.056485]  dump_header+0x4c/0x30a
> > [   12.056485]  ? del_timer_sync+0x20/0x30
> > [   12.056485]  out_of_memory.cold.47+0xa/0x7e
> > [   12.056485]  __alloc_pages_slowpath.constprop.123+0x82f/0xc00
> > [   12.056485]  __alloc_pages_nodemask+0x289/0x2c0
> > [   12.056485]  __get_free_pages+0x8/0x30
> > [   12.056485]  fill_page_cache_func+0x39/0xb0
> > [   12.056485]  process_one_work+0x1ed/0x3b0
> > [   12.056485]  ? process_one_work+0x3b0/0x3b0
> > [   12.060485]  worker_thread+0x28/0x3c0
> > [   12.060485]  ? process_one_work+0x3b0/0x3b0
> > [   12.060485]  kthread+0x138/0x160
> > [   12.060485]  ? kthread_park+0x80/0x80
> > [   12.060485]  ret_from_fork+0x22/0x30
> > [   12.062156] Mem-Info:
> > [   12.062350] active_anon:0 inactive_anon:0 isolated_anon:0
> > [   12.062350]  active_file:0 inactive_file:0 isolated_file:0
> > [   12.062350]  unevictable:0 dirty:0 writeback:0
> > [   12.062350]  slab_reclaimable:2797 slab_unreclaimable:80920
> > [   12.062350]  mapped:1 shmem:2 pagetables:8 bounce:0
> > [   12.062350]  free:10488 free_pcp:1227 free_cma:0
> > ...
> > [   12.101610] Out of memory and no killable processes...
> > [   12.102042] Kernel panic - not syncing: System is deadlocked on memory
> > [   12.102583] CPU: 1 PID: 377 Comm: kworker/1:1H Not tainted 5.11.0-rc3+ #510
> > [   12.102600] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
> > <snip>
> > 
> > Having a fallback mechanism we should not go with "GFP_KERNEL | __GFP_NOWARN"
> > that implies a "hard" page request involving OOM killer. Replace such set with
> > the same as the one used for a single argument.
> > 
> > Thus it will follow same rules:
> >     a) minimize a fallback hitting;
> >     b) avoid of OOM invoking;
> >     c) do a light-wait page request;
> >     d) avoid of dipping into the emergency reserves.
> > 
> > With this change an rcuscale and the parameters which are in question
> > never runs into "Kernel panic".
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> 
> I did have some misgivings about this one, but after a closer look at
> the GFP flags you suggest along with offlist discussions it looks like
> what needs to happen.  So thank you for persisting!  ;-)
> 
> I did the usual wordsmithing as shown below, so please check to make
> sure that I did not mess anything up.
> 
Looks good to me, i mean a rewording of the commit message :)

--
Vlad Rezki

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Uladzislau Rezki]

Hello, Paul.

> To stress and test a single argument of kfree_rcu() call, we
> should to have a special coverage for it. We used to have it
> in the test-suite related to vmalloc stressing. The reason is
> the rcuscale is a correct place for RCU related things.
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  kernel/rcu/rcuscale.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> index 06491d5530db..e17745a155f9 100644
> --- a/kernel/rcu/rcuscale.c
> +++ b/kernel/rcu/rcuscale.c
> @@ -94,6 +94,7 @@ torture_param(bool, shutdown, RCUSCALE_SHUTDOWN,
>  torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
>  torture_param(int, writer_holdoff, 0, "Holdoff (us) between GPs, zero to disable");
>  torture_param(int, kfree_rcu_test, 0, "Do we run a kfree_rcu() scale test?");
> +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
>  torture_param(int, kfree_mult, 1, "Multiple of kfree_obj size to allocate.");
>  
>  static char *scale_type = "rcu";
> @@ -667,10 +668,14 @@ kfree_scale_thread(void *arg)
>  
>  		for (i = 0; i < kfree_alloc_num; i++) {
>  			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> +
>  			if (!alloc_ptr)
>  				return -ENOMEM;
>  
> -			kfree_rcu(alloc_ptr, rh);
> +			if (kfree_rcu_test_single)
> +				kfree_rcu(alloc_ptr);
> +			else
> +				kfree_rcu(alloc_ptr, rh);
>  		}
>  
>  		cond_resched();
> -- 
> 2.20.1
>
What is about this change? Do you have any concern or comments?

--
Vlad Rezki

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Paul E. McKenney]

On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> To stress and test a single argument of kfree_rcu() call, we
> should to have a special coverage for it. We used to have it
> in the test-suite related to vmalloc stressing. The reason is
> the rcuscale is a correct place for RCU related things.
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>

This is a great addition, but it would be even better if there was
a way to say "test both in one run".  One way to do this is to have
torture_param() variables for both kfree_rcu_test_single and (say)
kfree_rcu_test_double, both bool and both initialized to false.  If both
have the same value (false or true) both are tested, otherwise only
the one with value true is tested.  The value of this is that it allows
testing of both options with one test.

See the gp_cond, gp_exp, gp_normal, gp_poll, and gp_sync torture_param()
variables in kernel/rcu/rcutorture.c and how they are handled in
rcu_torture_write_types() for one way to do this.

If you wanted to vary the intensity of the testing (but I don't see why
this would be useful), the various weight_* torture_param() variables
are dealt with by kernel/scftorture.c in scf_torture_init().

Thoughts?

							Thanx, Paul

> ---
>  kernel/rcu/rcuscale.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> index 06491d5530db..e17745a155f9 100644
> --- a/kernel/rcu/rcuscale.c
> +++ b/kernel/rcu/rcuscale.c
> @@ -94,6 +94,7 @@ torture_param(bool, shutdown, RCUSCALE_SHUTDOWN,
>  torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
>  torture_param(int, writer_holdoff, 0, "Holdoff (us) between GPs, zero to disable");
>  torture_param(int, kfree_rcu_test, 0, "Do we run a kfree_rcu() scale test?");
> +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
>  torture_param(int, kfree_mult, 1, "Multiple of kfree_obj size to allocate.");
>  
>  static char *scale_type = "rcu";
> @@ -667,10 +668,14 @@ kfree_scale_thread(void *arg)
>  
>  		for (i = 0; i < kfree_alloc_num; i++) {
>  			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> +
>  			if (!alloc_ptr)
>  				return -ENOMEM;
>  
> -			kfree_rcu(alloc_ptr, rh);
> +			if (kfree_rcu_test_single)
> +				kfree_rcu(alloc_ptr);
> +			else
> +				kfree_rcu(alloc_ptr, rh);
>  		}
>  
>  		cond_resched();
> -- 
> 2.20.1
> 

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Uladzislau Rezki]

On Thu, Feb 04, 2021 at 01:46:48PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> > To stress and test a single argument of kfree_rcu() call, we
> > should to have a special coverage for it. We used to have it
> > in the test-suite related to vmalloc stressing. The reason is
> > the rcuscale is a correct place for RCU related things.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> 
> This is a great addition, but it would be even better if there was
> a way to say "test both in one run".  One way to do this is to have
> torture_param() variables for both kfree_rcu_test_single and (say)
> kfree_rcu_test_double, both bool and both initialized to false.  If both
> have the same value (false or true) both are tested, otherwise only
> the one with value true is tested.  The value of this is that it allows
> testing of both options with one test.
> 
Make sense to me :)

From ba083a543a123455455c81230b7b5a9aa2a9cb7f Mon Sep 17 00:00:00 2001
From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
Date: Fri, 29 Jan 2021 19:51:27 +0100
Subject: [PATCH v2 1/1] rcuscale: add kfree_rcu() single-argument scale test

To stress and test a single argument of kfree_rcu() call, we
should to have a special coverage for it. We used to have it
in the test-suite related to vmalloc stressing. The reason is
the rcuscale is a correct place for RCU related things.

Therefore introduce two torture_param() variables, one is for
single-argument scale test and another one for double-argument
scale test.

By default kfree_rcu_test_single and kfree_rcu_test_double are
initialized to false. If both have the same value (false or true)
both are tested in one run, otherwise only the one with value
true is tested. The value of this is that it allows testing of
both options with one test.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 kernel/rcu/rcuscale.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 06491d5530db..0cde5c17f06c 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
 torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
 torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
 torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
+torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
+torture_param(int, kfree_rcu_test_double, 0, "Do we run a kfree_rcu() double-argument scale test?");
 
 static struct task_struct **kfree_reader_tasks;
 static int kfree_nrealthreads;
@@ -641,7 +643,7 @@ kfree_scale_thread(void *arg)
 {
 	int i, loop = 0;
 	long me = (long)arg;
-	struct kfree_obj *alloc_ptr;
+	struct kfree_obj *alloc_ptr[2];
 	u64 start_time, end_time;
 	long long mem_begin, mem_during = 0;
 
@@ -665,12 +667,33 @@ kfree_scale_thread(void *arg)
 			mem_during = (mem_during + si_mem_available()) / 2;
 		}
 
+		// By default kfree_rcu_test_single and kfree_rcu_test_double are
+		// initialized to false. If both have the same value (false or true)
+		// both are tested in one run, otherwise only the one with value
+		// true is tested.
 		for (i = 0; i < kfree_alloc_num; i++) {
-			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
-			if (!alloc_ptr)
-				return -ENOMEM;
+			alloc_ptr[0] = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
+			alloc_ptr[1] = (kfree_rcu_test_single == kfree_rcu_test_double) ?
+				kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL) : NULL;
+
+			// 0 ptr. is freed either over single or double argument.
+			if (alloc_ptr[0]) {
+				if (kfree_rcu_test_single == kfree_rcu_test_double ||
+						kfree_rcu_test_single) {
+					kfree_rcu(alloc_ptr[0]);
+				} else {
+					kfree_rcu(alloc_ptr[0], rh);
+				}
+			}
+
+			// 1 ptr. is always freed over double argument.
+			if (alloc_ptr[1])
+				kfree_rcu(alloc_ptr[1], rh);
 
-			kfree_rcu(alloc_ptr, rh);
+			if (!alloc_ptr[0] ||
+					(kfree_rcu_test_single == kfree_rcu_test_double &&
+						!alloc_ptr[1]))
+				return -ENOMEM;
 		}
 
 		cond_resched();
-- 
2.20.1

--
Vlad Rezki

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Paul E. McKenney]

On Tue, Feb 09, 2021 at 09:13:43PM +0100, Uladzislau Rezki wrote:
> On Thu, Feb 04, 2021 at 01:46:48PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> > > To stress and test a single argument of kfree_rcu() call, we
> > > should to have a special coverage for it. We used to have it
> > > in the test-suite related to vmalloc stressing. The reason is
> > > the rcuscale is a correct place for RCU related things.
> > > 
> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > 
> > This is a great addition, but it would be even better if there was
> > a way to say "test both in one run".  One way to do this is to have
> > torture_param() variables for both kfree_rcu_test_single and (say)
> > kfree_rcu_test_double, both bool and both initialized to false.  If both
> > have the same value (false or true) both are tested, otherwise only
> > the one with value true is tested.  The value of this is that it allows
> > testing of both options with one test.
> > 
> Make sense to me :)
> 
> >From ba083a543a123455455c81230b7b5a9aa2a9cb7f Mon Sep 17 00:00:00 2001
> From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
> Date: Fri, 29 Jan 2021 19:51:27 +0100
> Subject: [PATCH v2 1/1] rcuscale: add kfree_rcu() single-argument scale test
> 
> To stress and test a single argument of kfree_rcu() call, we
> should to have a special coverage for it. We used to have it
> in the test-suite related to vmalloc stressing. The reason is
> the rcuscale is a correct place for RCU related things.
> 
> Therefore introduce two torture_param() variables, one is for
> single-argument scale test and another one for double-argument
> scale test.
> 
> By default kfree_rcu_test_single and kfree_rcu_test_double are
> initialized to false. If both have the same value (false or true)
> both are tested in one run, otherwise only the one with value
> true is tested. The value of this is that it allows testing of
> both options with one test.
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  kernel/rcu/rcuscale.c | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> index 06491d5530db..0cde5c17f06c 100644
> --- a/kernel/rcu/rcuscale.c
> +++ b/kernel/rcu/rcuscale.c
> @@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
>  torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
>  torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
>  torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
> +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
> +torture_param(int, kfree_rcu_test_double, 0, "Do we run a kfree_rcu() double-argument scale test?");

Good!  But why int instead of bool?

>  static struct task_struct **kfree_reader_tasks;
>  static int kfree_nrealthreads;
> @@ -641,7 +643,7 @@ kfree_scale_thread(void *arg)
>  {
>  	int i, loop = 0;
>  	long me = (long)arg;
> -	struct kfree_obj *alloc_ptr;
> +	struct kfree_obj *alloc_ptr[2];

You lost me on this one...

>  	u64 start_time, end_time;
>  	long long mem_begin, mem_during = 0;
>  
> @@ -665,12 +667,33 @@ kfree_scale_thread(void *arg)
>  			mem_during = (mem_during + si_mem_available()) / 2;
>  		}
>  
> +		// By default kfree_rcu_test_single and kfree_rcu_test_double are
> +		// initialized to false. If both have the same value (false or true)
> +		// both are tested in one run, otherwise only the one with value
> +		// true is tested.
>  		for (i = 0; i < kfree_alloc_num; i++) {
> -			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> -			if (!alloc_ptr)
> -				return -ENOMEM;
> +			alloc_ptr[0] = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> +			alloc_ptr[1] = (kfree_rcu_test_single == kfree_rcu_test_double) ?
> +				kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL) : NULL;
> +
> +			// 0 ptr. is freed either over single or double argument.
> +			if (alloc_ptr[0]) {
> +				if (kfree_rcu_test_single == kfree_rcu_test_double ||
> +						kfree_rcu_test_single) {
> +					kfree_rcu(alloc_ptr[0]);
> +				} else {
> +					kfree_rcu(alloc_ptr[0], rh);
> +				}
> +			}
> +
> +			// 1 ptr. is always freed over double argument.
> +			if (alloc_ptr[1])
> +				kfree_rcu(alloc_ptr[1], rh);
>  
> -			kfree_rcu(alloc_ptr, rh);
> +			if (!alloc_ptr[0] ||
> +					(kfree_rcu_test_single == kfree_rcu_test_double &&
> +						!alloc_ptr[1]))
> +				return -ENOMEM;

How about something like this?

	bool krts = kfree_rcu_test_single || kfree_rcu_test_single == kfree_rcu_test_double;
	bool krtd = kfree_rcu_test_double || kfree_rcu_test_single == kfree_rcu_test_double;
	bool krtb = kfree_rcu_test_single && kfree_rcu_test_double;
	DEFINE_TORTURE_RANDOM(tr);

	...

			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
			if (!alloc_ptr)
				return -ENOMEM;
			if (krtd || (krtb && (torture_random(&tr) & 0x800)))
				kfree_rcu(alloc_ptr, rh);
			else
				kfree_rcu(alloc_ptr);

>  		}
>  
>  		cond_resched();

And this is why I was so confused about the earlier OOMs.  We need
something stronger, and not here, but rather inside the above loop.
The function rcu_torture_fwd_prog_cond_resched() does what is needed,
which needs to be moved to kernel/torture.c or to be a static inline in
include/linux/torture.h so that it can be invoked here.

The flooding we are looking to emulate has to have frequent trips into
userspace, and rcu_torture_fwd_prog_cond_resched() is the way that we
emulate those trips.

But please make this change be a separate patch.

							Thanx, Paul

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Uladzislau Rezki]

On Tue, Feb 09, 2021 at 05:00:52PM -0800, Paul E. McKenney wrote:
> On Tue, Feb 09, 2021 at 09:13:43PM +0100, Uladzislau Rezki wrote:
> > On Thu, Feb 04, 2021 at 01:46:48PM -0800, Paul E. McKenney wrote:
> > > On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > To stress and test a single argument of kfree_rcu() call, we
> > > > should to have a special coverage for it. We used to have it
> > > > in the test-suite related to vmalloc stressing. The reason is
> > > > the rcuscale is a correct place for RCU related things.
> > > > 
> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > 
> > > This is a great addition, but it would be even better if there was
> > > a way to say "test both in one run".  One way to do this is to have
> > > torture_param() variables for both kfree_rcu_test_single and (say)
> > > kfree_rcu_test_double, both bool and both initialized to false.  If both
> > > have the same value (false or true) both are tested, otherwise only
> > > the one with value true is tested.  The value of this is that it allows
> > > testing of both options with one test.
> > > 
> > Make sense to me :)
> > 
> > >From ba083a543a123455455c81230b7b5a9aa2a9cb7f Mon Sep 17 00:00:00 2001
> > From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
> > Date: Fri, 29 Jan 2021 19:51:27 +0100
> > Subject: [PATCH v2 1/1] rcuscale: add kfree_rcu() single-argument scale test
> > 
> > To stress and test a single argument of kfree_rcu() call, we
> > should to have a special coverage for it. We used to have it
> > in the test-suite related to vmalloc stressing. The reason is
> > the rcuscale is a correct place for RCU related things.
> > 
> > Therefore introduce two torture_param() variables, one is for
> > single-argument scale test and another one for double-argument
> > scale test.
> > 
> > By default kfree_rcu_test_single and kfree_rcu_test_double are
> > initialized to false. If both have the same value (false or true)
> > both are tested in one run, otherwise only the one with value
> > true is tested. The value of this is that it allows testing of
> > both options with one test.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > ---
> >  kernel/rcu/rcuscale.c | 33 ++++++++++++++++++++++++++++-----
> >  1 file changed, 28 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> > index 06491d5530db..0cde5c17f06c 100644
> > --- a/kernel/rcu/rcuscale.c
> > +++ b/kernel/rcu/rcuscale.c
> > @@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
> >  torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
> >  torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
> >  torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
> > +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
> > +torture_param(int, kfree_rcu_test_double, 0, "Do we run a kfree_rcu() double-argument scale test?");
> 
> Good!  But why int instead of bool?
> 
> >  static struct task_struct **kfree_reader_tasks;
> >  static int kfree_nrealthreads;
> > @@ -641,7 +643,7 @@ kfree_scale_thread(void *arg)
> >  {
> >  	int i, loop = 0;
> >  	long me = (long)arg;
> > -	struct kfree_obj *alloc_ptr;
> > +	struct kfree_obj *alloc_ptr[2];
> 
> You lost me on this one...
> 
> >  	u64 start_time, end_time;
> >  	long long mem_begin, mem_during = 0;
> >  
> > @@ -665,12 +667,33 @@ kfree_scale_thread(void *arg)
> >  			mem_during = (mem_during + si_mem_available()) / 2;
> >  		}
> >  
> > +		// By default kfree_rcu_test_single and kfree_rcu_test_double are
> > +		// initialized to false. If both have the same value (false or true)
> > +		// both are tested in one run, otherwise only the one with value
> > +		// true is tested.
> >  		for (i = 0; i < kfree_alloc_num; i++) {
> > -			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > -			if (!alloc_ptr)
> > -				return -ENOMEM;
> > +			alloc_ptr[0] = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > +			alloc_ptr[1] = (kfree_rcu_test_single == kfree_rcu_test_double) ?
> > +				kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL) : NULL;
> > +
> > +			// 0 ptr. is freed either over single or double argument.
> > +			if (alloc_ptr[0]) {
> > +				if (kfree_rcu_test_single == kfree_rcu_test_double ||
> > +						kfree_rcu_test_single) {
> > +					kfree_rcu(alloc_ptr[0]);
> > +				} else {
> > +					kfree_rcu(alloc_ptr[0], rh);
> > +				}
> > +			}
> > +
> > +			// 1 ptr. is always freed over double argument.
> > +			if (alloc_ptr[1])
> > +				kfree_rcu(alloc_ptr[1], rh);
> >  
> > -			kfree_rcu(alloc_ptr, rh);
> > +			if (!alloc_ptr[0] ||
> > +					(kfree_rcu_test_single == kfree_rcu_test_double &&
> > +						!alloc_ptr[1]))
> > +				return -ENOMEM;
> 
> How about something like this?
> 
> 	bool krts = kfree_rcu_test_single || kfree_rcu_test_single == kfree_rcu_test_double;
> 	bool krtd = kfree_rcu_test_double || kfree_rcu_test_single == kfree_rcu_test_double;
> 	bool krtb = kfree_rcu_test_single && kfree_rcu_test_double;
> 	DEFINE_TORTURE_RANDOM(tr);
> 
> 	...
> 
> 			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> 			if (!alloc_ptr)
> 				return -ENOMEM;
> 			if (krtd || (krtb && (torture_random(&tr) & 0x800)))
> 				kfree_rcu(alloc_ptr, rh);
> 			else
> 				kfree_rcu(alloc_ptr);
> 
> >  		}
> >  
> >  		cond_resched();
>
Sorry for my late answer. I got it differently as we discussed offline.
Please see below the v3. Hope we are on the same page now :)

From e7181d01c5f7fab3418d6df155ccf06aff189328 Mon Sep 17 00:00:00 2001
From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
Date: Fri, 29 Jan 2021 19:51:27 +0100
Subject: [PATCH v3 1/1] rcuscale: add kfree_rcu() single-argument scale test

To stress and test a single argument of kfree_rcu() call, we
should to have a special coverage for it. We used to have it
in the test-suite related to vmalloc stressing. The reason is
the rcuscale is a correct place for RCU related things.

Therefore introduce two torture_param() variables, one is for
single-argument scale test and another one for double-argument
scale test.

By default kfree_rcu_test_single and kfree_rcu_test_double are
initialized to false. If both have the same value (false or true)
both are randomly tested, otherwise only the one with value true
is tested. The value of this is that it allows testing of both
options with one test.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 kernel/rcu/rcuscale.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 06491d5530db..0fb540e2b22b 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
 torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
 torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
 torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
+torture_param(bool, kfree_rcu_test_single, false, "Do we run a kfree_rcu() single-argument scale test?");
+torture_param(bool, kfree_rcu_test_double, false, "Do we run a kfree_rcu() double-argument scale test?");
 
 static struct task_struct **kfree_reader_tasks;
 static int kfree_nrealthreads;
@@ -644,10 +646,13 @@ kfree_scale_thread(void *arg)
 	struct kfree_obj *alloc_ptr;
 	u64 start_time, end_time;
 	long long mem_begin, mem_during = 0;
+	bool kfree_rcu_test_both;
+	DEFINE_TORTURE_RANDOM(tr);
 
 	VERBOSE_SCALEOUT_STRING("kfree_scale_thread task started");
 	set_cpus_allowed_ptr(current, cpumask_of(me % nr_cpu_ids));
 	set_user_nice(current, MAX_NICE);
+	kfree_rcu_test_both = (kfree_rcu_test_single == kfree_rcu_test_double);
 
 	start_time = ktime_get_mono_fast_ns();
 
@@ -670,7 +675,15 @@ kfree_scale_thread(void *arg)
 			if (!alloc_ptr)
 				return -ENOMEM;
 
-			kfree_rcu(alloc_ptr, rh);
+			// By default kfree_rcu_test_single and kfree_rcu_test_double are
+			// initialized to false. If both have the same value (false or true)
+			// both are randomly tested, otherwise only the one with value true
+			// is tested.
+			if ((kfree_rcu_test_single && !kfree_rcu_test_double) ||
+					(kfree_rcu_test_both && torture_random(&tr) & 0x800))
+				kfree_rcu(alloc_ptr);
+			else
+				kfree_rcu(alloc_ptr, rh);
 		}
 
 		cond_resched();
-- 
2.20.1

> 
> And this is why I was so confused about the earlier OOMs.  We need
> something stronger, and not here, but rather inside the above loop.
> The function rcu_torture_fwd_prog_cond_resched() does what is needed,
> which needs to be moved to kernel/torture.c or to be a static inline in
> include/linux/torture.h so that it can be invoked here.
> 
> The flooding we are looking to emulate has to have frequent trips into
> userspace, and rcu_torture_fwd_prog_cond_resched() is the way that we
> emulate those trips.
> 
> But please make this change be a separate patch.
> 
OK. I will have a look at it!

--
Vlad Rezki

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Paul E. McKenney]

On Mon, Feb 15, 2021 at 05:27:05PM +0100, Uladzislau Rezki wrote:
> On Tue, Feb 09, 2021 at 05:00:52PM -0800, Paul E. McKenney wrote:
> > On Tue, Feb 09, 2021 at 09:13:43PM +0100, Uladzislau Rezki wrote:
> > > On Thu, Feb 04, 2021 at 01:46:48PM -0800, Paul E. McKenney wrote:
> > > > On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > > To stress and test a single argument of kfree_rcu() call, we
> > > > > should to have a special coverage for it. We used to have it
> > > > > in the test-suite related to vmalloc stressing. The reason is
> > > > > the rcuscale is a correct place for RCU related things.
> > > > > 
> > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > > 
> > > > This is a great addition, but it would be even better if there was
> > > > a way to say "test both in one run".  One way to do this is to have
> > > > torture_param() variables for both kfree_rcu_test_single and (say)
> > > > kfree_rcu_test_double, both bool and both initialized to false.  If both
> > > > have the same value (false or true) both are tested, otherwise only
> > > > the one with value true is tested.  The value of this is that it allows
> > > > testing of both options with one test.
> > > > 
> > > Make sense to me :)
> > > 
> > > >From ba083a543a123455455c81230b7b5a9aa2a9cb7f Mon Sep 17 00:00:00 2001
> > > From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
> > > Date: Fri, 29 Jan 2021 19:51:27 +0100
> > > Subject: [PATCH v2 1/1] rcuscale: add kfree_rcu() single-argument scale test
> > > 
> > > To stress and test a single argument of kfree_rcu() call, we
> > > should to have a special coverage for it. We used to have it
> > > in the test-suite related to vmalloc stressing. The reason is
> > > the rcuscale is a correct place for RCU related things.
> > > 
> > > Therefore introduce two torture_param() variables, one is for
> > > single-argument scale test and another one for double-argument
> > > scale test.
> > > 
> > > By default kfree_rcu_test_single and kfree_rcu_test_double are
> > > initialized to false. If both have the same value (false or true)
> > > both are tested in one run, otherwise only the one with value
> > > true is tested. The value of this is that it allows testing of
> > > both options with one test.
> > > 
> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > ---
> > >  kernel/rcu/rcuscale.c | 33 ++++++++++++++++++++++++++++-----
> > >  1 file changed, 28 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> > > index 06491d5530db..0cde5c17f06c 100644
> > > --- a/kernel/rcu/rcuscale.c
> > > +++ b/kernel/rcu/rcuscale.c
> > > @@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
> > >  torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
> > >  torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
> > >  torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
> > > +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
> > > +torture_param(int, kfree_rcu_test_double, 0, "Do we run a kfree_rcu() double-argument scale test?");
> > 
> > Good!  But why int instead of bool?
> > 
> > >  static struct task_struct **kfree_reader_tasks;
> > >  static int kfree_nrealthreads;
> > > @@ -641,7 +643,7 @@ kfree_scale_thread(void *arg)
> > >  {
> > >  	int i, loop = 0;
> > >  	long me = (long)arg;
> > > -	struct kfree_obj *alloc_ptr;
> > > +	struct kfree_obj *alloc_ptr[2];
> > 
> > You lost me on this one...
> > 
> > >  	u64 start_time, end_time;
> > >  	long long mem_begin, mem_during = 0;
> > >  
> > > @@ -665,12 +667,33 @@ kfree_scale_thread(void *arg)
> > >  			mem_during = (mem_during + si_mem_available()) / 2;
> > >  		}
> > >  
> > > +		// By default kfree_rcu_test_single and kfree_rcu_test_double are
> > > +		// initialized to false. If both have the same value (false or true)
> > > +		// both are tested in one run, otherwise only the one with value
> > > +		// true is tested.
> > >  		for (i = 0; i < kfree_alloc_num; i++) {
> > > -			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > > -			if (!alloc_ptr)
> > > -				return -ENOMEM;
> > > +			alloc_ptr[0] = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > > +			alloc_ptr[1] = (kfree_rcu_test_single == kfree_rcu_test_double) ?
> > > +				kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL) : NULL;
> > > +
> > > +			// 0 ptr. is freed either over single or double argument.
> > > +			if (alloc_ptr[0]) {
> > > +				if (kfree_rcu_test_single == kfree_rcu_test_double ||
> > > +						kfree_rcu_test_single) {
> > > +					kfree_rcu(alloc_ptr[0]);
> > > +				} else {
> > > +					kfree_rcu(alloc_ptr[0], rh);
> > > +				}
> > > +			}
> > > +
> > > +			// 1 ptr. is always freed over double argument.
> > > +			if (alloc_ptr[1])
> > > +				kfree_rcu(alloc_ptr[1], rh);
> > >  
> > > -			kfree_rcu(alloc_ptr, rh);
> > > +			if (!alloc_ptr[0] ||
> > > +					(kfree_rcu_test_single == kfree_rcu_test_double &&
> > > +						!alloc_ptr[1]))
> > > +				return -ENOMEM;
> > 
> > How about something like this?
> > 
> > 	bool krts = kfree_rcu_test_single || kfree_rcu_test_single == kfree_rcu_test_double;
> > 	bool krtd = kfree_rcu_test_double || kfree_rcu_test_single == kfree_rcu_test_double;
> > 	bool krtb = kfree_rcu_test_single && kfree_rcu_test_double;
> > 	DEFINE_TORTURE_RANDOM(tr);
> > 
> > 	...
> > 
> > 			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > 			if (!alloc_ptr)
> > 				return -ENOMEM;
> > 			if (krtd || (krtb && (torture_random(&tr) & 0x800)))
> > 				kfree_rcu(alloc_ptr, rh);
> > 			else
> > 				kfree_rcu(alloc_ptr);
> > 
> > >  		}
> > >  
> > >  		cond_resched();
> >
> Sorry for my late answer. I got it differently as we discussed offline.
> Please see below the v3. Hope we are on the same page now :)

This does look good to me!  Could you please send it as an email
containing only the patch, just to make it official?  And to catch the
attention of anyone who might have tuned out of this email thread.  ;-)

							Thanx, Paul

> >From e7181d01c5f7fab3418d6df155ccf06aff189328 Mon Sep 17 00:00:00 2001
> From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
> Date: Fri, 29 Jan 2021 19:51:27 +0100
> Subject: [PATCH v3 1/1] rcuscale: add kfree_rcu() single-argument scale test
> 
> To stress and test a single argument of kfree_rcu() call, we
> should to have a special coverage for it. We used to have it
> in the test-suite related to vmalloc stressing. The reason is
> the rcuscale is a correct place for RCU related things.
> 
> Therefore introduce two torture_param() variables, one is for
> single-argument scale test and another one for double-argument
> scale test.
> 
> By default kfree_rcu_test_single and kfree_rcu_test_double are
> initialized to false. If both have the same value (false or true)
> both are randomly tested, otherwise only the one with value true
> is tested. The value of this is that it allows testing of both
> options with one test.
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  kernel/rcu/rcuscale.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> index 06491d5530db..0fb540e2b22b 100644
> --- a/kernel/rcu/rcuscale.c
> +++ b/kernel/rcu/rcuscale.c
> @@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
>  torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
>  torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
>  torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
> +torture_param(bool, kfree_rcu_test_single, false, "Do we run a kfree_rcu() single-argument scale test?");
> +torture_param(bool, kfree_rcu_test_double, false, "Do we run a kfree_rcu() double-argument scale test?");
>  
>  static struct task_struct **kfree_reader_tasks;
>  static int kfree_nrealthreads;
> @@ -644,10 +646,13 @@ kfree_scale_thread(void *arg)
>  	struct kfree_obj *alloc_ptr;
>  	u64 start_time, end_time;
>  	long long mem_begin, mem_during = 0;
> +	bool kfree_rcu_test_both;
> +	DEFINE_TORTURE_RANDOM(tr);
>  
>  	VERBOSE_SCALEOUT_STRING("kfree_scale_thread task started");
>  	set_cpus_allowed_ptr(current, cpumask_of(me % nr_cpu_ids));
>  	set_user_nice(current, MAX_NICE);
> +	kfree_rcu_test_both = (kfree_rcu_test_single == kfree_rcu_test_double);
>  
>  	start_time = ktime_get_mono_fast_ns();
>  
> @@ -670,7 +675,15 @@ kfree_scale_thread(void *arg)
>  			if (!alloc_ptr)
>  				return -ENOMEM;
>  
> -			kfree_rcu(alloc_ptr, rh);
> +			// By default kfree_rcu_test_single and kfree_rcu_test_double are
> +			// initialized to false. If both have the same value (false or true)
> +			// both are randomly tested, otherwise only the one with value true
> +			// is tested.
> +			if ((kfree_rcu_test_single && !kfree_rcu_test_double) ||
> +					(kfree_rcu_test_both && torture_random(&tr) & 0x800))
> +				kfree_rcu(alloc_ptr);
> +			else
> +				kfree_rcu(alloc_ptr, rh);
>  		}
>  
>  		cond_resched();
> -- 
> 2.20.1
> 
> > 
> > And this is why I was so confused about the earlier OOMs.  We need
> > something stronger, and not here, but rather inside the above loop.
> > The function rcu_torture_fwd_prog_cond_resched() does what is needed,
> > which needs to be moved to kernel/torture.c or to be a static inline in
> > include/linux/torture.h so that it can be invoked here.
> > 
> > The flooding we are looking to emulate has to have frequent trips into
> > userspace, and rcu_torture_fwd_prog_cond_resched() is the way that we
> > emulate those trips.
> > 
> > But please make this change be a separate patch.
> > 
> OK. I will have a look at it!
> 
> --
> Vlad Rezki

Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test

[Uladzislau Rezki]

On Tue, Feb 16, 2021 at 09:35:02AM -0800, Paul E. McKenney wrote:
> On Mon, Feb 15, 2021 at 05:27:05PM +0100, Uladzislau Rezki wrote:
> > On Tue, Feb 09, 2021 at 05:00:52PM -0800, Paul E. McKenney wrote:
> > > On Tue, Feb 09, 2021 at 09:13:43PM +0100, Uladzislau Rezki wrote:
> > > > On Thu, Feb 04, 2021 at 01:46:48PM -0800, Paul E. McKenney wrote:
> > > > > On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > > > To stress and test a single argument of kfree_rcu() call, we
> > > > > > should to have a special coverage for it. We used to have it
> > > > > > in the test-suite related to vmalloc stressing. The reason is
> > > > > > the rcuscale is a correct place for RCU related things.
> > > > > > 
> > > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > > > 
> > > > > This is a great addition, but it would be even better if there was
> > > > > a way to say "test both in one run".  One way to do this is to have
> > > > > torture_param() variables for both kfree_rcu_test_single and (say)
> > > > > kfree_rcu_test_double, both bool and both initialized to false.  If both
> > > > > have the same value (false or true) both are tested, otherwise only
> > > > > the one with value true is tested.  The value of this is that it allows
> > > > > testing of both options with one test.
> > > > > 
> > > > Make sense to me :)
> > > > 
> > > > >From ba083a543a123455455c81230b7b5a9aa2a9cb7f Mon Sep 17 00:00:00 2001
> > > > From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
> > > > Date: Fri, 29 Jan 2021 19:51:27 +0100
> > > > Subject: [PATCH v2 1/1] rcuscale: add kfree_rcu() single-argument scale test
> > > > 
> > > > To stress and test a single argument of kfree_rcu() call, we
> > > > should to have a special coverage for it. We used to have it
> > > > in the test-suite related to vmalloc stressing. The reason is
> > > > the rcuscale is a correct place for RCU related things.
> > > > 
> > > > Therefore introduce two torture_param() variables, one is for
> > > > single-argument scale test and another one for double-argument
> > > > scale test.
> > > > 
> > > > By default kfree_rcu_test_single and kfree_rcu_test_double are
> > > > initialized to false. If both have the same value (false or true)
> > > > both are tested in one run, otherwise only the one with value
> > > > true is tested. The value of this is that it allows testing of
> > > > both options with one test.
> > > > 
> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > > ---
> > > >  kernel/rcu/rcuscale.c | 33 ++++++++++++++++++++++++++++-----
> > > >  1 file changed, 28 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> > > > index 06491d5530db..0cde5c17f06c 100644
> > > > --- a/kernel/rcu/rcuscale.c
> > > > +++ b/kernel/rcu/rcuscale.c
> > > > @@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
> > > >  torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
> > > >  torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
> > > >  torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
> > > > +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
> > > > +torture_param(int, kfree_rcu_test_double, 0, "Do we run a kfree_rcu() double-argument scale test?");
> > > 
> > > Good!  But why int instead of bool?
> > > 
> > > >  static struct task_struct **kfree_reader_tasks;
> > > >  static int kfree_nrealthreads;
> > > > @@ -641,7 +643,7 @@ kfree_scale_thread(void *arg)
> > > >  {
> > > >  	int i, loop = 0;
> > > >  	long me = (long)arg;
> > > > -	struct kfree_obj *alloc_ptr;
> > > > +	struct kfree_obj *alloc_ptr[2];
> > > 
> > > You lost me on this one...
> > > 
> > > >  	u64 start_time, end_time;
> > > >  	long long mem_begin, mem_during = 0;
> > > >  
> > > > @@ -665,12 +667,33 @@ kfree_scale_thread(void *arg)
> > > >  			mem_during = (mem_during + si_mem_available()) / 2;
> > > >  		}
> > > >  
> > > > +		// By default kfree_rcu_test_single and kfree_rcu_test_double are
> > > > +		// initialized to false. If both have the same value (false or true)
> > > > +		// both are tested in one run, otherwise only the one with value
> > > > +		// true is tested.
> > > >  		for (i = 0; i < kfree_alloc_num; i++) {
> > > > -			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > > > -			if (!alloc_ptr)
> > > > -				return -ENOMEM;
> > > > +			alloc_ptr[0] = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > > > +			alloc_ptr[1] = (kfree_rcu_test_single == kfree_rcu_test_double) ?
> > > > +				kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL) : NULL;
> > > > +
> > > > +			// 0 ptr. is freed either over single or double argument.
> > > > +			if (alloc_ptr[0]) {
> > > > +				if (kfree_rcu_test_single == kfree_rcu_test_double ||
> > > > +						kfree_rcu_test_single) {
> > > > +					kfree_rcu(alloc_ptr[0]);
> > > > +				} else {
> > > > +					kfree_rcu(alloc_ptr[0], rh);
> > > > +				}
> > > > +			}
> > > > +
> > > > +			// 1 ptr. is always freed over double argument.
> > > > +			if (alloc_ptr[1])
> > > > +				kfree_rcu(alloc_ptr[1], rh);
> > > >  
> > > > -			kfree_rcu(alloc_ptr, rh);
> > > > +			if (!alloc_ptr[0] ||
> > > > +					(kfree_rcu_test_single == kfree_rcu_test_double &&
> > > > +						!alloc_ptr[1]))
> > > > +				return -ENOMEM;
> > > 
> > > How about something like this?
> > > 
> > > 	bool krts = kfree_rcu_test_single || kfree_rcu_test_single == kfree_rcu_test_double;
> > > 	bool krtd = kfree_rcu_test_double || kfree_rcu_test_single == kfree_rcu_test_double;
> > > 	bool krtb = kfree_rcu_test_single && kfree_rcu_test_double;
> > > 	DEFINE_TORTURE_RANDOM(tr);
> > > 
> > > 	...
> > > 
> > > 			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > > 			if (!alloc_ptr)
> > > 				return -ENOMEM;
> > > 			if (krtd || (krtb && (torture_random(&tr) & 0x800)))
> > > 				kfree_rcu(alloc_ptr, rh);
> > > 			else
> > > 				kfree_rcu(alloc_ptr);
> > > 
> > > >  		}
> > > >  
> > > >  		cond_resched();
> > >
> > Sorry for my late answer. I got it differently as we discussed offline.
> > Please see below the v3. Hope we are on the same page now :)
> 
> This does look good to me!  Could you please send it as an email
> containing only the patch, just to make it official?  And to catch the
> attention of anyone who might have tuned out of this email thread.  ;-)
> 
I will send out as a fresh patch :)

--
Vlad Rezki