From: Namhyung Kim <namhyung@kernel.org>
To: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: rostedt@goodmis.org, tglx@linutronix.de, mhiramat@kernel.org,
vedang.patel@intel.com, bigeasy@linutronix.de,
joel.opensrc@gmail.com, joelaf@google.com,
mathieu.desnoyers@efficios.com, baohong.liu@intel.com,
rajvi.jingar@intel.com, julia@ni.com, fengguang.wu@intel.com,
linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org,
kernel-team@lge.com
Subject: Re: [PATCH v7 19/37] tracing: Generalize per-element hist trigger data
Date: Fri, 8 Dec 2017 22:06:29 +0900 [thread overview]
Message-ID: <20171208130629.GB7312@danjae.aot.lge.com> (raw)
In-Reply-To: <28c717b25a6440648171c99fc2fc94f63164096a.1512593081.git.tom.zanussi@linux.intel.com>
On Wed, Dec 06, 2017 at 04:38:00PM -0600, Tom Zanussi wrote:
> Up until now, hist triggers only needed per-element support for saving
> 'comm' data, which was saved directly as a private data pointer.
>
> In anticipation of the need to save other data besides 'comm', add a
> new hist_elt_data struct for the purpose, and switch the current
> 'comm'-related code over to that.
>
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
> kernel/trace/trace_events_hist.c | 76 +++++++++++++++++++++++-----------------
> 1 file changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index f75f7bc..3aeab8e 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -289,6 +289,10 @@ static struct hist_field *find_var(struct hist_trigger_data *hist_data,
> return NULL;
> }
>
> +struct hist_elt_data {
> + char *comm;
> +};
> +
> static const char *hist_field_name(struct hist_field *field,
> unsigned int level)
> {
> @@ -503,45 +507,61 @@ static inline void save_comm(char *comm, struct task_struct *task)
> memcpy(comm, task->comm, TASK_COMM_LEN);
> }
>
> -static void hist_trigger_elt_comm_free(struct tracing_map_elt *elt)
> +static void hist_elt_data_free(struct hist_elt_data *elt_data)
> +{
> + kfree(elt_data->comm);
> + kfree(elt_data);
> +}
> +
> +static void hist_trigger_elt_data_free(struct tracing_map_elt *elt)
> {
> - kfree((char *)elt->private_data);
> + struct hist_elt_data *elt_data = elt->private_data;
> +
> + hist_elt_data_free(elt_data);
> }
>
> -static int hist_trigger_elt_comm_alloc(struct tracing_map_elt *elt)
> +static int hist_trigger_elt_data_alloc(struct tracing_map_elt *elt)
> {
> struct hist_trigger_data *hist_data = elt->map->private_data;
> + unsigned int size = TASK_COMM_LEN + 1;
AFAIK you don't need to do '+ 1' since task->comm is always terminated
by a NUL character using strlcpy().
Thanks,
Namhyung
> + struct hist_elt_data *elt_data;
> struct hist_field *key_field;
> unsigned int i;
>
> + elt_data = kzalloc(sizeof(*elt_data), GFP_KERNEL);
> + if (!elt_data)
> + return -ENOMEM;
> +
> for_each_hist_key_field(i, hist_data) {
> key_field = hist_data->fields[i];
>
> if (key_field->flags & HIST_FIELD_FL_EXECNAME) {
> - unsigned int size = TASK_COMM_LEN + 1;
> -
> - elt->private_data = kzalloc(size, GFP_KERNEL);
> - if (!elt->private_data)
> + elt_data->comm = kzalloc(size, GFP_KERNEL);
> + if (!elt_data->comm) {
> + kfree(elt_data);
> return -ENOMEM;
> + }
> break;
> }
> }
>
> + elt->private_data = elt_data;
> +
> return 0;
> }
>
> -static void hist_trigger_elt_comm_init(struct tracing_map_elt *elt)
> +static void hist_trigger_elt_data_init(struct tracing_map_elt *elt)
> {
> - char *comm = elt->private_data;
> + struct hist_elt_data *elt_data = elt->private_data;
>
> - if (comm)
> - save_comm(comm, current);
> + if (elt_data->comm)
> + save_comm(elt_data->comm, current);
> }
>
> -static const struct tracing_map_ops hist_trigger_elt_comm_ops = {
> - .elt_alloc = hist_trigger_elt_comm_alloc,
> - .elt_free = hist_trigger_elt_comm_free,
> - .elt_init = hist_trigger_elt_comm_init,
> +static const struct tracing_map_ops hist_trigger_elt_data_ops = {
> + .elt_alloc = hist_trigger_elt_data_alloc,
> + .elt_free = hist_trigger_elt_data_free,
> + .elt_init = hist_trigger_elt_data_init,
> };
>
> static const char *get_hist_field_flags(struct hist_field *hist_field)
> @@ -1481,21 +1501,6 @@ static int create_tracing_map_fields(struct hist_trigger_data *hist_data)
> return 0;
> }
>
> -static bool need_tracing_map_ops(struct hist_trigger_data *hist_data)
> -{
> - struct hist_field *key_field;
> - unsigned int i;
> -
> - for_each_hist_key_field(i, hist_data) {
> - key_field = hist_data->fields[i];
> -
> - if (key_field->flags & HIST_FIELD_FL_EXECNAME)
> - return true;
> - }
> -
> - return false;
> -}
> -
> static struct hist_trigger_data *
> create_hist_data(unsigned int map_bits,
> struct hist_trigger_attrs *attrs,
> @@ -1521,8 +1526,7 @@ static bool need_tracing_map_ops(struct hist_trigger_data *hist_data)
> if (ret)
> goto free;
>
> - if (need_tracing_map_ops(hist_data))
> - map_ops = &hist_trigger_elt_comm_ops;
> + map_ops = &hist_trigger_elt_data_ops;
>
> hist_data->map = tracing_map_create(map_bits, hist_data->key_size,
> map_ops, hist_data);
> @@ -1710,7 +1714,13 @@ static void hist_trigger_stacktrace_print(struct seq_file *m,
> seq_printf(m, "%s: [%llx] %-55s", field_name,
> uval, str);
> } else if (key_field->flags & HIST_FIELD_FL_EXECNAME) {
> - char *comm = elt->private_data;
> + struct hist_elt_data *elt_data = elt->private_data;
> + char *comm;
> +
> + if (WARN_ON_ONCE(!elt_data))
> + return;
> +
> + comm = elt_data->comm;
>
> uval = *(u64 *)(key + key_field->offset);
> seq_printf(m, "%s: %-16s[%10llu]", field_name,
> --
> 1.9.3
>
next prev parent reply other threads:[~2017-12-08 13:09 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-06 22:37 [PATCH v7 00/37] tracing: Inter-event (e.g. latency) support Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 01/37] tracing: Move hist trigger Documentation to histogram.txt Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 02/37] tracing: Add Documentation for log2 modifier Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 03/37] tracing: Add support to detect and avoid duplicates Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 04/37] tracing: Remove code which merges duplicates Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 05/37] ring-buffer: Add interface for setting absolute time stamps Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 06/37] ring-buffer: Redefine the unimplemented RINGBUF_TYPE_TIME_STAMP Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 07/37] tracing: Add timestamp_mode trace file Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 08/37] tracing: Give event triggers access to ring_buffer_event Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 09/37] tracing: Add ring buffer event param to hist field functions Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 10/37] tracing: Break out hist trigger assignment parsing Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 11/37] tracing: Add hist trigger timestamp support Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 12/37] tracing: Add per-element variable support to tracing_map Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 13/37] tracing: Add hist_data member to hist_field Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 14/37] tracing: Add usecs modifier for hist trigger timestamps Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 15/37] tracing: Add variable support to hist triggers Tom Zanussi
2017-12-08 13:02 ` Namhyung Kim
2017-12-08 19:54 ` Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 16/37] tracing: Account for variables in named trigger compatibility Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 17/37] tracing: Move get_hist_field_flags() Tom Zanussi
2017-12-06 22:37 ` [PATCH v7 18/37] tracing: Add simple expression support to hist triggers Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 19/37] tracing: Generalize per-element hist trigger data Tom Zanussi
2017-12-08 13:06 ` Namhyung Kim [this message]
2017-12-08 19:56 ` Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 20/37] tracing: Pass tracing_map_elt to hist_field accessor functions Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 21/37] tracing: Add hist_field 'type' field Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 22/37] tracing: Add variable reference handling to hist triggers Tom Zanussi
2017-12-11 15:17 ` Namhyung Kim
2017-12-11 17:53 ` Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 23/37] tracing: Add hist trigger action hook Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 24/37] tracing: Add support for 'synthetic' events Tom Zanussi
2017-12-12 14:42 ` Namhyung Kim
2017-12-12 16:07 ` Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 25/37] tracing: Add support for 'field variables' Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 26/37] tracing: Add 'onmatch' hist trigger action support Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 27/37] tracing: Add 'onmax' " Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 28/37] tracing: Allow whitespace to surround hist trigger filter Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 29/37] tracing: Add cpu field for hist triggers Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 30/37] tracing: Add hist trigger support for variable reference aliases Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 31/37] tracing: Add 'last error' error facility for hist triggers Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 32/37] tracing: Add inter-event hist trigger Documentation Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 33/37] tracing: Make tracing_set_clock() non-static Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 34/37] tracing: Add a clock attribute for hist triggers Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 35/37] tracing: Increase trace_recursive_lock() limit for synthetic events Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 36/37] tracing: Add inter-event blurb to HIST_TRIGGERS config option Tom Zanussi
2017-12-06 22:38 ` [PATCH v7 37/37] selftests: ftrace: Add inter-event hist triggers testcases Tom Zanussi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171208130629.GB7312@danjae.aot.lge.com \
--to=namhyung@kernel.org \
--cc=baohong.liu@intel.com \
--cc=bigeasy@linutronix.de \
--cc=fengguang.wu@intel.com \
--cc=joel.opensrc@gmail.com \
--cc=joelaf@google.com \
--cc=julia@ni.com \
--cc=kernel-team@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=rajvi.jingar@intel.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=tom.zanussi@linux.intel.com \
--cc=vedang.patel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).