Re: [External] Re: [PATCH 1/2] ftrace: clear module from hash of all ftrace ops

[Steven Rostedt]

On Wed, 29 Jul 2020 00:59:33 +0800
Chengming Zhou <zhouchengming@xxxxxxxxxxxxx> wrote:


> > i.e.
> >
> >   # echo some_module_function > set_ftrace_filter
> >   # rmmod module_with_that_function
> >   # insmod module_with_same_address_of_function
> >   # echo function > current_tracer
> >
> > Now the tr->ops->hash would still have the function of the original
> > module.  
> 
> I thought all ftrace_ops has non empty func_hash are on the ftrace 
> global list...

Nope, the two are disjoint.

> 
> Well, so I just leave this function unmodified.
> 
> Just call that new function register_ftrace_ops_hash() from 
> ftrace_release_mod.

I would say to have anything that uses one of the
ftrace_set_filter/notrace* functions, to also register itself for
module removal.

	register_ftrace_mod_removal(struct ftrace_ops *ops);

and then also have a unregister_ftrace_mod_removal() as there needs to
be a way to remove it from the list before the ops gets freed.

Then these functions would add the ops to a list, and this list is
traversed to remove modules. The trace_arrays can register their ops
too, so you can update that function.

-- Steve


> 
> Thanks!
> 
> >
> > Either have all owners of ftrace_ops handle this case, or add a helper
> > function to handle it for them. But using ftarce_ops_list is the wrong
> > place to do it.
> >
> > -- Steve
> >
> >  
> >> +	mutex_lock(&ftrace_lock);
> >> +
> >> +	do_for_each_ftrace_op(op, ftrace_ops_list) {
> >> +		if (!op->func_hash)
> >>   			continue;
> >> -		mutex_lock(&tr->ops->func_hash->regex_lock);
> >> -		clear_mod_from_hash(pg, tr->ops->func_hash->filter_hash);
> >> -		clear_mod_from_hash(pg, tr->ops->func_hash->notrace_hash);
> >> -		mutex_unlock(&tr->ops->func_hash->regex_lock);
> >> -	}
> >> -	mutex_unlock(&trace_types_lock);
> >> +		mutex_lock(&op->func_hash->regex_lock);
> >> +		clear_mod_from_hash(pg, op->func_hash->filter_hash);
> >> +		clear_mod_from_hash(pg, op->func_hash->notrace_hash);
> >> +		mutex_unlock(&op->func_hash->regex_lock);
> >> +	} while_for_each_ftrace_op(op);
> >> +
> >> +	mutex_unlock(&ftrace_lock);
> >>   }
> >>   
> >>   static void ftrace_free_mod_map(struct rcu_head *rcu)