This series adds support for a --cpu-list option, which is
much more human friendly than -M:
# trace-cmd record --cpu-list 1,4,10-15 [...]
The first patch is a small refectoring needed to
make --cpu-list support fit nicely. The second patch
adds the new option.
v2
--
- Use the CPU_SET() API instead of cpu.h, which
makes the cpumask dynamic and avoid various bugs
- Small cleanups and renamings
Luiz Capitulino (2):
trace-cmd record: refactor set_mask()
trace-cmd record: add --cpu-list option
Documentation/trace-cmd-record.1.txt | 4 +
trace-local.h | 2 +-
trace-record.c | 258 ++++++++++++++++++++++++++++++++---
3 files changed, 245 insertions(+), 19 deletions(-)
--
2.5.5
This commit makes set_mask() more specialized: all
it does now is to write the cpumask to tracing_cpumask.
The handling of "-M -1" is now done by the newly
added alloc_mask_from_hex().
Also, uneeded checks are dropped and
buffer_instance->cpumask points to dynamic memory.
This work is a preparation for supporting the
"--cpu-list" option in the next commit.
Signed-off-by: Luiz Capitulino <[email protected]>
---
trace-local.h | 2 +-
trace-record.c | 54 ++++++++++++++++++++++++++++++++++--------------------
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/trace-local.h b/trace-local.h
index 62363d0..0ac39bf 100644
--- a/trace-local.h
+++ b/trace-local.h
@@ -143,7 +143,7 @@ struct func_list {
struct buffer_instance {
struct buffer_instance *next;
const char *name;
- const char *cpumask;
+ char *cpumask;
struct event_list *events;
struct event_list **event_next;
diff --git a/trace-record.c b/trace-record.c
index 22b6835..0f1f2c4 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -69,6 +69,8 @@ enum trace_type {
TRACE_TYPE_PROFILE = (1 << 4) | TRACE_TYPE_STREAM,
};
+#define CPUMASK_STR_MAX 4096 /* Don't expect more than 32768 CPUS */
+
static int rt_prio;
static int keep;
@@ -2078,27 +2080,24 @@ static void update_pid_event_filters(struct buffer_instance *instance)
update_event_filters(instance);
}
-static void set_mask(struct buffer_instance *instance)
-{
- const char *mask = instance->cpumask;
- struct stat st;
- char cpumask[4096]; /* Don't expect more than 32768 CPUS */
- char *path;
- int fd;
- int ret;
- if (!mask)
- return;
+static char *alloc_mask_from_hex(const char *str)
+{
+ char *cpumask;
- if (strcmp(mask, "-1") == 0) {
+ if (strcmp(str, "-1") == 0) {
/* set all CPUs */
int bytes = (cpu_count + 7) / 8;
int last = cpu_count % 8;
int i;
- if (bytes > 4095) {
+ cpumask = malloc(CPUMASK_STR_MAX);
+ if (!cpumask)
+ die("can't allocate cpumask");
+
+ if (bytes > (CPUMASK_STR_MAX-1)) {
warning("cpumask can't handle more than 32768 CPUS!");
- bytes = 4095;
+ bytes = CPUMASK_STR_MAX-1;
}
sprintf(cpumask, "%x", (1 << last) - 1);
@@ -2107,18 +2106,32 @@ static void set_mask(struct buffer_instance *instance)
cpumask[i] = 'f';
cpumask[i+1] = 0;
-
- mask = cpumask;
+ } else {
+ cpumask = strdup(str);
+ if (!cpumask)
+ die("can't allocate cpumask");
}
+ return cpumask;
+}
+
+static void set_mask(struct buffer_instance *instance)
+{
+ struct stat st;
+ char *path;
+ int fd;
+ int ret;
+
+ if (!instance->cpumask)
+ return;
+
path = get_instance_file(instance, "tracing_cpumask");
if (!path)
die("could not allocate path");
ret = stat(path, &st);
if (ret < 0) {
- if (mask)
- warning("%s not found", path);
+ warning("%s not found", path);
goto out;
}
@@ -2126,12 +2139,13 @@ static void set_mask(struct buffer_instance *instance)
if (fd < 0)
die("could not open %s\n", path);
- if (mask)
- write(fd, mask, strlen(mask));
+ write(fd, instance->cpumask, strlen(instance->cpumask));
close(fd);
out:
tracecmd_put_tracing_file(path);
+ free(instance->cpumask);
+ instance->cpumask = NULL;
}
static void enable_events(struct buffer_instance *instance)
@@ -4530,7 +4544,7 @@ void trace_record (int argc, char **argv)
max_kb = atoi(optarg);
break;
case 'M':
- instance->cpumask = optarg;
+ instance->cpumask = alloc_mask_from_hex(optarg);
break;
case 't':
if (extract)
--
2.5.5
With --cpu-list you can do:
# trace-cmd record --cpu-list 1,4,10-15 [...]
Which is much more human friendly than -M.
Support for --cpu-list is implemented by dynamically
allocating a cpu_set_t object and setting the parsed
CPUs. Using the CPU_SET API allows for more robost
error detection.
Signed-off-by: Luiz Capitulino <[email protected]>
---
Documentation/trace-cmd-record.1.txt | 4 +
trace-record.c | 208 +++++++++++++++++++++++++++++++++++
2 files changed, 212 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 0f1f2c4..49d76db 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -23,6 +23,7 @@
#include <string.h>
#include <stdarg.h>
#include <getopt.h>
+#include <limits.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/time.h>
@@ -2080,6 +2081,208 @@ static void update_pid_event_filters(struct buffer_instance *instance)
update_event_filters(instance);
}
+struct cpuset {
+ int nr_cpus;
+ size_t size;
+ cpu_set_t *set;
+};
+
+static void set_cpu(int cpu, struct cpuset *set)
+{
+ if (cpu < 0 || cpu > set->nr_cpus - 1)
+ die("invalid cpu in range");
+ CPU_SET_S(cpu, set->size, set->set);
+}
+
+static int read_cpu_nr(const char *str, int *ret)
+{
+ char *endptr;
+ int val;
+
+ errno = 0;
+ val = strtol(str, &endptr, 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 parse_single_cpu(const char *str, struct cpuset *set)
+{
+ int cpu, err;
+
+ err = read_cpu_nr(str, &cpu);
+ if (err)
+ return -1;
+
+ set_cpu(cpu, set);
+ return 0;
+}
+
+static int parse_range_one(char *str, char **saveptr)
+{
+ int err, cpu;
+ char *p;
+
+ p = strtok_r(str, "-", saveptr);
+ if (!p)
+ return -1;
+
+ err = read_cpu_nr(p, &cpu);
+ if (err)
+ return -1;
+
+ return cpu;
+}
+
+static int parse_range(const char *str, int *begin, int *end)
+{
+ char *saveptr, *range;
+ int ret;
+
+ range = strdup(str);
+ if (!range)
+ return -1;
+
+ ret = parse_range_one(range, &saveptr);
+ if (ret < 0)
+ goto out;
+ *begin = ret;
+
+ ret = parse_range_one(NULL, &saveptr);
+ if (ret < 0)
+ goto out;
+ *end = ret;
+
+out:
+ free(range);
+ return ret < 0 ? -1 : 0;
+}
+
+static int parse_cpu_range(const char *str, struct cpuset *set)
+{
+ int i, ret, begin, end;
+
+ ret = parse_range(str, &begin, &end);
+ if (ret < 0 || begin > end)
+ return -1;
+
+ for (i = begin; i <= end; i++)
+ set_cpu(i, set);
+
+ return 0;
+}
+
+static int has_range(const char *str)
+{
+ return strchr(str, '-') != NULL;
+}
+
+static int parse_cpu_list(const char *cpu_list, struct cpuset *set)
+{
+ char *saveptr, *str, *p;
+ int err = 0;
+
+ str = strdup(cpu_list);
+ if (!str)
+ return -1;
+
+ p = strtok_r(str, ",", &saveptr);
+ while (p) {
+ if (has_range(p))
+ err = parse_cpu_range(p, set);
+ else
+ err = parse_single_cpu(p, set);
+ if (err)
+ goto out;
+ p = strtok_r(NULL, ",", &saveptr);
+ }
+
+out:
+ free(str);
+ return err;
+}
+
+static int val_to_char(int v)
+{
+ if (v >= 0 && v < 10)
+ return '0' + v;
+ else if (v >= 10 && v < 16)
+ return ('a' - 10) + v;
+ else
+ return -1;
+}
+
+/* From util-linux */
+static char *cpu_set_to_str(char *str, size_t len, struct cpuset *set)
+{
+ char *ptr = str;
+ char *ret = NULL;
+ int cpu;
+
+ for (cpu = (8 * set->size) - 4; cpu >= 0; cpu -= 4) {
+ char val = 0;
+
+ if (len == (size_t) (ptr - str))
+ break;
+
+ if (CPU_ISSET_S(cpu, set->size, set->set))
+ val |= 1;
+ if (CPU_ISSET_S(cpu + 1, set->size, set->set))
+ val |= 2;
+ if (CPU_ISSET_S(cpu + 2, set->size, set->set))
+ val |= 4;
+ if (CPU_ISSET_S(cpu + 3, set->size, set->set))
+ val |= 8;
+
+ if (!ret && val)
+ ret = ptr;
+ *ptr++ = val_to_char(val);
+ }
+
+ *ptr = '\0';
+ return ret ? ret : ptr - 1;
+}
+
+static char *alloc_mask_from_list(const char *cpu_list)
+{
+ struct cpuset set;
+ char *str;
+ int ret;
+
+ set.nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
+ if (set.nr_cpus < 0)
+ die("can't get number of processors\n");
+
+ set.set = CPU_ALLOC(set.nr_cpus);
+ if (!set.set)
+ die("can't allocate cpu_set\n");
+
+ set.size = CPU_ALLOC_SIZE(set.nr_cpus);
+ CPU_ZERO_S(set.size, set.set);
+
+ ret = parse_cpu_list(cpu_list, &set);
+ if (ret < 0)
+ die("invalid cpu list specified");
+
+ str = malloc(CPUMASK_STR_MAX);
+ if (!str)
+ die("can't allocate memory\n");
+
+ cpu_set_to_str(str, CPUMASK_STR_MAX, &set);
+ CPU_FREE(set.set);
+
+ return str;
+}
static char *alloc_mask_from_hex(const char *str)
{
@@ -4137,6 +4340,7 @@ enum {
OPT_nosplice = 253,
OPT_funcstack = 254,
OPT_date = 255,
+ OPT_cpulist = 256,
};
void trace_record (int argc, char **argv)
@@ -4338,6 +4542,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", required_argument, NULL, OPT_cpulist},
{"max-graph-depth", required_argument, NULL, OPT_max_graph_depth},
{"debug", no_argument, NULL, OPT_debug},
{"help", no_argument, NULL, '?'},
@@ -4546,6 +4751,9 @@ void trace_record (int argc, char **argv)
case 'M':
instance->cpumask = alloc_mask_from_hex(optarg);
break;
+ case OPT_cpulist:
+ instance->cpumask = alloc_mask_from_list(optarg);
+ break;
case 't':
if (extract)
topt = 1; /* Extract top instance also */
--
2.5.5
Looking through my inbox, I stumbled on this.
On Tue, 3 Jan 2017 11:54:06 -0500
Luiz Capitulino <[email protected]> wrote:
> On Tue, 3 Jan 2017 11:45:30 -0500
> Steven Rostedt <[email protected]> wrote:
>
> > On Tue, 3 Jan 2017 11:32:58 -0500
> > Luiz Capitulino <[email protected]> wrote:
> >
> > > On Tue, 22 Nov 2016 15:20:50 -0500
> > > Luiz Capitulino <[email protected]> wrote:
> > >
> > > > This series adds support for a --cpu-list option, which is
> > > > much more human friendly than -M:
> > > >
> > > > # trace-cmd record --cpu-list 1,4,10-15 [...]
> > > >
> > > > The first patch is a small refectoring needed to
> > > > make --cpu-list support fit nicely. The second patch
> > > > adds the new option.
> > >
> > > ping?
> > >
> >
> > Thanks for the reminder ;-)
> >
> > I'm going to try to get to this this week, but as this is my first week
> > on my new job (not to mention the new year), you may need to ping me
> > again.
>
> I see, no rush then.
I hope you really meant that, as I really did need another ping.
I think this is a good option to have. I've Cc'd linux-trace-devel, and
hopefully we'll get this feature added much sooner than the next 3
years!
-- Steve
On 2020-03-03 10:07 p.m., Steven Rostedt wrote:
> Looking through my inbox, I stumbled on this.
>
> On Tue, 3 Jan 2017 11:54:06 -0500
> Luiz Capitulino <[email protected]> wrote:
>
>> On Tue, 3 Jan 2017 11:45:30 -0500
>> Steven Rostedt <[email protected]> wrote:
>>
>>> On Tue, 3 Jan 2017 11:32:58 -0500
>>> Luiz Capitulino <[email protected]> wrote:
>>>
>>>> On Tue, 22 Nov 2016 15:20:50 -0500
>>>> Luiz Capitulino <[email protected]> wrote:
>>>>
>>>>> This series adds support for a --cpu-list option, which is
>>>>> much more human friendly than -M:
>>>>>
>>>>> # trace-cmd record --cpu-list 1,4,10-15 [...]
>>>>>
>>>>> The first patch is a small refectoring needed to
>>>>> make --cpu-list support fit nicely. The second patch
>>>>> adds the new option.
>>>>
>>>> ping?
>>>>
>>>
>>> Thanks for the reminder ;-)
>>>
>>> I'm going to try to get to this this week, but as this is my first week
>>> on my new job (not to mention the new year), you may need to ping me
>>> again.
>>
>> I see, no rush then.
>
> I hope you really meant that, as I really did need another ping.
>
> I think this is a good option to have. I've Cc'd linux-trace-devel, and
> hopefully we'll get this feature added much sooner than the next 3
> years!
That's great to hear! :)
- Luiz