Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

On Tue, 15 Nov 2016 16:38:19 -0500
Steven Rostedt  wrote:

> On Fri, 28 Oct 2016 17:14:56 -0400
> Luiz Capitulino  wrote:
> 
> 
> > > 
> > > cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
> > > from below.
> > > 
> > > What happens if this range is greater than 64? We already have boxes
> > > that run this with 80 CPUs. Looks to be out of memory range to me.
> > 
> > The best solution is probably to detect the number of CPUs at run-time
> > and use the CPU_SET() API. The lazy and ugly solution is to just fail.
> > 
> > Any objections to the CPU_SET() solution?
> >   
> 
> I thought I replied to this, but I'm guessing the reply is still
> pending.
> 
> I have no objecting. I'd like to see a patch though.

I imagined you didn't have objections :) I haven't started working
on this yet because I'm very busy at other stuff. But I'll go
back to this soon.

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

On Tue, 15 Nov 2016 16:38:19 -0500
Steven Rostedt  wrote:

> On Fri, 28 Oct 2016 17:14:56 -0400
> Luiz Capitulino  wrote:
> 
> 
> > > 
> > > cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
> > > from below.
> > > 
> > > What happens if this range is greater than 64? We already have boxes
> > > that run this with 80 CPUs. Looks to be out of memory range to me.
> > 
> > The best solution is probably to detect the number of CPUs at run-time
> > and use the CPU_SET() API. The lazy and ugly solution is to just fail.
> > 
> > Any objections to the CPU_SET() solution?
> >   
> 
> I thought I replied to this, but I'm guessing the reply is still
> pending.
> 
> I have no objecting. I'd like to see a patch though.

I imagined you didn't have objections :) I haven't started working
on this yet because I'm very busy at other stuff. But I'll go
back to this soon.

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

On Fri, 28 Oct 2016 17:14:56 -0400
Luiz Capitulino  wrote:


> > 
> > cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
> > from below.
> > 
> > What happens if this range is greater than 64? We already have boxes
> > that run this with 80 CPUs. Looks to be out of memory range to me.  
> 
> The best solution is probably to detect the number of CPUs at run-time
> and use the CPU_SET() API. The lazy and ugly solution is to just fail.
> 
> Any objections to the CPU_SET() solution?
> 

I thought I replied to this, but I'm guessing the reply is still
pending.

I have no objecting. I'd like to see a patch though.

Thanks,

-- Steve

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

On Fri, 28 Oct 2016 17:14:56 -0400
Luiz Capitulino  wrote:


> > 
> > cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
> > from below.
> > 
> > What happens if this range is greater than 64? We already have boxes
> > that run this with 80 CPUs. Looks to be out of memory range to me.  
> 
> The best solution is probably to detect the number of CPUs at run-time
> and use the CPU_SET() API. The lazy and ugly solution is to just fail.
> 
> Any objections to the CPU_SET() solution?
> 

I thought I replied to this, but I'm guessing the reply is still
pending.

I have no objecting. I'd like to see a patch though.

Thanks,

-- Steve

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

On Fri, 28 Oct 2016 17:15:57 -0400
Luiz Capitulino  wrote:

> On Fri, 28 Oct 2016 15:50:10 -0400
> Steven Rostedt  wrote:
> 
> > On Fri, 28 Oct 2016 15:49:22 -0400
> > Steven Rostedt  wrote:
> > 
> > > Sorry it took so long to look at this, but I finally got around to
> > > it ;-)
> > >   
> > 
> > I did apply patches 1 and 2. I'm hoping to get them out tomorrow, or
> > while I'm at plumbers.
> 
> OK, I also posted a minor fix:
> 
>  [PATCH] trace-cmd: Documentation: ignore all man sections

Yep, I included that one already.

Thanks!

-- Steve

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

On Fri, 28 Oct 2016 17:15:57 -0400
Luiz Capitulino  wrote:

> On Fri, 28 Oct 2016 15:50:10 -0400
> Steven Rostedt  wrote:
> 
> > On Fri, 28 Oct 2016 15:49:22 -0400
> > Steven Rostedt  wrote:
> > 
> > > Sorry it took so long to look at this, but I finally got around to
> > > it ;-)
> > >   
> > 
> > I did apply patches 1 and 2. I'm hoping to get them out tomorrow, or
> > while I'm at plumbers.
> 
> OK, I also posted a minor fix:
> 
>  [PATCH] trace-cmd: Documentation: ignore all man sections

Yep, I included that one already.

Thanks!

-- Steve

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

On Fri, 28 Oct 2016 15:50:10 -0400
Steven Rostedt  wrote:

> On Fri, 28 Oct 2016 15:49:22 -0400
> Steven Rostedt  wrote:
> 
> > Sorry it took so long to look at this, but I finally got around to
> > it ;-)
> >   
> 
> I did apply patches 1 and 2. I'm hoping to get them out tomorrow, or
> while I'm at plumbers.

