Re: [PATCH 23/32] tracing: Add 'onmatch' hist trigger action support

[Namhyung Kim]

On Mon, Jun 26, 2017 at 05:49:24PM -0500, Tom Zanussi wrote:
> Add an 'onmatch(matching.event).<synthetic_event_name>(param list)'
> hist trigger action which is invoked with the set of variables or
> event fields named in the 'param list'.  The result is the generation
> of a synthetic event that consists of the values contained in those
> variables and/or fields at the time the invoking event was hit.
> 
> As an example the below defines a simple synthetic event using a
> variable defined on the sched_wakeup_new event, and shows the event
> definition with unresolved fields, since the sched_wakeup_new event
> with the testpid variable hasn't been defined yet:
> 
>     # echo 'wakeup_new_test pid_t pid; int prio' >> \
>       /sys/kernel/debug/tracing/synthetic_events
> 
>     # cat /sys/kernel/debug/tracing/synthetic_events
>       wakeup_new_test pid_t pid; int prio
> 
> The following hist trigger both defines a testpid variable and
> specifies an onmatch() trace action that uses that variable along with
> a non-variable field to generate a wakeup_new_test synthetic event
> whenever a sched_wakeup_new event occurs, which because of the 'if
> comm == "cyclictest"' filter only happens when the executable is
> cyclictest:
> 
>     # echo 'hist:keys=testpid=pid:\
>       onmatch(sched.sched_wakeup_new).wakeup_new_test($testpid, prio) \
>         if comm=="cyclictest"' >> \
>       /sys/kernel/debug/tracing/events/sched/sched_wakeup_new/trigger
> 
> Creating and displaying a histogram based on those events is now just
> a matter of using the fields and new synthetic event in the
> tracing/events/synthetic directory, as usual:
> 
>     # echo 'hist:keys=pid,prio:sort=pid,prio' >> \
>       /sys/kernel/debug/tracing/events/synthetic/wakeup_new_test/trigger
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@xxxxxxxxxxxxxxx>
> ---

This patch is also big and it'd be better splitting - add type to
hist_field, add variable handling and implement onmatch() action.


>  kernel/trace/trace_events_hist.c | 955 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 940 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index e11b3a3..b1f859c 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -59,6 +59,7 @@ struct hist_field {
>  	unsigned int			size;
>  	unsigned int			offset;
>  	unsigned int                    is_signed;
> +	const char			*type;
>  	struct hist_field		*operands[HIST_FIELD_OPERANDS_MAX];
>  	struct hist_trigger_data	*hist_data;
>  	struct hist_var			var;
> @@ -243,6 +244,16 @@ struct hist_trigger_attrs {
>  	unsigned int	n_actions;
>  };
>  
> +struct field_var {
> +	struct hist_field	*var;
> +	struct hist_field	*val;
> +};
> +
> +struct field_var_hist {
> +	struct hist_trigger_data	*hist_data;
> +	char				*cmd;
> +};
> +
>  struct hist_trigger_data {
>  	struct hist_field               *fields[HIST_FIELDS_MAX];
>  	unsigned int			n_vals;
> @@ -263,6 +274,14 @@ struct hist_trigger_data {
>  
>  	struct action_data		*actions[HIST_ACTIONS_MAX];
>  	unsigned int			n_actions;
> +
> +	struct hist_field		*synth_var_refs[SYNTH_FIELDS_MAX];
> +	unsigned int			n_synth_var_refs;
> +	struct field_var		*field_vars[SYNTH_FIELDS_MAX];
> +	unsigned int			n_field_vars;
> +	unsigned int			n_field_var_str;
> +	struct field_var_hist		*field_var_hists[SYNTH_FIELDS_MAX];
> +	unsigned int			n_field_var_hists;
>  };
>  
>  struct synth_field {
> @@ -291,7 +310,14 @@ typedef void (*action_fn_t) (struct hist_trigger_data *hist_data,
>  
>  struct action_data {
>  	action_fn_t		fn;
> +	unsigned int		n_params;
> +	char			*params[SYNTH_FIELDS_MAX];
> +
>  	unsigned int		var_ref_idx;
> +	char			*match_event;
> +	char			*match_event_system;
> +	char			*synth_event_name;
> +	struct synth_event	*synth_event;
>  };
>  
>  static LIST_HEAD(synth_event_list);
> @@ -802,6 +828,50 @@ static struct synth_event *alloc_synth_event(char *event_name, int n_fields,
>  	return event;
>  }
>  
> +static void action_trace(struct hist_trigger_data *hist_data,
> +			 struct tracing_map_elt *elt, void *rec,
> +			 struct ring_buffer_event *rbe,
> +			 struct action_data *data, u64 *var_ref_vals)
> +{
> +	struct synth_event *event = data->synth_event;
> +
> +	trace_synth(event, var_ref_vals, data->var_ref_idx);
> +}
> +
> +static bool check_hist_action_refs(struct hist_trigger_data *hist_data,
> +				   struct synth_event *event)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < hist_data->n_actions; i++) {
> +		struct action_data *data = hist_data->actions[i];
> +
> +		if (data->fn == action_trace && data->synth_event == event)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static LIST_HEAD(hist_action_list);
> +static LIST_HEAD(hist_var_list);
> +
> +struct hist_var_data {
> +	struct list_head list;
> +	struct hist_trigger_data *hist_data;
> +};
> +
> +static bool check_synth_action_refs(struct synth_event *event)
> +{
> +	struct hist_var_data *var_data;
> +
> +	list_for_each_entry(var_data, &hist_action_list, list)
> +		if (check_hist_action_refs(var_data->hist_data, event))
> +			return true;
> +
> +	return false;
> +}
> +
>  static int create_synth_event(int argc, char **argv)
>  {
>  	struct synth_field *fields[SYNTH_FIELDS_MAX];
> @@ -832,15 +902,17 @@ static int create_synth_event(int argc, char **argv)
>  	event = find_synth_event(name);
>  	if (event) {
>  		if (delete_event) {
> +			if (check_synth_action_refs(event)) {
> +				ret = -EBUSY;
> +				goto out;
> +			}
>  			remove_synth_event(event);
>  			goto err;
>  		} else
>  			ret = -EEXIST;
>  		goto out;
> -	} else if (delete_event) {
> -		ret = -EINVAL;
> +	} else if (delete_event)
>  		goto out;
> -	}

Why is it changed?

Thanks,
Namhyung


>  
>  	if (argc < 2) {
>  		ret = -EINVAL;