| Message ID | 20230306122744.236790-1-douglas.raillard@arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d18a04157fc171fd48075e3dc96471bd3b87f0dd |
| Headers | show |
| Series | rcu: Fix rcu_torture_read ftrace event | expand |
On 3/6/2023 5:57 PM, Douglas RAILLARD wrote: > From: Douglas Raillard <douglas.raillard@arm.com> > > Fix the rcutorturename field so that its size is correctly reported in > the text format embedded in trace.dat files. As it stands, it is > reported as being of size 1: > > field:char rcutorturename[8]; offset:8; size:1; signed:0; > > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com> > --- > include/trace/events/rcu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h > index 90b2fb0292cb..012fa0d171b2 100644 > --- a/include/trace/events/rcu.h > +++ b/include/trace/events/rcu.h > @@ -768,7 +768,7 @@ TRACE_EVENT_RCU(rcu_torture_read, > TP_ARGS(rcutorturename, rhp, secs, c_old, c), > > TP_STRUCT__entry( > - __field(char, rcutorturename[RCUTORTURENAME_LEN]) > + __array(char, rcutorturename, RCUTORTURENAME_LEN) Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com> -Mukesh > __field(struct rcu_head *, rhp) > __field(unsigned long, secs) > __field(unsigned long, c_old)
[ Wondering why this didn't get picked up in v6.3-rc3, I see that the maintainers of RCU were not Cc'd :-( ] This is a bug that will cause unwanted results. I have a patch that will not let the kernel build when code like this is added. https://patchwork.kernel.org/project/linux-trace-kernel/patch/20230309221302.642e82d9@gandalf.local.home/ ( The kernel robot even failed when applying the above patch, because it caught the code that this patch fixes ) On Mon, 6 Mar 2023 12:27:43 +0000 Douglas RAILLARD <douglas.raillard@arm.com> wrote: > From: Douglas Raillard <douglas.raillard@arm.com> > > Fix the rcutorturename field so that its size is correctly reported in > the text format embedded in trace.dat files. As it stands, it is > reported as being of size 1: And that the offsets of the following fields will be incorrect as well. > > field:char rcutorturename[8]; offset:8; size:1; signed:0; > Please add: Cc: stable@vger.kernel.org Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()") Thanks, -- Steve > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com> > --- > include/trace/events/rcu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h > index 90b2fb0292cb..012fa0d171b2 100644 > --- a/include/trace/events/rcu.h > +++ b/include/trace/events/rcu.h > @@ -768,7 +768,7 @@ TRACE_EVENT_RCU(rcu_torture_read, > TP_ARGS(rcutorturename, rhp, secs, c_old, c), > > TP_STRUCT__entry( > - __field(char, rcutorturename[RCUTORTURENAME_LEN]) > + __array(char, rcutorturename, RCUTORTURENAME_LEN) > __field(struct rcu_head *, rhp) > __field(unsigned long, secs) > __field(unsigned long, c_old)
On Mon, Mar 20, 2023 at 11:20:15AM -0400, Steven Rostedt wrote: > > [ Wondering why this didn't get picked up in v6.3-rc3, I see that the > maintainers of RCU were not Cc'd :-( ] > > This is a bug that will cause unwanted results. I have a patch that will not > let the kernel build when code like this is added. > > https://patchwork.kernel.org/project/linux-trace-kernel/patch/20230309221302.642e82d9@gandalf.local.home/ > > ( The kernel robot even failed when applying the above patch, because it > caught the code that this patch fixes ) > > On Mon, 6 Mar 2023 12:27:43 +0000 > Douglas RAILLARD <douglas.raillard@arm.com> wrote: > > > From: Douglas Raillard <douglas.raillard@arm.com> > > > > Fix the rcutorturename field so that its size is correctly reported in > > the text format embedded in trace.dat files. As it stands, it is > > reported as being of size 1: > > And that the offsets of the following fields will be incorrect as well. > > > > > field:char rcutorturename[8]; offset:8; size:1; signed:0; > > > > Please add: > > Cc: stable@vger.kernel.org > Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()") Thank you, Steve! With those fixes, and with an ack or better from Steve, I will be happy to pull this in to -rcu. How urgent is this? The default destination would be the v6.5 merge window (not the upcoming one, but the one after that), so if you need it sooner, please let me know. Thanx, Paul > Thanks, > > -- Steve > > > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com> > > --- > > include/trace/events/rcu.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h > > index 90b2fb0292cb..012fa0d171b2 100644 > > --- a/include/trace/events/rcu.h > > +++ b/include/trace/events/rcu.h > > @@ -768,7 +768,7 @@ TRACE_EVENT_RCU(rcu_torture_read, > > TP_ARGS(rcutorturename, rhp, secs, c_old, c), > > > > TP_STRUCT__entry( > > - __field(char, rcutorturename[RCUTORTURENAME_LEN]) > > + __array(char, rcutorturename, RCUTORTURENAME_LEN) > > __field(struct rcu_head *, rhp) > > __field(unsigned long, secs) > > __field(unsigned long, c_old) >
On Mon, 20 Mar 2023 09:58:11 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Mon, Mar 20, 2023 at 11:20:15AM -0400, Steven Rostedt wrote: > > > > [ Wondering why this didn't get picked up in v6.3-rc3, I see that the > > maintainers of RCU were not Cc'd :-( ] > > > > This is a bug that will cause unwanted results. I have a patch that will not > > let the kernel build when code like this is added. > > > > https://patchwork.kernel.org/project/linux-trace-kernel/patch/20230309221302.642e82d9@gandalf.local.home/ > > > > ( The kernel robot even failed when applying the above patch, because it > > caught the code that this patch fixes ) > > > > On Mon, 6 Mar 2023 12:27:43 +0000 > > Douglas RAILLARD <douglas.raillard@arm.com> wrote: > > > > > From: Douglas Raillard <douglas.raillard@arm.com> > > > > > > Fix the rcutorturename field so that its size is correctly reported in > > > the text format embedded in trace.dat files. As it stands, it is > > > reported as being of size 1: > > > > And that the offsets of the following fields will be incorrect as well. > > > > > > > > field:char rcutorturename[8]; offset:8; size:1; signed:0; > > > > > > > Please add: > > > > Cc: stable@vger.kernel.org > > Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()") > > Thank you, Steve! > > With those fixes, and with an ack or better from Steve, I will be happy > to pull this in to -rcu. How urgent is this? The default destination > would be the v6.5 merge window (not the upcoming one, but the one after > that), so if you need it sooner, please let me know. I would like my patch to get in this release, so if you can get it into this release too (before the next merge window) that would be great. This is a real bug. User space tooling can not parse this trace event (when it use to, so it is a regression), and my patch that prevents other trace events from making the same mistake will make this code as is fail the build. For this patch: Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org> -- Steve > > > > > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com> > > > --- > > > include/trace/events/rcu.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h > > > index 90b2fb0292cb..012fa0d171b2 100644 > > > --- a/include/trace/events/rcu.h > > > +++ b/include/trace/events/rcu.h > > > @@ -768,7 +768,7 @@ TRACE_EVENT_RCU(rcu_torture_read, > > > TP_ARGS(rcutorturename, rhp, secs, c_old, c), > > > > > > TP_STRUCT__entry( > > > - __field(char, rcutorturename[RCUTORTURENAME_LEN]) > > > + __array(char, rcutorturename, RCUTORTURENAME_LEN) > > > __field(struct rcu_head *, rhp) > > > __field(unsigned long, secs) > > > __field(unsigned long, c_old) > >
On Mon, Mar 20, 2023 at 01:36:50PM -0400, Steven Rostedt wrote: > On Mon, 20 Mar 2023 09:58:11 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > On Mon, Mar 20, 2023 at 11:20:15AM -0400, Steven Rostedt wrote: > > > > > > [ Wondering why this didn't get picked up in v6.3-rc3, I see that the > > > maintainers of RCU were not Cc'd :-( ] > > > > > > This is a bug that will cause unwanted results. I have a patch that will not > > > let the kernel build when code like this is added. > > > > > > https://patchwork.kernel.org/project/linux-trace-kernel/patch/20230309221302.642e82d9@gandalf.local.home/ > > > > > > ( The kernel robot even failed when applying the above patch, because it > > > caught the code that this patch fixes ) > > > > > > On Mon, 6 Mar 2023 12:27:43 +0000 > > > Douglas RAILLARD <douglas.raillard@arm.com> wrote: > > > > > > > From: Douglas Raillard <douglas.raillard@arm.com> > > > > > > > > Fix the rcutorturename field so that its size is correctly reported in > > > > the text format embedded in trace.dat files. As it stands, it is > > > > reported as being of size 1: > > > > > > And that the offsets of the following fields will be incorrect as well. > > > > > > > > > > > field:char rcutorturename[8]; offset:8; size:1; signed:0; > > > > > > > > > > Please add: > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()") > > > > Thank you, Steve! > > > > With those fixes, and with an ack or better from Steve, I will be happy > > to pull this in to -rcu. How urgent is this? The default destination > > would be the v6.5 merge window (not the upcoming one, but the one after > > that), so if you need it sooner, please let me know. > > I would like my patch to get in this release, so if you can get it into > this release too (before the next merge window) that would be great. This is > a real bug. User space tooling can not parse this trace event (when it use > to, so it is a regression), and my patch that prevents other trace events > from making the same mistake will make this code as is fail the build. > > For this patch: > > Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org> Fair enough, thank you all! Thanx, Paul > -- Steve > > > > > > > > > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com> > > > > --- > > > > include/trace/events/rcu.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h > > > > index 90b2fb0292cb..012fa0d171b2 100644 > > > > --- a/include/trace/events/rcu.h > > > > +++ b/include/trace/events/rcu.h > > > > @@ -768,7 +768,7 @@ TRACE_EVENT_RCU(rcu_torture_read, > > > > TP_ARGS(rcutorturename, rhp, secs, c_old, c), > > > > > > > > TP_STRUCT__entry( > > > > - __field(char, rcutorturename[RCUTORTURENAME_LEN]) > > > > + __array(char, rcutorturename, RCUTORTURENAME_LEN) > > > > __field(struct rcu_head *, rhp) > > > > __field(unsigned long, secs) > > > > __field(unsigned long, c_old) > > > >
diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h index 90b2fb0292cb..012fa0d171b2 100644 --- a/include/trace/events/rcu.h +++ b/include/trace/events/rcu.h @@ -768,7 +768,7 @@ TRACE_EVENT_RCU(rcu_torture_read, TP_ARGS(rcutorturename, rhp, secs, c_old, c), TP_STRUCT__entry( - __field(char, rcutorturename[RCUTORTURENAME_LEN]) + __array(char, rcutorturename, RCUTORTURENAME_LEN) __field(struct rcu_head *, rhp) __field(unsigned long, secs) __field(unsigned long, c_old)