| Message ID | 20240312072357.23517-1-qiang.zhang1211@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rcutorture: Make rcutorture support print rcu-tasks gp state | expand |
On Tue, Mar 12, 2024 at 03:23:57PM +0800, Zqiang wrote: > This commit make rcu-tasks related rcutorture test support rcu-tasks > gp state printing when the writer stall occurs or the at the end of > rcutorture test. > > The test is as follows: > [ 3872.548702] tasks-tracing: Start-test grace-period state: g4560 f0x0 > [ 4332.661283] tasks-tracing: End-test grace-period state: g41540 f0x0 total-gps=36980 > > [ 4401.381138] tasks: Start-test grace-period state: g8 f0x0 > [ 4565.619354] tasks: End-test grace-period state: g1732 f0x0 total-gps=1724 > > [ 4589.006917] tasks-rude: Start-test grace-period state: g8 f0x0 > [ 5059.379321] tasks-rude: End-test grace-period state: g8508 f0x0 total-gps=8500 > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> Again, good eyes, and that fix would work. But wouldn't it make more sense to create a new cur_ops function pointer for these functions? That might allow getting rid of the *_FLAVOR checks and values, plus simplify the code a bit. To make the function signatures work out, there would need to be an intermediate SRCU function to supply the right rcu_struct pointer. This would shorten and simplify the code a bit. Or is there some reason that this won't work? Why didn't I do that to start with? Well, the various RCU flavors appeared one at a time over some years... ;-) Thanx, Paul > --- > kernel/rcu/rcu.h | 8 ++++++++ > kernel/rcu/rcutorture.c | 3 +++ > kernel/rcu/tasks.h | 25 +++++++++++++++++++++++++ > 3 files changed, 36 insertions(+) > > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > index 86fce206560e..3353e3697645 100644 > --- a/kernel/rcu/rcu.h > +++ b/kernel/rcu/rcu.h > @@ -556,6 +556,14 @@ static inline unsigned long rcu_get_jiffies_lazy_flush(void) { return 0; } > static inline void rcu_set_jiffies_lazy_flush(unsigned long j) { } > #endif > > +#ifdef CONFIG_TASKS_RCU_GENERIC > +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > + unsigned long *gp_seq); > +#else > +static inline void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > + unsigned long *gp_seq) { } > +#endif > + > #if defined(CONFIG_TREE_RCU) > void rcutorture_get_gp_data(enum rcutorture_type test_type, int *flags, > unsigned long *gp_seq); > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > index dd7d5ba45740..91c03f37fd97 100644 > --- a/kernel/rcu/rcutorture.c > +++ b/kernel/rcu/rcutorture.c > @@ -2267,6 +2267,7 @@ rcu_torture_stats_print(void) > &flags, &gp_seq); > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, > &flags, &gp_seq); > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > wtp = READ_ONCE(writer_task); > pr_alert("??? Writer stall state %s(%d) g%lu f%#x ->state %#x cpu %d\n", > rcu_torture_writer_state_getname(), > @@ -3391,6 +3392,7 @@ rcu_torture_cleanup(void) > > rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > pr_alert("%s: End-test grace-period state: g%ld f%#x total-gps=%ld\n", > cur_ops->name, (long)gp_seq, flags, > rcutorture_seq_diff(gp_seq, start_gp_seq)); > @@ -3763,6 +3765,7 @@ rcu_torture_init(void) > rcu_torture_print_module_parms(cur_ops, "Start of test"); > rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > start_gp_seq = gp_seq; > pr_alert("%s: Start-test grace-period state: g%ld f%#x\n", > cur_ops->name, (long)gp_seq, flags); > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h > index e83adcdb49b5..b1254cf3c210 100644 > --- a/kernel/rcu/tasks.h > +++ b/kernel/rcu/tasks.h > @@ -2149,6 +2149,31 @@ late_initcall(rcu_tasks_verify_schedule_work); > static void rcu_tasks_initiate_self_tests(void) { } > #endif /* #else #ifdef CONFIG_PROVE_RCU */ > > +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > + unsigned long *gp_seq) > +{ > + switch (test_type) { > + case RCU_TASKS_FLAVOR: > +#ifdef CONFIG_TASKS_RCU > + *gp_seq = rcu_seq_current(&rcu_tasks.tasks_gp_seq); > +#endif > + break; > + case RCU_TASKS_RUDE_FLAVOR: > +#ifdef CONFIG_TASKS_RUDE_RCU > + *gp_seq = rcu_seq_current(&rcu_tasks_rude.tasks_gp_seq); > +#endif > + break; > + case RCU_TASKS_TRACING_FLAVOR: > +#ifdef CONFIG_TASKS_TRACE_RCU > + *gp_seq = rcu_seq_current(&rcu_tasks_trace.tasks_gp_seq); > +#endif > + break; > + default: > + break; > + } > +} > +EXPORT_SYMBOL_GPL(rcutaskstorture_get_gp_data); > + > void __init tasks_cblist_init_generic(void) > { > lockdep_assert_irqs_disabled(); > -- > 2.17.1 >
> > On Tue, Mar 12, 2024 at 03:23:57PM +0800, Zqiang wrote: > > This commit make rcu-tasks related rcutorture test support rcu-tasks > > gp state printing when the writer stall occurs or the at the end of > > rcutorture test. > > > > The test is as follows: > > [ 3872.548702] tasks-tracing: Start-test grace-period state: g4560 f0x0 > > [ 4332.661283] tasks-tracing: End-test grace-period state: g41540 f0x0 total-gps=36980 > > > > [ 4401.381138] tasks: Start-test grace-period state: g8 f0x0 > > [ 4565.619354] tasks: End-test grace-period state: g1732 f0x0 total-gps=1724 > > > > [ 4589.006917] tasks-rude: Start-test grace-period state: g8 f0x0 > > [ 5059.379321] tasks-rude: End-test grace-period state: g8508 f0x0 total-gps=8500 > > > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > > Again, good eyes, and that fix would work. But wouldn't it make more > sense to create a new cur_ops function pointer for these functions? > That might allow getting rid of the *_FLAVOR checks and values, plus > simplify the code a bit. To make the function signatures work out, > there would need to be an intermediate SRCU function to supply the right > rcu_struct pointer. > > This would shorten and simplify the code a bit. > > Or is there some reason that this won't work? Hi, Paul Thanks for your suggestion, I will resend with reference to the suggestions above. Thanks Zqiang > > Why didn't I do that to start with? Well, the various RCU flavors > appeared one at a time over some years... ;-) > > Thanx, Paul > > > --- > > kernel/rcu/rcu.h | 8 ++++++++ > > kernel/rcu/rcutorture.c | 3 +++ > > kernel/rcu/tasks.h | 25 +++++++++++++++++++++++++ > > 3 files changed, 36 insertions(+) > > > > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > > index 86fce206560e..3353e3697645 100644 > > --- a/kernel/rcu/rcu.h > > +++ b/kernel/rcu/rcu.h > > @@ -556,6 +556,14 @@ static inline unsigned long rcu_get_jiffies_lazy_flush(void) { return 0; } > > static inline void rcu_set_jiffies_lazy_flush(unsigned long j) { } > > #endif > > > > +#ifdef CONFIG_TASKS_RCU_GENERIC > > +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > + unsigned long *gp_seq); > > +#else > > +static inline void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > + unsigned long *gp_seq) { } > > +#endif > > + > > #if defined(CONFIG_TREE_RCU) > > void rcutorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > unsigned long *gp_seq); > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > index dd7d5ba45740..91c03f37fd97 100644 > > --- a/kernel/rcu/rcutorture.c > > +++ b/kernel/rcu/rcutorture.c > > @@ -2267,6 +2267,7 @@ rcu_torture_stats_print(void) > > &flags, &gp_seq); > > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, > > &flags, &gp_seq); > > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > wtp = READ_ONCE(writer_task); > > pr_alert("??? Writer stall state %s(%d) g%lu f%#x ->state %#x cpu %d\n", > > rcu_torture_writer_state_getname(), > > @@ -3391,6 +3392,7 @@ rcu_torture_cleanup(void) > > > > rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); > > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > pr_alert("%s: End-test grace-period state: g%ld f%#x total-gps=%ld\n", > > cur_ops->name, (long)gp_seq, flags, > > rcutorture_seq_diff(gp_seq, start_gp_seq)); > > @@ -3763,6 +3765,7 @@ rcu_torture_init(void) > > rcu_torture_print_module_parms(cur_ops, "Start of test"); > > rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); > > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > start_gp_seq = gp_seq; > > pr_alert("%s: Start-test grace-period state: g%ld f%#x\n", > > cur_ops->name, (long)gp_seq, flags); > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h > > index e83adcdb49b5..b1254cf3c210 100644 > > --- a/kernel/rcu/tasks.h > > +++ b/kernel/rcu/tasks.h > > @@ -2149,6 +2149,31 @@ late_initcall(rcu_tasks_verify_schedule_work); > > static void rcu_tasks_initiate_self_tests(void) { } > > #endif /* #else #ifdef CONFIG_PROVE_RCU */ > > > > +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > + unsigned long *gp_seq) > > +{ > > + switch (test_type) { > > + case RCU_TASKS_FLAVOR: > > +#ifdef CONFIG_TASKS_RCU > > + *gp_seq = rcu_seq_current(&rcu_tasks.tasks_gp_seq); > > +#endif > > + break; > > + case RCU_TASKS_RUDE_FLAVOR: > > +#ifdef CONFIG_TASKS_RUDE_RCU > > + *gp_seq = rcu_seq_current(&rcu_tasks_rude.tasks_gp_seq); > > +#endif > > + break; > > + case RCU_TASKS_TRACING_FLAVOR: > > +#ifdef CONFIG_TASKS_TRACE_RCU > > + *gp_seq = rcu_seq_current(&rcu_tasks_trace.tasks_gp_seq); > > +#endif > > + break; > > + default: > > + break; > > + } > > +} > > +EXPORT_SYMBOL_GPL(rcutaskstorture_get_gp_data); > > + > > void __init tasks_cblist_init_generic(void) > > { > > lockdep_assert_irqs_disabled(); > > -- > > 2.17.1 > >
On Sun, Mar 17, 2024 at 03:16:43PM +0800, Z qiang wrote: > > > > On Tue, Mar 12, 2024 at 03:23:57PM +0800, Zqiang wrote: > > > This commit make rcu-tasks related rcutorture test support rcu-tasks > > > gp state printing when the writer stall occurs or the at the end of > > > rcutorture test. > > > > > > The test is as follows: > > > [ 3872.548702] tasks-tracing: Start-test grace-period state: g4560 f0x0 > > > [ 4332.661283] tasks-tracing: End-test grace-period state: g41540 f0x0 total-gps=36980 > > > > > > [ 4401.381138] tasks: Start-test grace-period state: g8 f0x0 > > > [ 4565.619354] tasks: End-test grace-period state: g1732 f0x0 total-gps=1724 > > > > > > [ 4589.006917] tasks-rude: Start-test grace-period state: g8 f0x0 > > > [ 5059.379321] tasks-rude: End-test grace-period state: g8508 f0x0 total-gps=8500 > > > > > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > > > > Again, good eyes, and that fix would work. But wouldn't it make more > > sense to create a new cur_ops function pointer for these functions? > > That might allow getting rid of the *_FLAVOR checks and values, plus > > simplify the code a bit. To make the function signatures work out, > > there would need to be an intermediate SRCU function to supply the right > > rcu_struct pointer. > > > > This would shorten and simplify the code a bit. > > > > Or is there some reason that this won't work? > > Hi, Paul > > Thanks for your suggestion, I will resend with reference to the > suggestions above. Very good, looking forward to seeing it! Thanx, Paul > Thanks > Zqiang > > > > > > Why didn't I do that to start with? Well, the various RCU flavors > > appeared one at a time over some years... ;-) > > > > Thanx, Paul > > > > > --- > > > kernel/rcu/rcu.h | 8 ++++++++ > > > kernel/rcu/rcutorture.c | 3 +++ > > > kernel/rcu/tasks.h | 25 +++++++++++++++++++++++++ > > > 3 files changed, 36 insertions(+) > > > > > > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > > > index 86fce206560e..3353e3697645 100644 > > > --- a/kernel/rcu/rcu.h > > > +++ b/kernel/rcu/rcu.h > > > @@ -556,6 +556,14 @@ static inline unsigned long rcu_get_jiffies_lazy_flush(void) { return 0; } > > > static inline void rcu_set_jiffies_lazy_flush(unsigned long j) { } > > > #endif > > > > > > +#ifdef CONFIG_TASKS_RCU_GENERIC > > > +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > > + unsigned long *gp_seq); > > > +#else > > > +static inline void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > > + unsigned long *gp_seq) { } > > > +#endif > > > + > > > #if defined(CONFIG_TREE_RCU) > > > void rcutorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > > unsigned long *gp_seq); > > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > > index dd7d5ba45740..91c03f37fd97 100644 > > > --- a/kernel/rcu/rcutorture.c > > > +++ b/kernel/rcu/rcutorture.c > > > @@ -2267,6 +2267,7 @@ rcu_torture_stats_print(void) > > > &flags, &gp_seq); > > > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, > > > &flags, &gp_seq); > > > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > > wtp = READ_ONCE(writer_task); > > > pr_alert("??? Writer stall state %s(%d) g%lu f%#x ->state %#x cpu %d\n", > > > rcu_torture_writer_state_getname(), > > > @@ -3391,6 +3392,7 @@ rcu_torture_cleanup(void) > > > > > > rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); > > > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > > pr_alert("%s: End-test grace-period state: g%ld f%#x total-gps=%ld\n", > > > cur_ops->name, (long)gp_seq, flags, > > > rcutorture_seq_diff(gp_seq, start_gp_seq)); > > > @@ -3763,6 +3765,7 @@ rcu_torture_init(void) > > > rcu_torture_print_module_parms(cur_ops, "Start of test"); > > > rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > > srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); > > > + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); > > > start_gp_seq = gp_seq; > > > pr_alert("%s: Start-test grace-period state: g%ld f%#x\n", > > > cur_ops->name, (long)gp_seq, flags); > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h > > > index e83adcdb49b5..b1254cf3c210 100644 > > > --- a/kernel/rcu/tasks.h > > > +++ b/kernel/rcu/tasks.h > > > @@ -2149,6 +2149,31 @@ late_initcall(rcu_tasks_verify_schedule_work); > > > static void rcu_tasks_initiate_self_tests(void) { } > > > #endif /* #else #ifdef CONFIG_PROVE_RCU */ > > > > > > +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, > > > + unsigned long *gp_seq) > > > +{ > > > + switch (test_type) { > > > + case RCU_TASKS_FLAVOR: > > > +#ifdef CONFIG_TASKS_RCU > > > + *gp_seq = rcu_seq_current(&rcu_tasks.tasks_gp_seq); > > > +#endif > > > + break; > > > + case RCU_TASKS_RUDE_FLAVOR: > > > +#ifdef CONFIG_TASKS_RUDE_RCU > > > + *gp_seq = rcu_seq_current(&rcu_tasks_rude.tasks_gp_seq); > > > +#endif > > > + break; > > > + case RCU_TASKS_TRACING_FLAVOR: > > > +#ifdef CONFIG_TASKS_TRACE_RCU > > > + *gp_seq = rcu_seq_current(&rcu_tasks_trace.tasks_gp_seq); > > > +#endif > > > + break; > > > + default: > > > + break; > > > + } > > > +} > > > +EXPORT_SYMBOL_GPL(rcutaskstorture_get_gp_data); > > > + > > > void __init tasks_cblist_init_generic(void) > > > { > > > lockdep_assert_irqs_disabled(); > > > -- > > > 2.17.1 > > >
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h index 86fce206560e..3353e3697645 100644 --- a/kernel/rcu/rcu.h +++ b/kernel/rcu/rcu.h @@ -556,6 +556,14 @@ static inline unsigned long rcu_get_jiffies_lazy_flush(void) { return 0; } static inline void rcu_set_jiffies_lazy_flush(unsigned long j) { } #endif +#ifdef CONFIG_TASKS_RCU_GENERIC +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, + unsigned long *gp_seq); +#else +static inline void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, + unsigned long *gp_seq) { } +#endif + #if defined(CONFIG_TREE_RCU) void rcutorture_get_gp_data(enum rcutorture_type test_type, int *flags, unsigned long *gp_seq); diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index dd7d5ba45740..91c03f37fd97 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -2267,6 +2267,7 @@ rcu_torture_stats_print(void) &flags, &gp_seq); srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); wtp = READ_ONCE(writer_task); pr_alert("??? Writer stall state %s(%d) g%lu f%#x ->state %#x cpu %d\n", rcu_torture_writer_state_getname(), @@ -3391,6 +3392,7 @@ rcu_torture_cleanup(void) rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); pr_alert("%s: End-test grace-period state: g%ld f%#x total-gps=%ld\n", cur_ops->name, (long)gp_seq, flags, rcutorture_seq_diff(gp_seq, start_gp_seq)); @@ -3763,6 +3765,7 @@ rcu_torture_init(void) rcu_torture_print_module_parms(cur_ops, "Start of test"); rcutorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); srcutorture_get_gp_data(cur_ops->ttype, srcu_ctlp, &flags, &gp_seq); + rcutaskstorture_get_gp_data(cur_ops->ttype, &flags, &gp_seq); start_gp_seq = gp_seq; pr_alert("%s: Start-test grace-period state: g%ld f%#x\n", cur_ops->name, (long)gp_seq, flags); diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h index e83adcdb49b5..b1254cf3c210 100644 --- a/kernel/rcu/tasks.h +++ b/kernel/rcu/tasks.h @@ -2149,6 +2149,31 @@ late_initcall(rcu_tasks_verify_schedule_work); static void rcu_tasks_initiate_self_tests(void) { } #endif /* #else #ifdef CONFIG_PROVE_RCU */ +void rcutaskstorture_get_gp_data(enum rcutorture_type test_type, int *flags, + unsigned long *gp_seq) +{ + switch (test_type) { + case RCU_TASKS_FLAVOR: +#ifdef CONFIG_TASKS_RCU + *gp_seq = rcu_seq_current(&rcu_tasks.tasks_gp_seq); +#endif + break; + case RCU_TASKS_RUDE_FLAVOR: +#ifdef CONFIG_TASKS_RUDE_RCU + *gp_seq = rcu_seq_current(&rcu_tasks_rude.tasks_gp_seq); +#endif + break; + case RCU_TASKS_TRACING_FLAVOR: +#ifdef CONFIG_TASKS_TRACE_RCU + *gp_seq = rcu_seq_current(&rcu_tasks_trace.tasks_gp_seq); +#endif + break; + default: + break; + } +} +EXPORT_SYMBOL_GPL(rcutaskstorture_get_gp_data); + void __init tasks_cblist_init_generic(void) { lockdep_assert_irqs_disabled();
This commit make rcu-tasks related rcutorture test support rcu-tasks gp state printing when the writer stall occurs or the at the end of rcutorture test. The test is as follows: [ 3872.548702] tasks-tracing: Start-test grace-period state: g4560 f0x0 [ 4332.661283] tasks-tracing: End-test grace-period state: g41540 f0x0 total-gps=36980 [ 4401.381138] tasks: Start-test grace-period state: g8 f0x0 [ 4565.619354] tasks: End-test grace-period state: g1732 f0x0 total-gps=1724 [ 4589.006917] tasks-rude: Start-test grace-period state: g8 f0x0 [ 5059.379321] tasks-rude: End-test grace-period state: g8508 f0x0 total-gps=8500 Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> --- kernel/rcu/rcu.h | 8 ++++++++ kernel/rcu/rcutorture.c | 3 +++ kernel/rcu/tasks.h | 25 +++++++++++++++++++++++++ 3 files changed, 36 insertions(+)