OK, I also posted a minor fix:

 [PATCH] trace-cmd: Documentation: ignore all man sections

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

On Fri, 28 Oct 2016 15:50:10 -0400
Steven Rostedt  wrote:

> On Fri, 28 Oct 2016 15:49:22 -0400
> Steven Rostedt  wrote:
> 
> > Sorry it took so long to look at this, but I finally got around to
> > it ;-)
> >   
> 
> I did apply patches 1 and 2. I'm hoping to get them out tomorrow, or
> while I'm at plumbers.

OK, I also posted a minor fix:

 [PATCH] trace-cmd: Documentation: ignore all man sections

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

On Fri, 28 Oct 2016 15:49:22 -0400
Steven Rostedt  wrote:

> Sorry it took so long to look at this, but I finally got around to
> it ;-)
> 
> 
> On Fri,  7 Oct 2016 12:47:11 -0400
> Luiz Capitulino  wrote:
> 
> > With --cpu-list you can do:
> > 
> >   # trace-cmd record --cpu-list 1,4,10-15 [...]
> > 
> > Which is much more human friendly than -M.
> > 
> > Signed-off-by: Luiz Capitulino 
> > ---
> >  Documentation/trace-cmd-record.1.txt |   4 +
> >  trace-record.c   | 138 
> > +++
> >  2 files changed, 142 insertions(+)
> > 
> > diff --git a/Documentation/trace-cmd-record.1.txt 
> > b/Documentation/trace-cmd-record.1.txt
> > index b80520e..d7e806a 100644
> > --- a/Documentation/trace-cmd-record.1.txt
> > +++ b/Documentation/trace-cmd-record.1.txt
> > @@ -304,6 +304,10 @@ OPTIONS
> >  executed will not be changed. This is useful if you want to monitor the
> >  output of the command being executed, but not see the output from 
> > trace-cmd.
> >  
> > +*--cpu-list list*::
> > +List of CPUs to be traced. The "list" argument can be comma separated
> > +(eg. 1,2,4,5), a range (eg. 1-10) or a mix of the two (eg. 1,2,10-15).
> > +
> >  EXAMPLES
> >  
> >  
> > diff --git a/trace-record.c b/trace-record.c
> > index b042993..4452979 100644
> > --- a/trace-record.c
> > +++ b/trace-record.c
> > @@ -23,6 +23,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -47,6 +48,7 @@
> >  
> >  #include "trace-local.h"
> >  #include "trace-msg.h"
> > +#include "cpu.h"
> >  
> >  #define _STR(x) #x
> >  #define STR(x) _STR(x)
> > @@ -179,6 +181,118 @@ static struct tracecmd_recorder *recorder;
> >  
> >  static int ignore_event_not_found = 0;
> >  
> > +static int read_cpu_nr(const char *str, int *ret)
> > +{
> > +   char *endptr;
> > +   int val;
> > +
> > +   errno = 0;
> > +   val = strtol(str, , 10);
> > +
> > +   if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
> > +   || (errno != 0 && val == 0))
> > +   return -1;
> > +
> > +   if (endptr == str)
> > +   return -1;
> > +
> > +   if (*endptr != '\0')
> > +   return -1;
> > +
> > +   *ret = val;
> > +   return 0;
> > +}
> > +
> > +static int set_single_cpu(const char *str, uint64_t *ret_mask)
> > +{
> > +   int cpu, err;
> > +
> > +   err = read_cpu_nr(str, );
> > +   if (err || cpu < 0)
> > +   return -1;
> > +
> > +   cpu_set(ret_mask, cpu);
> > +   return 0;
> > +}
> > +
> > +static int parse_one(char *str, char **saveptr)
> > +{
> > +   int err, cpu;
> > +   char *p;
> > +
> > +   p = strtok_r(str, "-", saveptr);
> > +   if (!p)
> > +   return -1;
> > +
> > +   err = read_cpu_nr(p, );
> > +   if (err)
> > +   return -1;
> > +
> > +   return cpu;
> > +}
> > +
> > +static int set_range_cpu(const char *str, uint64_t *ret_mask)  
> 
> ret_mask is a reference to uint64_t cpumask below.
> 
> > +{
> > +   int i, begin, end, err = -1;
> > +   char *saveptr, *range;
> > +
> > +   range = strdup(str);
> > +   if (!range)
> > +   return -1;
> > +
> > +   begin = parse_one(range, );
> > +   if (begin < 0)
> > +   goto out;
> > +
> > +   end = parse_one(NULL, );
> > +   if (begin < 0)
> > +   goto out;
> > +
> > +   if (begin > end)
> > +   goto out;
> > +
> > +   for (i = begin; i <= end; i++)
> > +   cpu_set(ret_mask, i);  
> 
> cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
> from below.
> 
> What happens if this range is greater than 64? We already have boxes
> that run this with 80 CPUs. Looks to be out of memory range to me.

The best solution is probably to detect the number of CPUs at run-time
and use the CPU_SET() API. The lazy and ugly solution is to just fail.

Any objections to the CPU_SET() solution?

> 
> > +
> > +   err = 0;
> > +
> > +out:
> > +   free(range);
> > +   return err;
> > +}
> > +
> > +static int has_range(const char *str)
> > +{
> > +   return strchr(str, '-') != NULL;
> > +}
> > +
> > +static int parse_cpumask_str(const char *cpumask_str, uint64_t *ret_mask)  
> 
> ret_mask is a reference to uint64_t cpumask below.
> 
> > +{
> > +   char *saveptr, *str, *p;
> > +   int err = 0;
> > +
> > +   str = strdup(cpumask_str);
> > +   if (!str)
> > +   return -1;
> > +
> > +   *ret_mask = 0;
> > +
> > +   p = strtok_r(str, ",", );
> > +   while (p) {
> > +   if (has_range(p))
> > +   err = set_range_cpu(p, ret_mask);  
> 
> Passing the reference of uint64_t cpumask from below to set_range_cpu()
> 
> > +   else
> > +   err = set_single_cpu(p, ret_mask);
> > +   if (err)
> > +   goto out;
> > +   p = strtok_r(NULL, ",", );
> > +   }
> > +
> > +out:
> > +   free(str);
> > +   return err;
> > +}
> > +
> >  static inline 

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

