| Message ID | 20250225110020.59221-1-urezki@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/3] rcutorture: Allow a negative value for nfakewriters | expand |
On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: > Currently "nfakewriters" parameter can be set to any value but > there is no possibility to adjust it automatically based on how > many CPUs a system has where a test is run on. > > To address this, if the "nfakewriters" is set to negative it will > be adjusted to num_online_cpus() during torture initialization. > > Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > --- > kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > index d98b3bd6d91f..f376262532ce 100644 > --- a/kernel/rcu/rcutorture.c > +++ b/kernel/rcu/rcutorture.c > @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); IMO, this should also be updated to reflect the possibily to set it negative and hence to number CPUs: torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); thanks, - Joel > > static int nrealnocbers; > static int nrealreaders; > +static int nrealfakewriters; > static struct task_struct *writer_task; > static struct task_struct **fakewriter_tasks; > static struct task_struct **reader_tasks; > @@ -1763,7 +1764,7 @@ rcu_torture_fakewriter(void *arg) > do { > torture_hrtimeout_jiffies(torture_random(&rand) % 10, &rand); > if (cur_ops->cb_barrier != NULL && > - torture_random(&rand) % (nfakewriters * 8) == 0) { > + torture_random(&rand) % (nrealfakewriters * 8) == 0) { > cur_ops->cb_barrier(); > } else { > switch (synctype[torture_random(&rand) % nsynctypes]) { > @@ -2568,7 +2569,7 @@ rcu_torture_print_module_parms(struct rcu_torture_ops *cur_ops, const char *tag) > "nocbs_nthreads=%d nocbs_toggle=%d " > "test_nmis=%d " > "preempt_duration=%d preempt_interval=%d\n", > - torture_type, tag, nrealreaders, nfakewriters, > + torture_type, tag, nrealreaders, nrealfakewriters, > stat_interval, verbose, test_no_idle_hz, shuffle_interval, > stutter, irqreader, fqs_duration, fqs_holdoff, fqs_stutter, > test_boost, cur_ops->can_boost, > @@ -3644,7 +3645,7 @@ rcu_torture_cleanup(void) > rcu_torture_reader_mbchk = NULL; > > if (fakewriter_tasks) { > - for (i = 0; i < nfakewriters; i++) > + for (i = 0; i < nrealfakewriters; i++) > torture_stop_kthread(rcu_torture_fakewriter, > fakewriter_tasks[i]); > kfree(fakewriter_tasks); > @@ -4066,6 +4067,14 @@ rcu_torture_init(void) > > rcu_torture_init_srcu_lockdep(); > > + if (nfakewriters >= 0) { > + nrealfakewriters = nfakewriters; > + } else { > + nrealfakewriters = num_online_cpus() - 2 - nfakewriters; > + if (nrealfakewriters <= 0) > + nrealfakewriters = 1; > + } > + > if (nreaders >= 0) { > nrealreaders = nreaders; > } else { > @@ -4122,8 +4131,9 @@ rcu_torture_init(void) > writer_task); > if (torture_init_error(firsterr)) > goto unwind; > - if (nfakewriters > 0) { > - fakewriter_tasks = kcalloc(nfakewriters, > + > + if (nrealfakewriters > 0) { > + fakewriter_tasks = kcalloc(nrealfakewriters, > sizeof(fakewriter_tasks[0]), > GFP_KERNEL); > if (fakewriter_tasks == NULL) { > @@ -4132,7 +4142,7 @@ rcu_torture_init(void) > goto unwind; > } > } > - for (i = 0; i < nfakewriters; i++) { > + for (i = 0; i < nrealfakewriters; i++) { > firsterr = torture_create_kthread(rcu_torture_fakewriter, > NULL, fakewriter_tasks[i]); > if (torture_init_error(firsterr)) > -- > 2.39.5 >
On Tue, Feb 25, 2025 at 04:24:09PM -0500, Joel Fernandes wrote: > On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: > > Currently "nfakewriters" parameter can be set to any value but > > there is no possibility to adjust it automatically based on how > > many CPUs a system has where a test is run on. > > > > To address this, if the "nfakewriters" is set to negative it will > > be adjusted to num_online_cpus() during torture initialization. > > > > Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > --- > > kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > index d98b3bd6d91f..f376262532ce 100644 > > --- a/kernel/rcu/rcutorture.c > > +++ b/kernel/rcu/rcutorture.c > > @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); > > IMO, this should also be updated to reflect the possibily to set it negative > and hence to number CPUs: > > torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); > You can set it to a negative as well as to number of CPUs or any other number. -- Uladzislau Rezki
On Wed, Feb 26, 2025 at 03:29:09PM +0100, Uladzislau Rezki wrote: > On Tue, Feb 25, 2025 at 04:24:09PM -0500, Joel Fernandes wrote: > > On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: > > > Currently "nfakewriters" parameter can be set to any value but > > > there is no possibility to adjust it automatically based on how > > > many CPUs a system has where a test is run on. > > > > > > To address this, if the "nfakewriters" is set to negative it will > > > be adjusted to num_online_cpus() during torture initialization. > > > > > > Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > --- > > > kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ > > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > > index d98b3bd6d91f..f376262532ce 100644 > > > --- a/kernel/rcu/rcutorture.c > > > +++ b/kernel/rcu/rcutorture.c > > > @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); > > > > IMO, this should also be updated to reflect the possibily to set it negative > > and hence to number CPUs: > > > > torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); > > > You can set it to a negative as well as to number of CPUs or any other > number. Joel does have a good point, though. Could I interest one of you in updating the Documentation/admin-guide/kernel-parameters.txt entry for rcutorture.nfakewriters? The rcutorture.nreaders entry is a decent guide. Thanx, Paul
On Wed, Feb 26, 2025 at 06:50:02AM -0800, Paul E. McKenney wrote: > On Wed, Feb 26, 2025 at 03:29:09PM +0100, Uladzislau Rezki wrote: > > On Tue, Feb 25, 2025 at 04:24:09PM -0500, Joel Fernandes wrote: > > > On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: > > > > Currently "nfakewriters" parameter can be set to any value but > > > > there is no possibility to adjust it automatically based on how > > > > many CPUs a system has where a test is run on. > > > > > > > > To address this, if the "nfakewriters" is set to negative it will > > > > be adjusted to num_online_cpus() during torture initialization. > > > > > > > > Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > > --- > > > > kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ > > > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > > > index d98b3bd6d91f..f376262532ce 100644 > > > > --- a/kernel/rcu/rcutorture.c > > > > +++ b/kernel/rcu/rcutorture.c > > > > @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); > > > > > > IMO, this should also be updated to reflect the possibily to set it negative > > > and hence to number CPUs: > > > > > > torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); > > > > > You can set it to a negative as well as to number of CPUs or any other > > number. > > Joel does have a good point, though. Could I interest one of you in > updating the Documentation/admin-guide/kernel-parameters.txt entry for > rcutorture.nfakewriters? The rcutorture.nreaders entry is a decent guide. > OK. Then i misunderstood Joel. It was about updating a documentation about this! Good point :) -- Uladzislau Rezki
On 2/26/2025 9:29 AM, Uladzislau Rezki wrote: > On Tue, Feb 25, 2025 at 04:24:09PM -0500, Joel Fernandes wrote: >> On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: >>> Currently "nfakewriters" parameter can be set to any value but >>> there is no possibility to adjust it automatically based on how >>> many CPUs a system has where a test is run on. >>> >>> To address this, if the "nfakewriters" is set to negative it will >>> be adjusted to num_online_cpus() during torture initialization. >>> >>> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> >>> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> >>> --- >>> kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ >>> 1 file changed, 16 insertions(+), 6 deletions(-) >>> >>> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c >>> index d98b3bd6d91f..f376262532ce 100644 >>> --- a/kernel/rcu/rcutorture.c >>> +++ b/kernel/rcu/rcutorture.c >>> @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); >> >> IMO, this should also be updated to reflect the possibily to set it negative >> and hence to number CPUs: >> >> torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); >> > You can set it to a negative as well as to number of CPUs or any other > number. Sorry I just meant amend the description to something like "Number of RCU fake writer threads (or set to -1 for NR_CPUs)", so user does not have to read code to know that (and update the kernel cmdline params document as well). thanks, - Joel
On Wed, Feb 26, 2025 at 12:49:41PM -0500, Joel Fernandes wrote: > > > On 2/26/2025 9:29 AM, Uladzislau Rezki wrote: > > On Tue, Feb 25, 2025 at 04:24:09PM -0500, Joel Fernandes wrote: > >> On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: > >>> Currently "nfakewriters" parameter can be set to any value but > >>> there is no possibility to adjust it automatically based on how > >>> many CPUs a system has where a test is run on. > >>> > >>> To address this, if the "nfakewriters" is set to negative it will > >>> be adjusted to num_online_cpus() during torture initialization. > >>> > >>> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > >>> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > >>> --- > >>> kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ > >>> 1 file changed, 16 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > >>> index d98b3bd6d91f..f376262532ce 100644 > >>> --- a/kernel/rcu/rcutorture.c > >>> +++ b/kernel/rcu/rcutorture.c > >>> @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); > >> > >> IMO, this should also be updated to reflect the possibily to set it negative > >> and hence to number CPUs: > >> > >> torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); > >> > > You can set it to a negative as well as to number of CPUs or any other > > number. > Sorry I just meant amend the description to something like "Number of RCU fake > writer threads (or set to -1 for NR_CPUs)", so user does not have to read code > to know that (and update the kernel cmdline params document as well). Should we also adjust the string for nreaders? Thanx, Paul
On Wed, Feb 26, 2025 at 10:04:35AM -0800, Paul E. McKenney wrote: > On Wed, Feb 26, 2025 at 12:49:41PM -0500, Joel Fernandes wrote: > > > > > > On 2/26/2025 9:29 AM, Uladzislau Rezki wrote: > > > On Tue, Feb 25, 2025 at 04:24:09PM -0500, Joel Fernandes wrote: > > >> On Tue, Feb 25, 2025 at 12:00:18PM +0100, Uladzislau Rezki (Sony) wrote: > > >>> Currently "nfakewriters" parameter can be set to any value but > > >>> there is no possibility to adjust it automatically based on how > > >>> many CPUs a system has where a test is run on. > > >>> > > >>> To address this, if the "nfakewriters" is set to negative it will > > >>> be adjusted to num_online_cpus() during torture initialization. > > >>> > > >>> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > > >>> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > >>> --- > > >>> kernel/rcu/rcutorture.c | 22 ++++++++++++++++------ > > >>> 1 file changed, 16 insertions(+), 6 deletions(-) > > >>> > > >>> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > >>> index d98b3bd6d91f..f376262532ce 100644 > > >>> --- a/kernel/rcu/rcutorture.c > > >>> +++ b/kernel/rcu/rcutorture.c > > >>> @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); > > >> > > >> IMO, this should also be updated to reflect the possibily to set it negative > > >> and hence to number CPUs: > > >> > > >> torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads"); > > >> > > > You can set it to a negative as well as to number of CPUs or any other > > > number. > > Sorry I just meant amend the description to something like "Number of RCU fake > > writer threads (or set to -1 for NR_CPUs)", so user does not have to read code > > to know that (and update the kernel cmdline params document as well). > > Should we also adjust the string for nreaders? > Makes sense. Both options are adjusted in same way. -- Uladzislau Rezki
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index d98b3bd6d91f..f376262532ce 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -148,6 +148,7 @@ MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, srcu, ...)"); static int nrealnocbers; static int nrealreaders; +static int nrealfakewriters; static struct task_struct *writer_task; static struct task_struct **fakewriter_tasks; static struct task_struct **reader_tasks; @@ -1763,7 +1764,7 @@ rcu_torture_fakewriter(void *arg) do { torture_hrtimeout_jiffies(torture_random(&rand) % 10, &rand); if (cur_ops->cb_barrier != NULL && - torture_random(&rand) % (nfakewriters * 8) == 0) { + torture_random(&rand) % (nrealfakewriters * 8) == 0) { cur_ops->cb_barrier(); } else { switch (synctype[torture_random(&rand) % nsynctypes]) { @@ -2568,7 +2569,7 @@ rcu_torture_print_module_parms(struct rcu_torture_ops *cur_ops, const char *tag) "nocbs_nthreads=%d nocbs_toggle=%d " "test_nmis=%d " "preempt_duration=%d preempt_interval=%d\n", - torture_type, tag, nrealreaders, nfakewriters, + torture_type, tag, nrealreaders, nrealfakewriters, stat_interval, verbose, test_no_idle_hz, shuffle_interval, stutter, irqreader, fqs_duration, fqs_holdoff, fqs_stutter, test_boost, cur_ops->can_boost, @@ -3644,7 +3645,7 @@ rcu_torture_cleanup(void) rcu_torture_reader_mbchk = NULL; if (fakewriter_tasks) { - for (i = 0; i < nfakewriters; i++) + for (i = 0; i < nrealfakewriters; i++) torture_stop_kthread(rcu_torture_fakewriter, fakewriter_tasks[i]); kfree(fakewriter_tasks); @@ -4066,6 +4067,14 @@ rcu_torture_init(void) rcu_torture_init_srcu_lockdep(); + if (nfakewriters >= 0) { + nrealfakewriters = nfakewriters; + } else { + nrealfakewriters = num_online_cpus() - 2 - nfakewriters; + if (nrealfakewriters <= 0) + nrealfakewriters = 1; + } + if (nreaders >= 0) { nrealreaders = nreaders; } else { @@ -4122,8 +4131,9 @@ rcu_torture_init(void) writer_task); if (torture_init_error(firsterr)) goto unwind; - if (nfakewriters > 0) { - fakewriter_tasks = kcalloc(nfakewriters, + + if (nrealfakewriters > 0) { + fakewriter_tasks = kcalloc(nrealfakewriters, sizeof(fakewriter_tasks[0]), GFP_KERNEL); if (fakewriter_tasks == NULL) { @@ -4132,7 +4142,7 @@ rcu_torture_init(void) goto unwind; } } - for (i = 0; i < nfakewriters; i++) { + for (i = 0; i < nrealfakewriters; i++) { firsterr = torture_create_kthread(rcu_torture_fakewriter, NULL, fakewriter_tasks[i]); if (torture_init_error(firsterr))