| Message ID | 20210805132717.23813-2-mathieu.desnoyers@efficios.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tracepoint static call fixes | expand |
On Thu, 5 Aug 2021 09:27:15 -0400 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > On transition from 2->1 callees, we should be comparing .data rather > than .func, because the same callback can be registered twice with > different data, and what we care about here is that the data of array > element 0 is unchanged to skip rcu sync. > > Link: https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ FYI, You only need to show one Fixes. > Fixes: d25e37d89dd2 ("tracepoint: Optimize using static_call()") The above is fixed by the one below. Which means all the stable kernels that have the above, will also have the below, and thus the above is just redundant. > Fixes: 547305a64632 ("tracepoint: Fix out of sync data passing by static caller") The above is what the patch actually fixes. > Fixes: 352384d5c84e ("tracepoints: Update static_call before tp_funcs when adding a tracepoint") How does this patch fix the above? Perhaps the above did not go enough to fix the issue, but it's unrelated. I'll remove the first and last Fixes tag. > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > Cc: Stefan Metzmacher <metze@samba.org> > Cc: <stable@vger.kernel.org> # 5.10+ The "# 5.10+" is now obsolete, and not needed. The Fixes tag is used to determine where this gets backported to. Other than that. This patch looks good. -- Steve > --- > kernel/tracepoint.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c > index fc32821f8240..133b6454b287 100644 > --- a/kernel/tracepoint.c > +++ b/kernel/tracepoint.c > @@ -338,7 +338,7 @@ static int tracepoint_remove_func(struct tracepoint *tp, > } else { > rcu_assign_pointer(tp->funcs, tp_funcs); > tracepoint_update_call(tp, tp_funcs, > - tp_funcs[0].func != old[0].func); > + tp_funcs[0].data != old[0].data); > } > release_probes(old); > return 0;
----- On Aug 5, 2021, at 1:07 PM, rostedt rostedt@goodmis.org wrote: > On Thu, 5 Aug 2021 09:27:15 -0400 > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > >> On transition from 2->1 callees, we should be comparing .data rather >> than .func, because the same callback can be registered twice with >> different data, and what we care about here is that the data of array >> element 0 is unchanged to skip rcu sync. >> >> Link: >> https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ > > FYI, You only need to show one Fixes. > >> Fixes: d25e37d89dd2 ("tracepoint: Optimize using static_call()") > > The above is fixed by the one below. Which means all the stable kernels > that have the above, will also have the below, and thus the above is > just redundant. > >> Fixes: 547305a64632 ("tracepoint: Fix out of sync data passing by static >> caller") > > The above is what the patch actually fixes. > >> Fixes: 352384d5c84e ("tracepoints: Update static_call before tp_funcs when >> adding a tracepoint") > > How does this patch fix the above? Perhaps the above did not go enough > to fix the issue, but it's unrelated. OK > > I'll remove the first and last Fixes tag. OK > >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> Cc: Steven Rostedt <rostedt@goodmis.org> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: "Paul E. McKenney" <paulmck@kernel.org> >> Cc: Stefan Metzmacher <metze@samba.org> >> Cc: <stable@vger.kernel.org> # 5.10+ > > The "# 5.10+" is now obsolete, and not needed. The Fixes tag is used to > determine where this gets backported to. > > Other than that. This patch looks good. Great, thanks! Mathieu > > -- Steve > >> --- >> kernel/tracepoint.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c >> index fc32821f8240..133b6454b287 100644 >> --- a/kernel/tracepoint.c >> +++ b/kernel/tracepoint.c >> @@ -338,7 +338,7 @@ static int tracepoint_remove_func(struct tracepoint *tp, >> } else { >> rcu_assign_pointer(tp->funcs, tp_funcs); >> tracepoint_update_call(tp, tp_funcs, >> - tp_funcs[0].func != old[0].func); >> + tp_funcs[0].data != old[0].data); >> } >> release_probes(old); > > return 0;
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c index fc32821f8240..133b6454b287 100644 --- a/kernel/tracepoint.c +++ b/kernel/tracepoint.c @@ -338,7 +338,7 @@ static int tracepoint_remove_func(struct tracepoint *tp, } else { rcu_assign_pointer(tp->funcs, tp_funcs); tracepoint_update_call(tp, tp_funcs, - tp_funcs[0].func != old[0].func); + tp_funcs[0].data != old[0].data); } release_probes(old); return 0;
On transition from 2->1 callees, we should be comparing .data rather than .func, because the same callback can be registered twice with different data, and what we care about here is that the data of array element 0 is unchanged to skip rcu sync. Link: https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ Fixes: d25e37d89dd2 ("tracepoint: Optimize using static_call()") Fixes: 547305a64632 ("tracepoint: Fix out of sync data passing by static caller") Fixes: 352384d5c84e ("tracepoints: Update static_call before tp_funcs when adding a tracepoint") Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: "Paul E. McKenney" <paulmck@kernel.org> Cc: Stefan Metzmacher <metze@samba.org> Cc: <stable@vger.kernel.org> # 5.10+ --- kernel/tracepoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)