Re: [PATCH v5] tracepoint: Do not fail unregistering a probe due to memory failure

[Alexey Kardashevskiy]

On 02/02/2021 04:10, Steven Rostedt wrote:
> On Mon, 1 Feb 2021 12:18:34 +1100
> Alexey Kardashevskiy <aik@xxxxxxxxx> wrote:
>
> > > Just curious, does the following patch fix it for v5?
> >
> >
> > Yes it does!
>
> Thanks for verifying.
>
> > > -- Steve
> > >
> > > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > > index 7261fa0f5e3c..cf3a6d104fdb 100644
> > > --- a/kernel/tracepoint.c
> > > +++ b/kernel/tracepoint.c
> > > @@ -306,6 +306,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> > >    			tp->unregfunc();
> > >    
> > >    		static_key_disable(&tp->key);
> > > +		tracepoint_synchronize_unregister();
> > >    		rcu_assign_pointer(tp->funcs, tp_funcs);
> > >    	} else {
> > >    		rcu_assign_pointer(tp->funcs, tp_funcs);
> > >    
>
> OK, since it would be expensive to do a synchronization on every removal
> like that, but the tp->funcs should not be reset.
>
> It appears that your check is still required, since the iterator has been
> added.
>
> The quick fix is the check you gave.
>
> But I think we could optimize this (not having to dereference the array
> twice, and do the check twice) by making the iterator part of the tp_funcs
> array, and having the rest of the array as its argument. But that can be a
> separate update.
>
> The check you added should be a patch and marked for stable. Care to send
> it, and mark it for stable as well as:
>
> Fixes: d25e37d89dd2f ("tracepoint: Optimize using static_call()")
>
> Thanks!


Posted as "[PATCH kernel] tracepoint: Fix race between tracing and 
removing tracepoint", hopefully I got it right. Thanks,



--
Alexey