On Fri, 28 Oct 2016 15:49:22 -0400
Steven Rostedt  wrote:

> Sorry it took so long to look at this, but I finally got around to
> it ;-)
> 
> 
> On Fri,  7 Oct 2016 12:47:11 -0400
> Luiz Capitulino  wrote:
> 
> > With --cpu-list you can do:
> > 
> >   # trace-cmd record --cpu-list 1,4,10-15 [...]
> > 
> > Which is much more human friendly than -M.
> > 
> > Signed-off-by: Luiz Capitulino 
> > ---
> >  Documentation/trace-cmd-record.1.txt |   4 +
> >  trace-record.c   | 138 
> > +++
> >  2 files changed, 142 insertions(+)
> > 
> > diff --git a/Documentation/trace-cmd-record.1.txt 
> > b/Documentation/trace-cmd-record.1.txt
> > index b80520e..d7e806a 100644
> > --- a/Documentation/trace-cmd-record.1.txt
> > +++ b/Documentation/trace-cmd-record.1.txt
> > @@ -304,6 +304,10 @@ OPTIONS
> >  executed will not be changed. This is useful if you want to monitor the
> >  output of the command being executed, but not see the output from 
> > trace-cmd.
> >  
> > +*--cpu-list list*::
> > +List of CPUs to be traced. The "list" argument can be comma separated
> > +(eg. 1,2,4,5), a range (eg. 1-10) or a mix of the two (eg. 1,2,10-15).
> > +
> >  EXAMPLES
> >  
> >  
> > diff --git a/trace-record.c b/trace-record.c
> > index b042993..4452979 100644
> > --- a/trace-record.c
> > +++ b/trace-record.c
> > @@ -23,6 +23,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -47,6 +48,7 @@
> >  
> >  #include "trace-local.h"
> >  #include "trace-msg.h"
> > +#include "cpu.h"
> >  
> >  #define _STR(x) #x
> >  #define STR(x) _STR(x)
> > @@ -179,6 +181,118 @@ static struct tracecmd_recorder *recorder;
> >  
> >  static int ignore_event_not_found = 0;
> >  
> > +static int read_cpu_nr(const char *str, int *ret)
> > +{
> > +   char *endptr;
> > +   int val;
> > +
> > +   errno = 0;
> > +   val = strtol(str, , 10);
> > +
> > +   if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
> > +   || (errno != 0 && val == 0))
> > +   return -1;
> > +
> > +   if (endptr == str)
> > +   return -1;
> > +
> > +   if (*endptr != '\0')
> > +   return -1;
> > +
> > +   *ret = val;
> > +   return 0;
> > +}
> > +
> > +static int set_single_cpu(const char *str, uint64_t *ret_mask)
> > +{
> > +   int cpu, err;
> > +
> > +   err = read_cpu_nr(str, );
> > +   if (err || cpu < 0)
> > +   return -1;
> > +
> > +   cpu_set(ret_mask, cpu);
> > +   return 0;
> > +}
> > +
> > +static int parse_one(char *str, char **saveptr)
> > +{
> > +   int err, cpu;
> > +   char *p;
> > +
> > +   p = strtok_r(str, "-", saveptr);
> > +   if (!p)
> > +   return -1;
> > +
> > +   err = read_cpu_nr(p, );
> > +   if (err)
> > +   return -1;
> > +
> > +   return cpu;
> > +}
> > +
> > +static int set_range_cpu(const char *str, uint64_t *ret_mask)  
> 
> ret_mask is a reference to uint64_t cpumask below.
> 
> > +{
> > +   int i, begin, end, err = -1;
> > +   char *saveptr, *range;
> > +
> > +   range = strdup(str);
> > +   if (!range)
> > +   return -1;
> > +
> > +   begin = parse_one(range, );
> > +   if (begin < 0)
> > +   goto out;
> > +
> > +   end = parse_one(NULL, );
> > +   if (begin < 0)
> > +   goto out;
> > +
> > +   if (begin > end)
> > +   goto out;
> > +
> > +   for (i = begin; i <= end; i++)
> > +   cpu_set(ret_mask, i);  
> 
> cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
> from below.
> 
> What happens if this range is greater than 64? We already have boxes
> that run this with 80 CPUs. Looks to be out of memory range to me.

The best solution is probably to detect the number of CPUs at run-time
and use the CPU_SET() API. The lazy and ugly solution is to just fail.

Any objections to the CPU_SET() solution?

> 
> > +
> > +   err = 0;
> > +
> > +out:
> > +   free(range);
> > +   return err;
> > +}
> > +
> > +static int has_range(const char *str)
> > +{
> > +   return strchr(str, '-') != NULL;
> > +}
> > +
> > +static int parse_cpumask_str(const char *cpumask_str, uint64_t *ret_mask)  
> 
> ret_mask is a reference to uint64_t cpumask below.
> 
> > +{
> > +   char *saveptr, *str, *p;
> > +   int err = 0;
> > +
> > +   str = strdup(cpumask_str);
> > +   if (!str)
> > +   return -1;
> > +
> > +   *ret_mask = 0;
> > +
> > +   p = strtok_r(str, ",", );
> > +   while (p) {
> > +   if (has_range(p))
> > +   err = set_range_cpu(p, ret_mask);  
> 
> Passing the reference of uint64_t cpumask from below to set_range_cpu()
> 
> > +   else
> > +   err = set_single_cpu(p, ret_mask);
> > +   if (err)
> > +   goto out;
> > +   p = strtok_r(NULL, ",", );
> > +   }
> > +
> > +out:
> > +   free(str);
> > +   return err;
> > +}
> > +
> >  static inline int is_top_instance(struct buffer_instance *instance)
> >  {
> > 

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

On Fri, 28 Oct 2016 15:49:22 -0400
Steven Rostedt  wrote:

> Sorry it took so long to look at this, but I finally got around to
> it ;-)
> 

