2023-03-06 12:28:09

by Douglas RAILLARD

[permalink] [raw]
Subject: [PATCH] rcu: Fix rcu_torture_read ftrace event

From: Douglas Raillard <[email protected]>

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 <[email protected]>
---
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)
--
2.25.1



2023-03-06 12:55:05

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH] rcu: Fix rcu_torture_read ftrace event



On 3/6/2023 5:57 PM, Douglas RAILLARD wrote:
> From: Douglas Raillard <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>

-Mukesh

> __field(struct rcu_head *, rhp)
> __field(unsigned long, secs)
> __field(unsigned long, c_old)

2023-03-20 15:27:52

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] rcu: Fix rcu_torture_read ftrace event


[ 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/[email protected]/

( 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 <[email protected]> wrote:

> From: Douglas Raillard <[email protected]>
>
> 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: [email protected]
Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()")

Thanks,

-- Steve

> Signed-off-by: Douglas Raillard <[email protected]>
> ---
> 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)


2023-03-20 17:05:45

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: Fix rcu_torture_read ftrace event

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/[email protected]/
>
> ( 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 <[email protected]> wrote:
>
> > From: Douglas Raillard <[email protected]>
> >
> > 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: [email protected]
> 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 <[email protected]>
> > ---
> > 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)
>

2023-03-20 17:41:41

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] rcu: Fix rcu_torture_read ftrace event

On Mon, 20 Mar 2023 09:58:11 -0700
"Paul E. McKenney" <[email protected]> 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/[email protected]/
> >
> > ( 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 <[email protected]> wrote:
> >
> > > From: Douglas Raillard <[email protected]>
> > >
> > > 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: [email protected]
> > 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) <[email protected]>

-- Steve


> >
> > > Signed-off-by: Douglas Raillard <[email protected]>
> > > ---
> > > 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)
> >


2023-03-20 18:57:17

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: Fix rcu_torture_read ftrace event

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" <[email protected]> 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/[email protected]/
> > >
> > > ( 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 <[email protected]> wrote:
> > >
> > > > From: Douglas Raillard <[email protected]>
> > > >
> > > > 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: [email protected]
> > > 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) <[email protected]>

Fair enough, thank you all!

Thanx, Paul

> -- Steve
>
>
> > >
> > > > Signed-off-by: Douglas Raillard <[email protected]>
> > > > ---
> > > > 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)
> > >
>