Re: [PATCH v11 12/28] tracing: Add hist trigger support for user-defined sorting ('sort=' param)

[Namhyung Kim <namhyung@kernel.org>]

On Thu, Oct 22, 2015 at 01:14:16PM -0500, Tom Zanussi wrote:
> Allow users to specify keys and/or values to sort on.  With this
> addition, keys and values specified using the 'keys=' and 'vals='
> keywords can be used to sort the hist trigger output via a new 'sort='
> keyword.  If multiple sort keys are specified, the output will be
> sorted using the second key as a secondary sort key, etc.  The default
> sort order is ascending; if the user wants a different sort order,
> '.descending' can be appended to the specific sort key.  Before this
> addition, output was always sorted by 'hitcount' in ascending order.
> 
> This expands the hist trigger syntax from this:
> 
>     # echo hist:keys=xxx:vals=yyy \
>           [ if filter] > event/trigger
> 
> to this:
> 
>     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending \
>           [ if filter] > event/trigger
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---

[SNIP]
>  static int create_sort_keys(struct hist_trigger_data *hist_data)
>  {
> +	char *fields_str = hist_data->attrs->sort_key_str;
> +	struct ftrace_event_field *field = NULL;
> +	struct tracing_map_sort_key *sort_key;
> +	unsigned int i, j;
>  	int ret = 0;
>  
>  	hist_data->n_sort_keys = 1; /* sort_keys[0] is always hitcount */

This comment should be updated.

Thanks,
Namhyung


>  
> +	if (!fields_str)
> +		goto out;
> +
> +	strsep(&fields_str, "=");
> +	if (!fields_str) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < TRACING_MAP_SORT_KEYS_MAX; i++) {
> +		char *field_str, *field_name;
> +
> +		sort_key = &hist_data->sort_keys[i];
> +
> +		field_str = strsep(&fields_str, ",");
> +		if (!field_str) {
> +			if (i == 0)
> +				ret = -EINVAL;
> +			break;
> +		}
> +
> +		if ((i == TRACING_MAP_SORT_KEYS_MAX - 1) && fields_str) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		field_name = strsep(&field_str, ".");
> +		if (!field_name) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		if (!strcmp(field_name, "hitcount")) {
> +			ret = is_descending(field_str);
> +			if (ret < 0)
> +				break;
> +			sort_key->descending = ret;
> +			continue;
> +		}
> +
> +		for (j = 1; j < hist_data->n_fields; j++) {
> +			field = hist_data->fields[j]->field;
> +			if (field && !strcmp(field_name, field->name)) {
> +				sort_key->field_idx = j;
> +				ret = is_descending(field_str);
> +				if (ret < 0)
> +					goto out;
> +				sort_key->descending = ret;
> +				break;
> +			}
> +		}
> +		if (j == hist_data->n_fields) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +	}
> +	hist_data->n_sort_keys = i;
> + out:
>  	return ret;
>  }
>  
> @@ -737,7 +817,26 @@ static int event_hist_trigger_print(struct seq_file *m,
>  	}
>  
>  	seq_puts(m, ":sort=");
> -	seq_puts(m, "hitcount");
> +
> +	for (i = 0; i < hist_data->n_sort_keys; i++) {
> +		struct tracing_map_sort_key *sort_key;
> +
> +		sort_key = &hist_data->sort_keys[i];
> +
> +		if (i > 0)
> +			seq_puts(m, ",");
> +
> +		if (sort_key->field_idx == HITCOUNT_IDX)
> +			seq_puts(m, "hitcount");
> +		else {
> +			unsigned int idx = sort_key->field_idx;
> +
> +			hist_field_print(m, hist_data->fields[idx]);
> +		}
> +
> +		if (sort_key->descending)
> +			seq_puts(m, ".descending");
> +	}
>  
>  	seq_printf(m, ":size=%u", (1 << hist_data->map->map_bits));
>  
> -- 
> 1.9.3
>