* [PATCH v2 0/2] Reset CPU mask
@ 2019-10-01 14:57 Tzvetomir Stoyanov (VMware)
2019-10-01 14:57 ` [PATCH v2 1/2] trace-cmd: Reset CPU mask after setting it in trace-cmd record with option -M Tzvetomir Stoyanov (VMware)
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-10-01 14:57 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
Reset CPU mask:
- After setting it in trace-cmd record with option -M, to its previous
value.
- By the "trace-cmd reset" command, to its default value - all CPUs are
enabled for tracing.
Fixes https://bugzilla.kernel.org/show_bug.cgi?id=204941
[
v2 changes:
Addressed Steven Rostedt and Eugene Syromyatnikov <evgsyr@gmail.com>
comments about the correct cpu mask in case we have more than 32 CPUs.
]
Tzvetomir Stoyanov (VMware) (2):
trace-cmd: Reset CPU mask after setting it in trace-cmd record with
option -M
trace-cmd: Reset CPU mask to its default value with "trace-cmd reset".
tracecmd/trace-record.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
--
2.21.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/2] trace-cmd: Reset CPU mask after setting it in trace-cmd record with option -M 2019-10-01 14:57 [PATCH v2 0/2] Reset CPU mask Tzvetomir Stoyanov (VMware) @ 2019-10-01 14:57 ` Tzvetomir Stoyanov (VMware) 2019-10-01 14:57 ` [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset" Tzvetomir Stoyanov (VMware) 2019-10-03 14:44 ` [PATCH v2 3/2] trace-cmd: Optimize the logic of reset_cpu_mask() Steven Rostedt 2 siblings, 0 replies; 7+ messages in thread From: Tzvetomir Stoyanov (VMware) @ 2019-10-01 14:57 UTC (permalink / raw) To: rostedt; +Cc: linux-trace-devel When trace-cmd is run with option -M, to set the tracing cpumask, it was not reset to its previous state. The trace-cmd record should put back the original value after using -M for the tracing cpu masks. Fixes https://bugzilla.kernel.org/show_bug.cgi?id=204941 Reported-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- tracecmd/trace-record.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 96d2c1a..69de82a 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -2403,6 +2403,7 @@ static void set_mask(struct buffer_instance *instance) path = get_instance_file(instance, "tracing_cpumask"); if (!path) die("could not allocate path"); + reset_save_file(path, RESET_DEFAULT_PRIO); ret = stat(path, &st); if (ret < 0) { -- 2.21.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset". 2019-10-01 14:57 [PATCH v2 0/2] Reset CPU mask Tzvetomir Stoyanov (VMware) 2019-10-01 14:57 ` [PATCH v2 1/2] trace-cmd: Reset CPU mask after setting it in trace-cmd record with option -M Tzvetomir Stoyanov (VMware) @ 2019-10-01 14:57 ` Tzvetomir Stoyanov (VMware) 2019-10-01 15:32 ` Steven Rostedt 2019-10-03 14:44 ` [PATCH v2 3/2] trace-cmd: Optimize the logic of reset_cpu_mask() Steven Rostedt 2 siblings, 1 reply; 7+ messages in thread From: Tzvetomir Stoyanov (VMware) @ 2019-10-01 14:57 UTC (permalink / raw) To: rostedt; +Cc: linux-trace-devel "trace-cmd reset" command should put all ftrace config to its default state, but trace cpumask was not reseted. The patch sets cpumask to its default value - all CPUs are enabled for tracing. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- tracecmd/trace-record.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 69de82a..6f3520c 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -4096,6 +4096,40 @@ static void reset_clock(void) write_instance_file(instance, "trace_clock", "local", "clock"); } +static void reset_cpu_mask(void) +{ + struct buffer_instance *instance; + int cpus = count_cpus(); + int fullwords; + char *buf; + int bits; + int len; + + fullwords = cpus / 32; + bits = cpus % 32; + len = (fullwords + 1) * 9; + + buf = malloc(len + 1); + if (!buf) { + warning("Unable to allocate %d bytes for cpu mask", len + 1); + return; + } + buf[0] = '\0'; + if (bits) { + sprintf(buf, "%x", (1 << bits) - 1); + } else if (fullwords) { + strcat(buf, "ffffffff"); + fullwords--; + } + while (fullwords-- > 0) + strcat(buf, ",ffffffff"); + + for_all_instances(instance) + write_instance_file(instance, "tracing_cpumask", buf, "cpumask"); + + free(buf); +} + static void reset_event_pid(void) { add_event_pid(""); @@ -4808,6 +4842,7 @@ void trace_reset(int argc, char **argv) reset_clock(); reset_event_pid(); reset_max_latency_instance(); + reset_cpu_mask(); tracecmd_remove_instances(); clear_func_filters(); /* restore tracing_on to 1 */ -- 2.21.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset". 2019-10-01 14:57 ` [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset" Tzvetomir Stoyanov (VMware) @ 2019-10-01 15:32 ` Steven Rostedt 2019-10-01 15:37 ` Tzvetomir Stoyanov 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2019-10-01 15:32 UTC (permalink / raw) To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel On Tue, 1 Oct 2019 17:57:40 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > "trace-cmd reset" command should put all ftrace config to its default > state, but trace cpumask was not reseted. The patch sets cpumask to > its default value - all CPUs are enabled for tracing. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > tracecmd/trace-record.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index 69de82a..6f3520c 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -4096,6 +4096,40 @@ static void reset_clock(void) > write_instance_file(instance, "trace_clock", "local", "clock"); > } > > +static void reset_cpu_mask(void) > +{ > + struct buffer_instance *instance; > + int cpus = count_cpus(); > + int fullwords; > + char *buf; > + int bits; > + int len; > + > + fullwords = cpus / 32; > + bits = cpus % 32; > + len = (fullwords + 1) * 9; > + > + buf = malloc(len + 1); > + if (!buf) { > + warning("Unable to allocate %d bytes for cpu mask", len + 1); > + return; > + } > + buf[0] = '\0'; > + if (bits) { > + sprintf(buf, "%x", (1 << bits) - 1); > + } else if (fullwords) { > + strcat(buf, "ffffffff"); > + fullwords--; > + } > + while (fullwords-- > 0) > + strcat(buf, ",ffffffff"); I like my way because it's a little more condensed ;-) But this works too and it's far from a fast path. As it's probably more readable, I'll all this one. -- Steve > + > + for_all_instances(instance) > + write_instance_file(instance, "tracing_cpumask", buf, "cpumask"); > + > + free(buf); > +} > + > static void reset_event_pid(void) > { > add_event_pid(""); > @@ -4808,6 +4842,7 @@ void trace_reset(int argc, char **argv) > reset_clock(); > reset_event_pid(); > reset_max_latency_instance(); > + reset_cpu_mask(); > tracecmd_remove_instances(); > clear_func_filters(); > /* restore tracing_on to 1 */ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset". 2019-10-01 15:32 ` Steven Rostedt @ 2019-10-01 15:37 ` Tzvetomir Stoyanov 2019-10-01 15:45 ` Steven Rostedt 0 siblings, 1 reply; 7+ messages in thread From: Tzvetomir Stoyanov @ 2019-10-01 15:37 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel On Tue, Oct 1, 2019 at 6:32 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 1 Oct 2019 17:57:40 +0300 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > "trace-cmd reset" command should put all ftrace config to its default > > state, but trace cpumask was not reseted. The patch sets cpumask to > > its default value - all CPUs are enabled for tracing. > > > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > > --- > > tracecmd/trace-record.c | 35 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > > index 69de82a..6f3520c 100644 > > --- a/tracecmd/trace-record.c > > +++ b/tracecmd/trace-record.c > > @@ -4096,6 +4096,40 @@ static void reset_clock(void) > > write_instance_file(instance, "trace_clock", "local", "clock"); > > } > > > > +static void reset_cpu_mask(void) > > +{ > > + struct buffer_instance *instance; > > + int cpus = count_cpus(); > > + int fullwords; > > + char *buf; > > + int bits; > > + int len; > > + > > + fullwords = cpus / 32; > > + bits = cpus % 32; > > + len = (fullwords + 1) * 9; > > + > > + buf = malloc(len + 1); > > + if (!buf) { > > + warning("Unable to allocate %d bytes for cpu mask", len + 1); > > + return; > > + } > > + buf[0] = '\0'; > > + if (bits) { > > + sprintf(buf, "%x", (1 << bits) - 1); > > + } else if (fullwords) { > > + strcat(buf, "ffffffff"); > > + fullwords--; > > + } > > + while (fullwords-- > 0) > > + strcat(buf, ",ffffffff"); > > I like my way because it's a little more condensed ;-) > > But this works too and it's far from a fast path. As it's probably > more readable, I'll all this one. > > -- Steve > Actually the only change here is to handle the case when bits is 0, not to put the leading ",". The ftrace complains with "Invalid argument" in that case. > > > + > > + for_all_instances(instance) > > + write_instance_file(instance, "tracing_cpumask", buf, "cpumask"); > > + > > + free(buf); > > +} > > + > > static void reset_event_pid(void) > > { > > add_event_pid(""); > > @@ -4808,6 +4842,7 @@ void trace_reset(int argc, char **argv) > > reset_clock(); > > reset_event_pid(); > > reset_max_latency_instance(); > > + reset_cpu_mask(); > > tracecmd_remove_instances(); > > clear_func_filters(); > > /* restore tracing_on to 1 */ > -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset". 2019-10-01 15:37 ` Tzvetomir Stoyanov @ 2019-10-01 15:45 ` Steven Rostedt 0 siblings, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2019-10-01 15:45 UTC (permalink / raw) To: Tzvetomir Stoyanov; +Cc: linux-trace-devel On Tue, 1 Oct 2019 18:37:54 +0300 Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > On Tue, Oct 1, 2019 at 6:32 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Tue, 1 Oct 2019 17:57:40 +0300 > > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > > > "trace-cmd reset" command should put all ftrace config to its default > > > state, but trace cpumask was not reseted. The patch sets cpumask to > > > its default value - all CPUs are enabled for tracing. > > > > > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > > > --- > > > tracecmd/trace-record.c | 35 +++++++++++++++++++++++++++++++++++ > > > 1 file changed, 35 insertions(+) > > > > > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > > > index 69de82a..6f3520c 100644 > > > --- a/tracecmd/trace-record.c > > > +++ b/tracecmd/trace-record.c > > > @@ -4096,6 +4096,40 @@ static void reset_clock(void) > > > write_instance_file(instance, "trace_clock", "local", "clock"); > > > } > > > > > > +static void reset_cpu_mask(void) > > > +{ > > > + struct buffer_instance *instance; > > > + int cpus = count_cpus(); > > > + int fullwords; > > > + char *buf; > > > + int bits; > > > + int len; > > > + > > > + fullwords = cpus / 32; > > > + bits = cpus % 32; > > > + len = (fullwords + 1) * 9; > > > + > > > + buf = malloc(len + 1); > > > + if (!buf) { > > > + warning("Unable to allocate %d bytes for cpu mask", len + 1); > > > + return; > > > + } > > > + buf[0] = '\0'; > > > + if (bits) { > > > + sprintf(buf, "%x", (1 << bits) - 1); > > > + } else if (fullwords) { > > > + strcat(buf, "ffffffff"); > > > + fullwords--; > > > + } > > > + while (fullwords-- > 0) > > > + strcat(buf, ",ffffffff"); > > > > I like my way because it's a little more condensed ;-) > > > > But this works too and it's far from a fast path. As it's probably > > more readable, I'll all this one. > > > > -- Steve > > > Actually the only change here is to handle the case when bits is 0, > not to put the leading ",". > The ftrace complains with "Invalid argument" in that case. > Mine handled that case too. The difference was the "cpus - 1" part of my code: fullwords = (cpus - 1) / 32; <-- bits = (cpus - 1) % 32 + 1; <-- len = (fullwords + 1) * 9; buf = malloc(len + 1); buf[0] = '\0'; if (bits) sprintf(buf, "%x", (unsigned int)((1ULL << bits) - 1)); while (fullwords-- > 0) strcat(buf, ",ffffffff"); The output of yours and mine were identical. When cpus was a power of 32, it would still have one less of "fullwords", and bits would be 32. Which is why I needed to add "1ULL" to make: (1 << 32) - 1 == 0xffffffff the fullwords was only for non bits. -- Steve ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/2] trace-cmd: Optimize the logic of reset_cpu_mask() 2019-10-01 14:57 [PATCH v2 0/2] Reset CPU mask Tzvetomir Stoyanov (VMware) 2019-10-01 14:57 ` [PATCH v2 1/2] trace-cmd: Reset CPU mask after setting it in trace-cmd record with option -M Tzvetomir Stoyanov (VMware) 2019-10-01 14:57 ` [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset" Tzvetomir Stoyanov (VMware) @ 2019-10-03 14:44 ` Steven Rostedt 2 siblings, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2019-10-03 14:44 UTC (permalink / raw) To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> Change the logic of reset_cpu_mask() to be a little more condensed, and also move the buffer onto the stack and eliminate the warning message as we no longer need to use malloc. Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> --- tracecmd/trace-record.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 6f3520cc..cfe67f90 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -4100,27 +4100,14 @@ static void reset_cpu_mask(void) { struct buffer_instance *instance; int cpus = count_cpus(); - int fullwords; - char *buf; - int bits; - int len; + int fullwords = (cpus - 1) / 32; + int bits = (cpus - 1) % 32 + 1; + int len = (fullwords + 1) * 9; + char buf[len + 1]; - fullwords = cpus / 32; - bits = cpus % 32; - len = (fullwords + 1) * 9; - - buf = malloc(len + 1); - if (!buf) { - warning("Unable to allocate %d bytes for cpu mask", len + 1); - return; - } buf[0] = '\0'; - if (bits) { - sprintf(buf, "%x", (1 << bits) - 1); - } else if (fullwords) { - strcat(buf, "ffffffff"); - fullwords--; - } + + sprintf(buf, "%x", (unsigned int)((1ULL << bits) - 1)); while (fullwords-- > 0) strcat(buf, ",ffffffff"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-03 14:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-01 14:57 [PATCH v2 0/2] Reset CPU mask Tzvetomir Stoyanov (VMware) 2019-10-01 14:57 ` [PATCH v2 1/2] trace-cmd: Reset CPU mask after setting it in trace-cmd record with option -M Tzvetomir Stoyanov (VMware) 2019-10-01 14:57 ` [PATCH v2 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset" Tzvetomir Stoyanov (VMware) 2019-10-01 15:32 ` Steven Rostedt 2019-10-01 15:37 ` Tzvetomir Stoyanov 2019-10-01 15:45 ` Steven Rostedt 2019-10-03 14:44 ` [PATCH v2 3/2] trace-cmd: Optimize the logic of reset_cpu_mask() Steven Rostedt
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).