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

Message ID 20190925110823.1242-3-tz.stoyanov@gmail.com
State Superseded
Headers show
Series
  • Reset CPU mask
Related show

Commit Message

Tzvetomir Stoyanov (VMware) Sept. 25, 2019, 11:08 a.m. UTC
"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(+)

Comments

Steven Rostedt Sept. 30, 2019, 6:44 p.m. UTC | #1
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 */
Steven Rostedt Oct. 1, 2019, 1:52 p.m. UTC | #2
[ 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

Patch
diff mbox series

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 */