[PATCH 0/2] Reset CPU mask

[PATCH 0/2] Reset CPU mask

[Tzvetomir Stoyanov (VMware)]

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

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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

-- 
2.21.0

[PATCH 1/2] trace-cmd: Reset CPU mask after setting it in trace-cmd record with option -M

[Tzvetomir Stoyanov (VMware)]

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

[PATCH 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset".

[Tzvetomir Stoyanov (VMware)]

"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 | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 69de82a..c41f55f 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4096,6 +4096,24 @@ static void reset_clock(void)
 		write_instance_file(instance, "trace_clock", "local", "clock");
 }
 
+static void reset_cpu_mask(void)
+{
+	char str[24];
+	int cpumask = 0;
+	int cpus = count_cpus();
+	struct buffer_instance *instance;
+
+	while (cpus--) {
+		cpumask <<= 1;
+		cpumask |= 1;
+	}
+	if (snprintf(str, 24, "%x", cpumask) <= 0)
+		return;
+
+	for_all_instances(instance)
+		write_instance_file(instance, "tracing_cpumask", str, "cpumask");
+}
+
 static void reset_event_pid(void)
 {
 	add_event_pid("");
@@ -4808,6 +4826,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

Re: [PATCH 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset".

[Steven Rostedt]

On Wed, 25 Sep 2019 14:08:23 +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 | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 69de82a..c41f55f 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -4096,6 +4096,24 @@ static void reset_clock(void)
>  		write_instance_file(instance, "trace_clock", "local", "clock");
>  }
>  
> +static void reset_cpu_mask(void)
> +{
> +	char str[24];
> +	int cpumask = 0;
> +	int cpus = count_cpus();
> +	struct buffer_instance *instance;
> +
> +	while (cpus--) {
> +		cpumask <<= 1;
> +		cpumask |= 1;
> +	}

First, you can accomplish the same with:

	(1 << cpus) - 1;

But then this only works if we have less than 32 CPUs.

What we would want is:

	int fullwords;
	char *buf;
	int bits;
	int cpus;
	int len;

	fullwords = cpus / 32;
	bits = cpus % 32;
	len = (fullwords + 1) * 8;

	buf = malloc(len + 1);
	buf[0] = '\0';

	if (bits)
		sprintf(buf, "%x", (1 << bits) - 1);
	while (fullwords-- > 0)
		strcat(buf, "ffffffff");


Because we may run this on machines with 1000s of CPUs!

(BTW, I tested the above under valgrind with the following values:
  1 31 32 33 126 127 128 129 with no memory leaks and the results
  looked good)

-- Steve


> +	if (snprintf(str, 24, "%x", cpumask) <= 0)
> +		return;
> +
> +	for_all_instances(instance)
> +		write_instance_file(instance, "tracing_cpumask", str, "cpumask");
> +}
> +
>  static void reset_event_pid(void)
>  {
>  	add_event_pid("");
> @@ -4808,6 +4826,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 */

Re: [PATCH 2/2] trace-cmd: Reset CPU mask to its default value with "trace-cmd reset".

[Steven Rostedt]

[ Added back the list, as this is good to document ]

On Tue, 1 Oct 2019 14:17:08 +0200
Eugene Syromyatnikov <evgsyr@gmail.com> wrote:

> On Mon, Sep 30, 2019 at 11:13 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >         if (bits)
> >                 sprintf(buf, "%x", (1 << bits) - 1);
> >         while (fullwords-- > 0)
> >                 strcat(buf, "ffffffff");  
> Additional commas that split 32-bit chunks are needed here, if I read
> lib/bitmap.c:__bitmap_parse correctly.
> 

You're right. I just tested it:

 # echo fffffffff > /debug/tracing/tracing_cpumask 
-bash: echo: write error: Value too large for defined data type
 # echo f,ffffffff > /debug/tracing/tracing_cpumask 
 # 


This should work then:

	int fullwords;
	char *buf;
	int bits;
	int cpus;
	int len;

	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");

-- Steve