I did apply patches 1 and 2. I'm hoping to get them out tomorrow, or
while I'm at plumbers.

-- Steve

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

On Fri, 28 Oct 2016 15:49:22 -0400
Steven Rostedt  wrote:

> Sorry it took so long to look at this, but I finally got around to
> it ;-)
> 

I did apply patches 1 and 2. I'm hoping to get them out tomorrow, or
while I'm at plumbers.

-- Steve

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

Sorry it took so long to look at this, but I finally got around to
it ;-)


On Fri,  7 Oct 2016 12:47:11 -0400
Luiz Capitulino  wrote:

> With --cpu-list you can do:
> 
>   # trace-cmd record --cpu-list 1,4,10-15 [...]
> 
> Which is much more human friendly than -M.
> 
> Signed-off-by: Luiz Capitulino 
> ---
>  Documentation/trace-cmd-record.1.txt |   4 +
>  trace-record.c   | 138 
> +++
>  2 files changed, 142 insertions(+)
> 
> diff --git a/Documentation/trace-cmd-record.1.txt 
> b/Documentation/trace-cmd-record.1.txt
> index b80520e..d7e806a 100644
> --- a/Documentation/trace-cmd-record.1.txt
> +++ b/Documentation/trace-cmd-record.1.txt
> @@ -304,6 +304,10 @@ OPTIONS
>  executed will not be changed. This is useful if you want to monitor the
>  output of the command being executed, but not see the output from 
> trace-cmd.
>  
> +*--cpu-list list*::
> +List of CPUs to be traced. The "list" argument can be comma separated
> +(eg. 1,2,4,5), a range (eg. 1-10) or a mix of the two (eg. 1,2,10-15).
> +
>  EXAMPLES
>  
>  
> diff --git a/trace-record.c b/trace-record.c
> index b042993..4452979 100644
> --- a/trace-record.c
> +++ b/trace-record.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -47,6 +48,7 @@
>  
>  #include "trace-local.h"
>  #include "trace-msg.h"
> +#include "cpu.h"
>  
>  #define _STR(x) #x
>  #define STR(x) _STR(x)
> @@ -179,6 +181,118 @@ static struct tracecmd_recorder *recorder;
>  
>  static int ignore_event_not_found = 0;
>  
> +static int read_cpu_nr(const char *str, int *ret)
> +{
> + char *endptr;
> + int val;
> +
> + errno = 0;
> + val = strtol(str, , 10);
> +
> + if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
> + || (errno != 0 && val == 0))
> + return -1;
> +
> + if (endptr == str)
> + return -1;
> +
> + if (*endptr != '\0')
> + return -1;
> +
> + *ret = val;
> + return 0;
> +}
> +
> +static int set_single_cpu(const char *str, uint64_t *ret_mask)
> +{
> + int cpu, err;
> +
> + err = read_cpu_nr(str, );
> + if (err || cpu < 0)
> + return -1;
> +
> + cpu_set(ret_mask, cpu);
> + return 0;
> +}
> +
> +static int parse_one(char *str, char **saveptr)
> +{
> + int err, cpu;
> + char *p;
> +
> + p = strtok_r(str, "-", saveptr);
> + if (!p)
> + return -1;
> +
> + err = read_cpu_nr(p, );
> + if (err)
> + return -1;
> +
> + return cpu;
> +}
> +
> +static int set_range_cpu(const char *str, uint64_t *ret_mask)

