Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1348096

Re: [PATCH v15 02/23] tracing: Add 'hist' event trigger command

From Steven Rostedt <rostedt@goodmis.org>
Newsgroups linux.kernel
Subject Re: [PATCH v15 02/23] tracing: Add 'hist' event trigger command
Date 2016-03-02 15:40 +0100
Message-ID <r8g6u-uK-37@gated-at.bofh.it> (permalink)
References <r6t7Q-5QF-3@gated-at.bofh.it> <r6t7Q-5QF-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, 26 Feb 2016 10:01:05 -0600
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:


> +static int create_hitcount_val(struct hist_trigger_data *hist_data)
> +{
> +	hist_data->fields[HITCOUNT_IDX] =
> +		create_hist_field(NULL, HIST_FIELD_FL_HITCOUNT);
> +	if (!hist_data->fields[HITCOUNT_IDX])
> +		return -ENOMEM;
> +
> +	if (WARN_ON(++hist_data->n_vals > TRACING_MAP_VALS_MAX))

Never put side effects in a WARN_ON() macro. Some configs define
WARN_ON() as a nop. Which would break this code. Always do the
following:

	hist_data->n_vals++;

	if (WARN_ON(hist_data->n_vals > TRACING_MAP_VALS_MAX))
> +		return -EINVAL;

> +
> +	return 0;
> +}
> +
> +static int create_val_fields(struct hist_trigger_data *hist_data,
> +			     struct trace_event_file *file)
> +{
> +	int ret;
> +
> +	ret = create_hitcount_val(hist_data);
> +
> +	return ret;
> +}
> +
> +static int create_key_field(struct hist_trigger_data *hist_data,
> +			    unsigned int key_idx,
> +			    struct trace_event_file *file,
> +			    char *field_str)
> +{
> +	struct ftrace_event_field *field = NULL;
> +	unsigned long flags = 0;
> +	unsigned int key_size;
> +	int ret = 0;
> +
> +	if (WARN_ON(key_idx >= TRACING_MAP_FIELDS_MAX))
> +		return -EINVAL;
> +
> +	flags |= HIST_FIELD_FL_KEY;
> +
> +	field = trace_find_event_field(file->event_call, field_str);
> +	if (!field) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	key_size = field->size;
> +
> +	hist_data->fields[key_idx] = create_hist_field(field, flags);
> +	if (!hist_data->fields[key_idx]) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	key_size = ALIGN(key_size, sizeof(u64));
> +	hist_data->fields[key_idx]->size = key_size;
> +	hist_data->key_size = key_size;
> +	if (hist_data->key_size > HIST_KEY_SIZE_MAX) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (WARN_ON(++hist_data->n_keys > TRACING_MAP_KEYS_MAX))

Same here.

> +		return -EINVAL;
> +
> +	ret = key_size;
> + out:
> +	return ret;
> +}
> +

-- Steve

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH v15 02/23] tracing: Add 'hist' event trigger command Tom Zanussi <tom.zanussi@linux.intel.com> - 2016-02-26 17:10 +0100
  Re: [PATCH v15 02/23] tracing: Add 'hist' event trigger command Steven Rostedt <rostedt@goodmis.org> - 2016-03-02 15:40 +0100

csiph-web