Re: [PATCH v11 17/28] tracing: Add hist trigger 'execname' modifier

2015-11-03 Thread Tom Zanussi 
On Mon, 2015-11-02 at 23:10 +0900, Namhyung Kim wrote:
> On Thu, Oct 22, 2015 at 01:14:21PM -0500, Tom Zanussi wrote:
> > Allow users to have pid fields displayed as program names in the output
> > by appending '.execname' to field names:
> > 
> ># echo hist:keys=aaa.execname ... \
> >   [ if filter] > event/trigger
> 
> So in this case, 'aaa' should be 'common_pid', right?
> 

Right.

> Also, it's just for display thus tasks which have different pid but
> same comm name will be saved in different entries, right?  If so, it'd
> be better to note that somewhere as it might confuse users..
> 

Right, good point, will document that.

> 
> > 
> > Signed-off-by: Tom Zanussi 
> > Tested-by: Masami Hiramatsu 
> > ---
> >  kernel/trace/trace.c |  3 +-
> >  kernel/trace/trace_events_hist.c | 86 
> > +++-
> >  2 files changed, 87 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 4f0968e..a143be0 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3822,7 +3822,8 @@ static const char readme_msg[] =
> > "\tfollowing modifiers to the field name, as applicable:\n\n"
> > "\t.hexdisplay a number as a hex value\n"
> > "\t.symdisplay an address as a symbol\n"
> > -   "\t.sym-offset display an address as a symbol and 
> > offset\n\n"
> > +   "\t.sym-offset display an address as a symbol and offset\n"
> > +   "\t.execname   display a common_pid as a program name\n\n"
> > "\tThe 'pause' parameter can be used to pause an existing hist\n"
> > "\ttrigger or to start a hist trigger but not log any events\n"
> > "\tuntil told to do so.  'continue' can be used to start or\n"
> > diff --git a/kernel/trace/trace_events_hist.c 
> > b/kernel/trace/trace_events_hist.c
> > index e6e5c6e..656973a 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -75,6 +75,7 @@ enum hist_field_flags {
> > HIST_FIELD_HEX  = 8,
> > HIST_FIELD_SYM  = 16,
> > HIST_FIELD_SYM_OFFSET   = 32,
> > +   HIST_FIELD_EXECNAME = 64,
> >  };
> >  
> >  struct hist_trigger_attrs {
> > @@ -218,6 +219,78 @@ static struct hist_trigger_attrs 
> > *parse_hist_trigger_attrs(char *trigger_str)
> > return ERR_PTR(ret);
> >  }
> >  
> > +static inline void save_comm(char *comm, struct task_struct *task)
> > +{
> > +   if (!task->pid) {
> > +   strcpy(comm, "");
> > +   return;
> > +   }
> > +
> > +   if (WARN_ON_ONCE(task->pid < 0)) {
> > +   strcpy(comm, "");
> > +   return;
> > +   }
> > +
> > +   if (task->pid > PID_MAX_DEFAULT) {
> > +   strcpy(comm, "<...>");
> > +   return;
> > +   }
> 
> I guess this comes from the saved_cmdlines code which uses a fixed
> array of PID_MAX_DEFAULT size.  But as it doesn't use the array, we
> can simply copy the task->comm IMHO.
> 
> 
> > +
> > +   memcpy(comm, task->comm, TASK_COMM_LEN);
> > +}
> > +
> > +static void hist_trigger_elt_free(struct tracing_map_elt *elt)
> > +{
> > +   kfree((char *)elt->private_data);
> > +}
> > +
> > +static int hist_trigger_elt_alloc(struct tracing_map_elt *elt)
> > +{
> > +   struct hist_trigger_data *hist_data = elt->map->private_data;
> > +   struct hist_field *key_field;
> > +   unsigned int i;
> > +
> > +   for (i = hist_data->n_vals; i < hist_data->n_fields; i++) {
> 
> I think it'd be better to add a list iterator macro like
> for_each_key_field() or something since it's easy to miss that the key
> fields always follows the value fields IMHO.
> 

Yeah, that's a good idea (sometimes I even forget that myself ;-)

> 
> > +   key_field = hist_data->fields[i];
> > +
> > +   if (key_field->flags & HIST_FIELD_EXECNAME) {
> > +   unsigned int size = TASK_COMM_LEN + 1;
> > +
> > +   elt->private_data = kzalloc(size, GFP_KERNEL);
> > +   if (!elt->private_data)
> > +   return -ENOMEM;
> > +   break;
> > +   }
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static void hist_trigger_elt_copy(struct tracing_map_elt *to,
> > + struct tracing_map_elt *from)
> > +{
> > +   char *comm_from = from->private_data;
> > +   char *comm_to = to->private_data;
> > +
> > +   if (comm_from)
> > +   memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
> > +}
> > +
> > +static void hist_trigger_elt_init(struct tracing_map_elt *elt)
> > +{
> > +   char *comm = elt->private_data;
> > +
> > +   if (comm)
> > +   save_comm(comm, current);
> > +}
> > +
> > +static struct tracing_map_ops hist_trigger_ops = {
> > +   .elt_alloc  = hist_trigger_elt_alloc,
> > +   .elt_copy   = hist_trigger_elt_copy,
> > +   .elt_free   = hist_trigger_elt_free,
> > +   .elt_init   =

Re: [PATCH v11 17/28] tracing: Add hist trigger 'execname' modifier

2015-11-03 Thread Tom Zanussi 
On Mon, 2015-11-02 at 23:10 +0900, Namhyung Kim wrote:
> On Thu, Oct 22, 2015 at 01:14:21PM -0500, Tom Zanussi wrote:
> > Allow users to have pid fields displayed as program names in the output
> > by appending '.execname' to field names:
> > 
> ># echo hist:keys=aaa.execname ... \
> >   [ if filter] > event/trigger
> 
> So in this case, 'aaa' should be 'common_pid', right?
> 

Right.

> Also, it's just for display thus tasks which have different pid but
> same comm name will be saved in different entries, right?  If so, it'd
> be better to note that somewhere as it might confuse users..
> 

Right, good point, will document that.

> 
> > 
> > Signed-off-by: Tom Zanussi 
> > Tested-by: Masami Hiramatsu 
> > ---
> >  kernel/trace/trace.c |  3 +-
> >  kernel/trace/trace_events_hist.c | 86 
> > +++-
> >  2 files changed, 87 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 4f0968e..a143be0 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3822,7 +3822,8 @@ static const char readme_msg[] =
> > "\tfollowing modifiers to the field name, as applicable:\n\n"
> > "\t.hexdisplay a number as a hex value\n"
> > "\t.symdisplay an address as a symbol\n"
> > -   "\t.sym-offset display an address as a symbol and 
> > offset\n\n"
> > +   "\t.sym-offset display an address as a symbol and offset\n"
> > +   "\t.execname   display a common_pid as a program name\n\n"
> > "\tThe 'pause' parameter can be used to pause an existing hist\n"
> > "\ttrigger or to start a hist trigger but not log any events\n"
> > "\tuntil told to do so.  'continue' can be used to start or\n"
> > diff --git a/kernel/trace/trace_events_hist.c 
> > b/kernel/trace/trace_events_hist.c
> > index e6e5c6e..656973a 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -75,6 +75,7 @@ enum hist_field_flags {
> > HIST_FIELD_HEX  = 8,
> > HIST_FIELD_SYM  = 16,
> > HIST_FIELD_SYM_OFFSET   = 32,
> > +   HIST_FIELD_EXECNAME = 64,
> >  };
> >  
> >  struct hist_trigger_attrs {
> > @@ -218,6 +219,78 @@ static struct hist_trigger_attrs 
> > *parse_hist_trigger_attrs(char *trigger_str)
> > return ERR_PTR(ret);
> >  }
> >  
> > +static inline void save_comm(char *comm, struct task_struct *task)
> > +{
> > +   if (!task->pid) {
> > +   strcpy(comm, "");
> > +   return;
> > +   }
> > +
> > +   if (WARN_ON_ONCE(task->pid < 0)) {
> > +   strcpy(comm, "");
> > +   return;
> > +   }
> > +
> > +   if (task->pid > PID_MAX_DEFAULT) {
> > +   strcpy(comm, "<...>");
> > +   return;
> > +   }
> 
> I guess this comes from the saved_cmdlines code which uses a fixed
> array of PID_MAX_DEFAULT size.  But as it doesn't use the array, we
> can simply copy the task->comm IMHO.
> 
> 
> > +
> > +   memcpy(comm, task->comm, TASK_COMM_LEN);
> > +}
> > +
> > +static void hist_trigger_elt_free(struct tracing_map_elt *elt)
> > +{
> > +   kfree((char *)elt->private_data);
> > +}
> > +
> > +static int hist_trigger_elt_alloc(struct tracing_map_elt *elt)
> > +{
> > +   struct hist_trigger_data *hist_data = elt->map->private_data;
> > +   struct hist_field *key_field;
> > +   unsigned int i;
> > +
> > +   for (i = hist_data->n_vals; i < hist_data->n_fields; i++) {
> 
> I think it'd be better to add a list iterator macro like
> for_each_key_field() or something since it's easy to miss that the key
> fields always follows the value fields IMHO.
> 

Yeah, that's a good idea (sometimes I even forget that myself ;-)

> 
> > +   key_field = hist_data->fields[i];
> > +
> > +   if (key_field->flags & HIST_FIELD_EXECNAME) {
> > +   unsigned int size = TASK_COMM_LEN + 1;
> > +
> > +   elt->private_data = kzalloc(size, GFP_KERNEL);
> > +   if (!elt->private_data)
> > +   return -ENOMEM;
> > +   break;
> > +   }
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static void hist_trigger_elt_copy(struct tracing_map_elt *to,
> > + struct tracing_map_elt *from)
> > +{
> > +   char *comm_from = from->private_data;
> > +   char *comm_to = to->private_data;
> > +
> > +   if (comm_from)
> > +   memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
> > +}
> > +
> > +static void hist_trigger_elt_init(struct tracing_map_elt *elt)
> > +{
> > +   char *comm = elt->private_data;
> > +
> > +   if (comm)
> > +   save_comm(comm, current);
> > +}
> > +
> > +static struct tracing_map_ops hist_trigger_ops = {
> > +   .elt_alloc  = hist_trigger_elt_alloc,
> > +   .elt_copy   = hist_trigger_elt_copy,
> > +   .elt_free   =

Re: [PATCH v11 17/28] tracing: Add hist trigger 'execname' modifier

2015-11-02 Thread Namhyung Kim 
On Thu, Oct 22, 2015 at 01:14:21PM -0500, Tom Zanussi wrote:
> Allow users to have pid fields displayed as program names in the output
> by appending '.execname' to field names:
> 
># echo hist:keys=aaa.execname ... \
>   [ if filter] > event/trigger

So in this case, 'aaa' should be 'common_pid', right?

Also, it's just for display thus tasks which have different pid but
same comm name will be saved in different entries, right?  If so, it'd
be better to note that somewhere as it might confuse users..


> 
> Signed-off-by: Tom Zanussi 
> Tested-by: Masami Hiramatsu 
> ---
>  kernel/trace/trace.c |  3 +-
>  kernel/trace/trace_events_hist.c | 86 
> +++-
>  2 files changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 4f0968e..a143be0 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3822,7 +3822,8 @@ static const char readme_msg[] =
>   "\tfollowing modifiers to the field name, as applicable:\n\n"
>   "\t.hexdisplay a number as a hex value\n"
>   "\t.symdisplay an address as a symbol\n"
> - "\t.sym-offset display an address as a symbol and 
> offset\n\n"
> + "\t.sym-offset display an address as a symbol and offset\n"
> + "\t.execname   display a common_pid as a program name\n\n"
>   "\tThe 'pause' parameter can be used to pause an existing hist\n"
>   "\ttrigger or to start a hist trigger but not log any events\n"
>   "\tuntil told to do so.  'continue' can be used to start or\n"
> diff --git a/kernel/trace/trace_events_hist.c 
> b/kernel/trace/trace_events_hist.c
> index e6e5c6e..656973a 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -75,6 +75,7 @@ enum hist_field_flags {
>   HIST_FIELD_HEX  = 8,
>   HIST_FIELD_SYM  = 16,
>   HIST_FIELD_SYM_OFFSET   = 32,
> + HIST_FIELD_EXECNAME = 64,
>  };
>  
>  struct hist_trigger_attrs {
> @@ -218,6 +219,78 @@ static struct hist_trigger_attrs 
> *parse_hist_trigger_attrs(char *trigger_str)
>   return ERR_PTR(ret);
>  }
>  
> +static inline void save_comm(char *comm, struct task_struct *task)
> +{
> + if (!task->pid) {
> + strcpy(comm, "");
> + return;
> + }
> +
> + if (WARN_ON_ONCE(task->pid < 0)) {
> + strcpy(comm, "");
> + return;
> + }
> +
> + if (task->pid > PID_MAX_DEFAULT) {
> + strcpy(comm, "<...>");
> + return;
> + }

I guess this comes from the saved_cmdlines code which uses a fixed
array of PID_MAX_DEFAULT size.  But as it doesn't use the array, we
can simply copy the task->comm IMHO.


> +
> + memcpy(comm, task->comm, TASK_COMM_LEN);
> +}
> +
> +static void hist_trigger_elt_free(struct tracing_map_elt *elt)
> +{
> + kfree((char *)elt->private_data);
> +}
> +
> +static int hist_trigger_elt_alloc(struct tracing_map_elt *elt)
> +{
> + struct hist_trigger_data *hist_data = elt->map->private_data;
> + struct hist_field *key_field;
> + unsigned int i;
> +
> + for (i = hist_data->n_vals; i < hist_data->n_fields; i++) {

I think it'd be better to add a list iterator macro like
for_each_key_field() or something since it's easy to miss that the key
fields always follows the value fields IMHO.


> + key_field = hist_data->fields[i];
> +
> + if (key_field->flags & HIST_FIELD_EXECNAME) {
> + unsigned int size = TASK_COMM_LEN + 1;
> +
> + elt->private_data = kzalloc(size, GFP_KERNEL);
> + if (!elt->private_data)
> + return -ENOMEM;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void hist_trigger_elt_copy(struct tracing_map_elt *to,
> +   struct tracing_map_elt *from)
> +{
> + char *comm_from = from->private_data;
> + char *comm_to = to->private_data;
> +
> + if (comm_from)
> + memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
> +}
> +
> +static void hist_trigger_elt_init(struct tracing_map_elt *elt)
> +{
> + char *comm = elt->private_data;
> +
> + if (comm)
> + save_comm(comm, current);
> +}
> +
> +static struct tracing_map_ops hist_trigger_ops = {
> + .elt_alloc  = hist_trigger_elt_alloc,
> + .elt_copy   = hist_trigger_elt_copy,
> + .elt_free   = hist_trigger_elt_free,
> + .elt_init   = hist_trigger_elt_init,
> +};

How about making it const?


> +
>  static void destroy_hist_field(struct hist_field *hist_field)
>  {
>   kfree(hist_field);
> @@ -376,6 +449,9 @@ static int create_key_field(struct hist_trigger_data 
> *hist_data,
>   flags |= HIST_FIELD_SYM;
>   else if (!strcmp(field_str, "sym-offset"))
>

Re: [PATCH v11 17/28] tracing: Add hist trigger 'execname' modifier

2015-11-02 Thread Namhyung Kim 
On Thu, Oct 22, 2015 at 01:14:21PM -0500, Tom Zanussi wrote:
> Allow users to have pid fields displayed as program names in the output
> by appending '.execname' to field names:
> 
># echo hist:keys=aaa.execname ... \
>   [ if filter] > event/trigger

So in this case, 'aaa' should be 'common_pid', right?

Also, it's just for display thus tasks which have different pid but
same comm name will be saved in different entries, right?  If so, it'd
be better to note that somewhere as it might confuse users..


> 
> Signed-off-by: Tom Zanussi 
> Tested-by: Masami Hiramatsu 
> ---
>  kernel/trace/trace.c |  3 +-
>  kernel/trace/trace_events_hist.c | 86 
> +++-
>  2 files changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 4f0968e..a143be0 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3822,7 +3822,8 @@ static const char readme_msg[] =
>   "\tfollowing modifiers to the field name, as applicable:\n\n"
>   "\t.hexdisplay a number as a hex value\n"
>   "\t.symdisplay an address as a symbol\n"
> - "\t.sym-offset display an address as a symbol and 
> offset\n\n"
> + "\t.sym-offset display an address as a symbol and offset\n"
> + "\t.execname   display a common_pid as a program name\n\n"
>   "\tThe 'pause' parameter can be used to pause an existing hist\n"
>   "\ttrigger or to start a hist trigger but not log any events\n"
>   "\tuntil told to do so.  'continue' can be used to start or\n"
> diff --git a/kernel/trace/trace_events_hist.c 
> b/kernel/trace/trace_events_hist.c
> index e6e5c6e..656973a 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -75,6 +75,7 @@ enum hist_field_flags {
>   HIST_FIELD_HEX  = 8,
>   HIST_FIELD_SYM  = 16,
>   HIST_FIELD_SYM_OFFSET   = 32,
> + HIST_FIELD_EXECNAME = 64,
>  };
>  
>  struct hist_trigger_attrs {
> @@ -218,6 +219,78 @@ static struct hist_trigger_attrs 
> *parse_hist_trigger_attrs(char *trigger_str)
>   return ERR_PTR(ret);
>  }
>  
> +static inline void save_comm(char *comm, struct task_struct *task)
> +{
> + if (!task->pid) {
> + strcpy(comm, "");
> + return;
> + }
> +
> + if (WARN_ON_ONCE(task->pid < 0)) {
> + strcpy(comm, "");
> + return;
> + }
> +
> + if (task->pid > PID_MAX_DEFAULT) {
> + strcpy(comm, "<...>");
> + return;
> + }

I guess this comes from the saved_cmdlines code which uses a fixed
array of PID_MAX_DEFAULT size.  But as it doesn't use the array, we
can simply copy the task->comm IMHO.


> +
> + memcpy(comm, task->comm, TASK_COMM_LEN);
> +}
> +
> +static void hist_trigger_elt_free(struct tracing_map_elt *elt)
> +{
> + kfree((char *)elt->private_data);
> +}
> +
> +static int hist_trigger_elt_alloc(struct tracing_map_elt *elt)
> +{
> + struct hist_trigger_data *hist_data = elt->map->private_data;
> + struct hist_field *key_field;
> + unsigned int i;
> +
> + for (i = hist_data->n_vals; i < hist_data->n_fields; i++) {

I think it'd be better to add a list iterator macro like
for_each_key_field() or something since it's easy to miss that the key
fields always follows the value fields IMHO.


> + key_field = hist_data->fields[i];
> +
> + if (key_field->flags & HIST_FIELD_EXECNAME) {
> + unsigned int size = TASK_COMM_LEN + 1;
> +
> + elt->private_data = kzalloc(size, GFP_KERNEL);
> + if (!elt->private_data)
> + return -ENOMEM;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void hist_trigger_elt_copy(struct tracing_map_elt *to,
> +   struct tracing_map_elt *from)
> +{
> + char *comm_from = from->private_data;
> + char *comm_to = to->private_data;
> +
> + if (comm_from)
> + memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
> +}
> +
> +static void hist_trigger_elt_init(struct tracing_map_elt *elt)
> +{
> + char *comm = elt->private_data;
> +
> + if (comm)
> + save_comm(comm, current);
> +}
> +
> +static struct tracing_map_ops hist_trigger_ops = {
> + .elt_alloc  = hist_trigger_elt_alloc,
> + .elt_copy   = hist_trigger_elt_copy,
> + .elt_free   = hist_trigger_elt_free,
> + .elt_init   = hist_trigger_elt_init,
> +};

How about making it const?


> +
>  static void destroy_hist_field(struct hist_field *hist_field)
>  {
>   kfree(hist_field);
> @@ -376,6 +449,9 @@ static int create_key_field(struct hist_trigger_data 
> *hist_data,
>   flags |= HIST_FIELD_SYM;

[PATCH v11 17/28] tracing: Add hist trigger 'execname' modifier

2015-10-22 Thread Tom Zanussi 
Allow users to have pid fields displayed as program names in the output
by appending '.execname' to field names:

   # echo hist:keys=aaa.execname ... \
  [ if filter] > event/trigger

Signed-off-by: Tom Zanussi 
Tested-by: Masami Hiramatsu 
---
 kernel/trace/trace.c |  3 +-
 kernel/trace/trace_events_hist.c | 86 +++-
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4f0968e..a143be0 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3822,7 +3822,8 @@ static const char readme_msg[] =
"\tfollowing modifiers to the field name, as applicable:\n\n"
"\t.hexdisplay a number as a hex value\n"
"\t.symdisplay an address as a symbol\n"
-   "\t.sym-offset display an address as a symbol and 
offset\n\n"
+   "\t.sym-offset display an address as a symbol and offset\n"
+   "\t.execname   display a common_pid as a program name\n\n"
"\tThe 'pause' parameter can be used to pause an existing hist\n"
"\ttrigger or to start a hist trigger but not log any events\n"
"\tuntil told to do so.  'continue' can be used to start or\n"
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index e6e5c6e..656973a 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -75,6 +75,7 @@ enum hist_field_flags {
HIST_FIELD_HEX  = 8,
HIST_FIELD_SYM  = 16,
HIST_FIELD_SYM_OFFSET   = 32,
+   HIST_FIELD_EXECNAME = 64,
 };
 
 struct hist_trigger_attrs {
@@ -218,6 +219,78 @@ static struct hist_trigger_attrs 
*parse_hist_trigger_attrs(char *trigger_str)
return ERR_PTR(ret);
 }
 
+static inline void save_comm(char *comm, struct task_struct *task)
+{
+   if (!task->pid) {
+   strcpy(comm, "");
+   return;
+   }
+
+   if (WARN_ON_ONCE(task->pid < 0)) {
+   strcpy(comm, "");
+   return;
+   }
+
+   if (task->pid > PID_MAX_DEFAULT) {
+   strcpy(comm, "<...>");
+   return;
+   }
+
+   memcpy(comm, task->comm, TASK_COMM_LEN);
+}
+
+static void hist_trigger_elt_free(struct tracing_map_elt *elt)
+{
+   kfree((char *)elt->private_data);
+}
+
+static int hist_trigger_elt_alloc(struct tracing_map_elt *elt)
+{
+   struct hist_trigger_data *hist_data = elt->map->private_data;
+   struct hist_field *key_field;
+   unsigned int i;
+
+   for (i = hist_data->n_vals; i < hist_data->n_fields; i++) {
+   key_field = hist_data->fields[i];
+
+   if (key_field->flags & HIST_FIELD_EXECNAME) {
+   unsigned int size = TASK_COMM_LEN + 1;
+
+   elt->private_data = kzalloc(size, GFP_KERNEL);
+   if (!elt->private_data)
+   return -ENOMEM;
+   break;
+   }
+   }
+
+   return 0;
+}
+
+static void hist_trigger_elt_copy(struct tracing_map_elt *to,
+ struct tracing_map_elt *from)
+{
+   char *comm_from = from->private_data;
+   char *comm_to = to->private_data;
+
+   if (comm_from)
+   memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
+}
+
+static void hist_trigger_elt_init(struct tracing_map_elt *elt)
+{
+   char *comm = elt->private_data;
+
+   if (comm)
+   save_comm(comm, current);
+}
+
+static struct tracing_map_ops hist_trigger_ops = {
+   .elt_alloc  = hist_trigger_elt_alloc,
+   .elt_copy   = hist_trigger_elt_copy,
+   .elt_free   = hist_trigger_elt_free,
+   .elt_init   = hist_trigger_elt_init,
+};
+
 static void destroy_hist_field(struct hist_field *hist_field)
 {
kfree(hist_field);
@@ -376,6 +449,9 @@ static int create_key_field(struct hist_trigger_data 
*hist_data,
flags |= HIST_FIELD_SYM;
else if (!strcmp(field_str, "sym-offset"))
flags |= HIST_FIELD_SYM_OFFSET;
+   else if (!strcmp(field_str, "execname") &&
+!strcmp(field_name, "common_pid"))
+   flags |= HIST_FIELD_EXECNAME;
else {
ret = -EINVAL;
goto out;
@@ -612,7 +688,7 @@ create_hist_data(unsigned int map_bits,
goto free;
 
hist_data->map = tracing_map_create(map_bits, hist_data->key_size,
-   NULL, hist_data);
+   _trigger_ops, hist_data);
if (IS_ERR(hist_data->map)) {
ret = PTR_ERR(hist_data->map);
hist_data->map = NULL;
@@ -732,6 +808,12 @@ hist_trigger_entry_print(struct seq_file *m,

[PATCH v11 17/28] tracing: Add hist trigger 'execname' modifier

2015-10-22 Thread Tom Zanussi 
Allow users to have pid fields displayed as program names in the output
by appending '.execname' to field names:

   # echo hist:keys=aaa.execname ... \
  [ if filter] > event/trigger

Signed-off-by: Tom Zanussi 
Tested-by: Masami Hiramatsu 
---
 kernel/trace/trace.c |  3 +-
 kernel/trace/trace_events_hist.c | 86 +++-
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4f0968e..a143be0 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3822,7 +3822,8 @@ static const char readme_msg[] =
"\tfollowing modifiers to the field name, as applicable:\n\n"
"\t.hexdisplay a number as a hex value\n"
"\t.symdisplay an address as a symbol\n"
-   "\t.sym-offset display an address as a symbol and 
offset\n\n"
+   "\t.sym-offset display an address as a symbol and offset\n"
+   "\t.execname   display a common_pid as a program name\n\n"
"\tThe 'pause' parameter can be used to pause an existing hist\n"
"\ttrigger or to start a hist trigger but not log any events\n"
"\tuntil told to do so.  'continue' can be used to start or\n"
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index e6e5c6e..656973a 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -75,6 +75,7 @@ enum hist_field_flags {
HIST_FIELD_HEX  = 8,
HIST_FIELD_SYM  = 16,
HIST_FIELD_SYM_OFFSET   = 32,
+   HIST_FIELD_EXECNAME = 64,
 };
 
 struct hist_trigger_attrs {
@@ -218,6 +219,78 @@ static struct hist_trigger_attrs 
*parse_hist_trigger_attrs(char *trigger_str)
return ERR_PTR(ret);
 }
 
+static inline void save_comm(char *comm, struct task_struct *task)
+{
+   if (!task->pid) {
+   strcpy(comm, "");
+   return;
+   }
+
+   if (WARN_ON_ONCE(task->pid < 0)) {
+   strcpy(comm, "");
+   return;
+   }
+
+   if (task->pid > PID_MAX_DEFAULT) {
+   strcpy(comm, "<...>");
+   return;
+   }
+
+   memcpy(comm, task->comm, TASK_COMM_LEN);
+}
+
+static void hist_trigger_elt_free(struct tracing_map_elt *elt)
+{
+   kfree((char *)elt->private_data);
+}
+
+static int hist_trigger_elt_alloc(struct tracing_map_elt *elt)
+{
+   struct hist_trigger_data *hist_data = elt->map->private_data;
+   struct hist_field *key_field;
+   unsigned int i;
+
+   for (i = hist_data->n_vals; i < hist_data->n_fields; i++) {
+   key_field = hist_data->fields[i];
+
+   if (key_field->flags & HIST_FIELD_EXECNAME) {
+   unsigned int size = TASK_COMM_LEN + 1;
+
+   elt->private_data = kzalloc(size, GFP_KERNEL);
+   if (!elt->private_data)
+   return -ENOMEM;
+   break;
+   }
+   }
+
+   return 0;
+}
+
+static void hist_trigger_elt_copy(struct tracing_map_elt *to,
+ struct tracing_map_elt *from)
+{
+   char *comm_from = from->private_data;
+   char *comm_to = to->private_data;
+
+   if (comm_from)
+   memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
+}
+
+static void hist_trigger_elt_init(struct tracing_map_elt *elt)
+{
+   char *comm = elt->private_data;
+
+   if (comm)
+   save_comm(comm, current);
+}
+
+static struct tracing_map_ops hist_trigger_ops = {
+   .elt_alloc  = hist_trigger_elt_alloc,
+   .elt_copy   = hist_trigger_elt_copy,
+   .elt_free   = hist_trigger_elt_free,
+   .elt_init   = hist_trigger_elt_init,
+};
+
 static void destroy_hist_field(struct hist_field *hist_field)
 {
kfree(hist_field);
@@ -376,6 +449,9 @@ static int create_key_field(struct hist_trigger_data 
*hist_data,
flags |= HIST_FIELD_SYM;
else if (!strcmp(field_str, "sym-offset"))
flags |= HIST_FIELD_SYM_OFFSET;
+   else if (!strcmp(field_str, "execname") &&
+!strcmp(field_name, "common_pid"))
+   flags |= HIST_FIELD_EXECNAME;
else {
ret = -EINVAL;
goto out;
@@ -612,7 +688,7 @@ create_hist_data(unsigned int map_bits,
goto free;
 
hist_data->map = tracing_map_create(map_bits, hist_data->key_size,
-   NULL, hist_data);
+   _trigger_ops, hist_data);
if (IS_ERR(hist_data->map)) {
ret = PTR_ERR(hist_data->map);
hist_data->map = NULL;
@@ -732,6 +808,12 @@