ret_mask is a reference to uint64_t cpumask below.

> +{
> + int i, begin, end, err = -1;
> + char *saveptr, *range;
> +
> + range = strdup(str);
> + if (!range)
> + return -1;
> +
> + begin = parse_one(range, );
> + if (begin < 0)
> + goto out;
> +
> + end = parse_one(NULL, );
> + if (begin < 0)
> + goto out;
> +
> + if (begin > end)
> + goto out;
> +
> + for (i = begin; i <= end; i++)
> + cpu_set(ret_mask, i);

cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
from below.

What happens if this range is greater than 64? We already have boxes
that run this with 80 CPUs. Looks to be out of memory range to me.

> +
> + err = 0;
> +
> +out:
> + free(range);
> + return err;
> +}
> +
> +static int has_range(const char *str)
> +{
> + return strchr(str, '-') != NULL;
> +}
> +
> +static int parse_cpumask_str(const char *cpumask_str, uint64_t *ret_mask)

ret_mask is a reference to uint64_t cpumask below.

> +{
> + char *saveptr, *str, *p;
> + int err = 0;
> +
> + str = strdup(cpumask_str);
> + if (!str)
> + return -1;
> +
> + *ret_mask = 0;
> +
> + p = strtok_r(str, ",", );
> + while (p) {
> + if (has_range(p))
> + err = set_range_cpu(p, ret_mask);

Passing the reference of uint64_t cpumask from below to set_range_cpu()

> + else
> + err = set_single_cpu(p, ret_mask);
> + if (err)
> + goto out;
> + p = strtok_r(NULL, ",", );
> + }
> +
> +out:
> + free(str);
> + return err;
> +}
> +
>  static inline int is_top_instance(struct buffer_instance *instance)
>  {
>   return instance == _instance;
> @@ -2114,6 +2228,25 @@ static char *alloc_mask_from_hex(const char *str)
>   return cpumask;
>  }
>  
> +static char *alloc_mask_from_list(const char *cpu_list)
> +{
> + char *cpumask_str;
> + uint64_t cpumask;

cpumask is a simple uint64_t (64 bits)

> + int ret;
> +
> + ret = parse_cpumask_str(cpu_list, );

Passed to parse_cpumask_str() by reference.

-- Steve

> + if (ret < 0)
> + 

Re: [PATCH 3/3] trace-cmd record: add --cpu-list option

[Steven Rostedt]

Sorry it took so long to look at this, but I finally got around to
it ;-)


On Fri,  7 Oct 2016 12:47:11 -0400
Luiz Capitulino  wrote:

> With --cpu-list you can do:
> 
>   # trace-cmd record --cpu-list 1,4,10-15 [...]
> 
> Which is much more human friendly than -M.
> 
> Signed-off-by: Luiz Capitulino 
> ---
>  Documentation/trace-cmd-record.1.txt |   4 +
>  trace-record.c   | 138 
> +++
>  2 files changed, 142 insertions(+)
> 
> diff --git a/Documentation/trace-cmd-record.1.txt 
> b/Documentation/trace-cmd-record.1.txt
> index b80520e..d7e806a 100644
> --- a/Documentation/trace-cmd-record.1.txt
> +++ b/Documentation/trace-cmd-record.1.txt
> @@ -304,6 +304,10 @@ OPTIONS
>  executed will not be changed. This is useful if you want to monitor the
>  output of the command being executed, but not see the output from 
> trace-cmd.
>  
> +*--cpu-list list*::
> +List of CPUs to be traced. The "list" argument can be comma separated
> +(eg. 1,2,4,5), a range (eg. 1-10) or a mix of the two (eg. 1,2,10-15).
> +
>  EXAMPLES
>  
>  
> diff --git a/trace-record.c b/trace-record.c
> index b042993..4452979 100644
> --- a/trace-record.c
> +++ b/trace-record.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -47,6 +48,7 @@
>  
>  #include "trace-local.h"
>  #include "trace-msg.h"
> +#include "cpu.h"
>  
>  #define _STR(x) #x
>  #define STR(x) _STR(x)
> @@ -179,6 +181,118 @@ static struct tracecmd_recorder *recorder;
>  
>  static int ignore_event_not_found = 0;
>  
> +static int read_cpu_nr(const char *str, int *ret)
> +{
> + char *endptr;
> + int val;
> +
> + errno = 0;
> + val = strtol(str, , 10);
> +
> + if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
> + || (errno != 0 && val == 0))
> + return -1;
> +
> + if (endptr == str)
> + return -1;
> +
> + if (*endptr != '\0')
> + return -1;
> +
> + *ret = val;
> + return 0;
> +}
> +
> +static int set_single_cpu(const char *str, uint64_t *ret_mask)
> +{
> + int cpu, err;
> +
> + err = read_cpu_nr(str, );
> + if (err || cpu < 0)
> + return -1;
> +
> + cpu_set(ret_mask, cpu);
> + return 0;
> +}
> +
> +static int parse_one(char *str, char **saveptr)
> +{
> + int err, cpu;
> + char *p;
> +
> + p = strtok_r(str, "-", saveptr);
> + if (!p)
> + return -1;
> +
> + err = read_cpu_nr(p, );
> + if (err)
> + return -1;
> +
> + return cpu;
> +}
> +
> +static int set_range_cpu(const char *str, uint64_t *ret_mask)

ret_mask is a reference to uint64_t cpumask below.

> +{
> + int i, begin, end, err = -1;
> + char *saveptr, *range;
> +
> + range = strdup(str);
> + if (!range)
> + return -1;
> +
> + begin = parse_one(range, );
> + if (begin < 0)
> + goto out;
> +
> + end = parse_one(NULL, );
> + if (begin < 0)
> + goto out;
> +
> + if (begin > end)
> + goto out;
> +
> + for (i = begin; i <= end; i++)
> + cpu_set(ret_mask, i);

cpu_set(ret_mask, i) sets bits from begin to end in uint64_t cpumask
from below.

What happens if this range is greater than 64? We already have boxes
that run this with 80 CPUs. Looks to be out of memory range to me.

> +
> + err = 0;
> +
> +out:
> + free(range);
> + return err;
> +}
> +
> +static int has_range(const char *str)
> +{
> + return strchr(str, '-') != NULL;
> +}
> +
> +static int parse_cpumask_str(const char *cpumask_str, uint64_t *ret_mask)

ret_mask is a reference to uint64_t cpumask below.

> +{
> + char *saveptr, *str, *p;
> + int err = 0;
> +
> + str = strdup(cpumask_str);
> + if (!str)
> + return -1;
> +
> + *ret_mask = 0;
> +
> + p = strtok_r(str, ",", );
> + while (p) {
> + if (has_range(p))
> + err = set_range_cpu(p, ret_mask);

Passing the reference of uint64_t cpumask from below to set_range_cpu()

> + else
> + err = set_single_cpu(p, ret_mask);
> + if (err)
> + goto out;
> + p = strtok_r(NULL, ",", );
> + }
> +
> +out:
> + free(str);
> + return err;
> +}
> +
>  static inline int is_top_instance(struct buffer_instance *instance)
>  {
>   return instance == _instance;
> @@ -2114,6 +2228,25 @@ static char *alloc_mask_from_hex(const char *str)
>   return cpumask;
>  }
>  
> +static char *alloc_mask_from_list(const char *cpu_list)
> +{
> + char *cpumask_str;
> + uint64_t cpumask;

cpumask is a simple uint64_t (64 bits)

> + int ret;
> +
> + ret = parse_cpumask_str(cpu_list, );

Passed to parse_cpumask_str() by reference.

-- Steve

> + if (ret < 0)
> + die("can't parse cpumask");
> +
> + cpumask_str 

[PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

With --cpu-list you can do:

  # trace-cmd record --cpu-list 1,4,10-15 [...]

Which is much more human friendly than -M.

Signed-off-by: Luiz Capitulino 
---
 Documentation/trace-cmd-record.1.txt |   4 +
 trace-record.c   | 138 +++
 2 files changed, 142 insertions(+)

diff --git a/Documentation/trace-cmd-record.1.txt 
b/Documentation/trace-cmd-record.1.txt
index b80520e..d7e806a 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -304,6 +304,10 @@ OPTIONS
 executed will not be changed. This is useful if you want to monitor the
 output of the command being executed, but not see the output from 
trace-cmd.
 
+*--cpu-list list*::
+List of CPUs to be traced. The "list" argument can be comma separated
+(eg. 1,2,4,5), a range (eg. 1-10) or a mix of the two (eg. 1,2,10-15).
+
 EXAMPLES
 
 
diff --git a/trace-record.c b/trace-record.c
index b042993..4452979 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -47,6 +48,7 @@
 
 #include "trace-local.h"
 #include "trace-msg.h"
+#include "cpu.h"
 
 #define _STR(x) #x
 #define STR(x) _STR(x)
@@ -179,6 +181,118 @@ static struct tracecmd_recorder *recorder;
 
 static int ignore_event_not_found = 0;
 
+static int read_cpu_nr(const char *str, int *ret)
+{
+   char *endptr;
+   int val;
+
+   errno = 0;
+   val = strtol(str, , 10);
+
+   if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
+   || (errno != 0 && val == 0))
+   return -1;
+
+   if (endptr == str)
+   return -1;
+
+   if (*endptr != '\0')
+   return -1;
+
+   *ret = val;
+   return 0;
+}
+
+static int set_single_cpu(const char *str, uint64_t *ret_mask)
+{
+   int cpu, err;
+
+   err = read_cpu_nr(str, );
+   if (err || cpu < 0)
+   return -1;
+
+   cpu_set(ret_mask, cpu);
+   return 0;
+}
+
+static int parse_one(char *str, char **saveptr)
+{
+   int err, cpu;
+   char *p;
+
+   p = strtok_r(str, "-", saveptr);
+   if (!p)
+   return -1;
+
+   err = read_cpu_nr(p, );
+   if (err)
+   return -1;
+
+   return cpu;
+}
+
+static int set_range_cpu(const char *str, uint64_t *ret_mask)
+{
+   int i, begin, end, err = -1;
+   char *saveptr, *range;
+
+   range = strdup(str);
+   if (!range)
+   return -1;
+
+   begin = parse_one(range, );
+   if (begin < 0)
+   goto out;
+
+   end = parse_one(NULL, );
+   if (begin < 0)
+   goto out;
+
+   if (begin > end)
+   goto out;
+
+   for (i = begin; i <= end; i++)
+   cpu_set(ret_mask, i);
+
+   err = 0;
+
+out:
+   free(range);
+   return err;
+}
+
+static int has_range(const char *str)
+{
+   return strchr(str, '-') != NULL;
+}
+
+static int parse_cpumask_str(const char *cpumask_str, uint64_t *ret_mask)
+{
+   char *saveptr, *str, *p;
+   int err = 0;
+
+   str = strdup(cpumask_str);
+   if (!str)
+   return -1;
+
+   *ret_mask = 0;
+
+   p = strtok_r(str, ",", );
+   while (p) {
+   if (has_range(p))
+   err = set_range_cpu(p, ret_mask);
+   else
+   err = set_single_cpu(p, ret_mask);
+   if (err)
+   goto out;
+   p = strtok_r(NULL, ",", );
+   }
+
+out:
+   free(str);
+   return err;
+}
+
 static inline int is_top_instance(struct buffer_instance *instance)
 {
return instance == _instance;
@@ -2114,6 +2228,25 @@ static char *alloc_mask_from_hex(const char *str)
return cpumask;
 }
 
+static char *alloc_mask_from_list(const char *cpu_list)
+{
+   char *cpumask_str;
+   uint64_t cpumask;
+   int ret;
+
+   ret = parse_cpumask_str(cpu_list, );
+   if (ret < 0)
+   die("can't parse cpumask");
+
+   cpumask_str = malloc(MASK_STR_MAX);
+   if (!cpumask_str)
+   die("can't allocate cpumask");
+
+   snprintf(cpumask_str, MASK_STR_MAX-1, "%lx", cpumask);
+   return cpumask_str;
+}
+
+
 static void set_mask(struct buffer_instance *instance)
 {
struct stat st;
@@ -4136,6 +4269,7 @@ enum {
OPT_nosplice= 253,
OPT_funcstack   = 254,
OPT_date= 255,
+   OPT_cpulist = 256,
 };
 
 void trace_record (int argc, char **argv)
@@ -4337,6 +4471,7 @@ void trace_record (int argc, char **argv)
{"stderr", no_argument, NULL, OPT_stderr},
{"by-comm", no_argument, NULL, OPT_bycomm},
{"ts-offset", required_argument, NULL, OPT_tsoffset},
+   

[PATCH 3/3] trace-cmd record: add --cpu-list option

[Luiz Capitulino]

With --cpu-list you can do:

  # trace-cmd record --cpu-list 1,4,10-15 [...]

Which is much more human friendly than -M.

Signed-off-by: Luiz Capitulino 
---
 Documentation/trace-cmd-record.1.txt |   4 +
 trace-record.c   | 138 +++
 2 files changed, 142 insertions(+)

diff --git a/Documentation/trace-cmd-record.1.txt 
b/Documentation/trace-cmd-record.1.txt
index b80520e..d7e806a 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -304,6 +304,10 @@ OPTIONS
 executed will not be changed. This is useful if you want to monitor the
 output of the command being executed, but not see the output from 
trace-cmd.
 
+*--cpu-list list*::
+List of CPUs to be traced. The "list" argument can be comma separated
+(eg. 1,2,4,5), a range (eg. 1-10) or a mix of the two (eg. 1,2,10-15).
+
 EXAMPLES
 
 
diff --git a/trace-record.c b/trace-record.c
index b042993..4452979 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -47,6 +48,7 @@
 
 #include "trace-local.h"
 #include "trace-msg.h"
+#include "cpu.h"
 
 #define _STR(x) #x
 #define STR(x) _STR(x)
@@ -179,6 +181,118 @@ static struct tracecmd_recorder *recorder;
 
 static int ignore_event_not_found = 0;
 
+static int read_cpu_nr(const char *str, int *ret)
+{
+   char *endptr;
+   int val;
+
+   errno = 0;
+   val = strtol(str, , 10);
+
+   if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
+   || (errno != 0 && val == 0))
+   return -1;
+
+   if (endptr == str)
+   return -1;
+
+   if (*endptr != '\0')
+   return -1;
+
+   *ret = val;
+   return 0;
+}
+
+static int set_single_cpu(const char *str, uint64_t *ret_mask)
+{
+   int cpu, err;
+
+   err = read_cpu_nr(str, );
+   if (err || cpu < 0)
+   return -1;
+
+   cpu_set(ret_mask, cpu);
+   return 0;
+}
+
+static int parse_one(char *str, char **saveptr)
+{
+   int err, cpu;
+   char *p;
+
+   p = strtok_r(str, "-", saveptr);
+   if (!p)
+   return -1;
+
+   err = read_cpu_nr(p, );
+   if (err)
+   return -1;
+
+   return cpu;
+}
+
+static int set_range_cpu(const char *str, uint64_t *ret_mask)
+{
+   int i, begin, end, err = -1;
+   char *saveptr, *range;
+
+   range = strdup(str);
+   if (!range)
+   return -1;
+
+   begin = parse_one(range, );
+   if (begin < 0)
+   goto out;
+
+   end = parse_one(NULL, );
+   if (begin < 0)
+   goto out;
+
+   if (begin > end)
+   goto out;
+
+   for (i = begin; i <= end; i++)
+   cpu_set(ret_mask, i);
+
+   err = 0;
+
+out:
+   free(range);
+   return err;
+}
+
+static int has_range(const char *str)
+{
+   return strchr(str, '-') != NULL;
+}
+
+static int parse_cpumask_str(const char *cpumask_str, uint64_t *ret_mask)
+{
+   char *saveptr, *str, *p;
+   int err = 0;
+
+   str = strdup(cpumask_str);
+   if (!str)
+   return -1;
+
+   *ret_mask = 0;
+
+   p = strtok_r(str, ",", );
+   while (p) {
+   if (has_range(p))
+   err = set_range_cpu(p, ret_mask);
+   else
+   err = set_single_cpu(p, ret_mask);
+   if (err)
+   goto out;
+   p = strtok_r(NULL, ",", );
+   }
+
+out:
+   free(str);
+   return err;
+}
+
 static inline int is_top_instance(struct buffer_instance *instance)
 {
return instance == _instance;
@@ -2114,6 +2228,25 @@ static char *alloc_mask_from_hex(const char *str)
return cpumask;
 }
 
+static char *alloc_mask_from_list(const char *cpu_list)
+{
+   char *cpumask_str;
+   uint64_t cpumask;
+   int ret;
+
+   ret = parse_cpumask_str(cpu_list, );
+   if (ret < 0)
+   die("can't parse cpumask");
+
+   cpumask_str = malloc(MASK_STR_MAX);
+   if (!cpumask_str)
+   die("can't allocate cpumask");
+
+   snprintf(cpumask_str, MASK_STR_MAX-1, "%lx", cpumask);
+   return cpumask_str;
+}
+
+
 static void set_mask(struct buffer_instance *instance)
 {
struct stat st;
@@ -4136,6 +4269,7 @@ enum {
OPT_nosplice= 253,
OPT_funcstack   = 254,
OPT_date= 255,
+   OPT_cpulist = 256,
 };
 
 void trace_record (int argc, char **argv)
@@ -4337,6 +4471,7 @@ void trace_record (int argc, char **argv)
{"stderr", no_argument, NULL, OPT_stderr},
{"by-comm", no_argument, NULL, OPT_bycomm},
{"ts-offset", required_argument, NULL, OPT_tsoffset},
+   {"cpu-list",