[PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

[PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

[Tommi Rantala]

Since glibc 2.28 when running 'perf top --stdio', input handling no
longer works, but hitting any key always just prints the "Mapped keys"
help text.

To fix it, call clearerr() in the display_thread() loop to clear any EOF
sticky errors, as instructed in the glibc NEWS file
(https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS):

 * All stdio functions now treat end-of-file as a sticky condition.  If you
   read from a file until EOF, and then the file is enlarged by another
   process, you must call clearerr or another function with the same effect
   (e.g. fseek, rewind) before you can read the additional data.  This
   corrects a longstanding C99 conformance bug.  It is most likely to affect
   programs that use stdio to read interactive input from a terminal.
   (Bug #1190.)

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/builtin-top.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f6dd1a63f159e..d2539b793f9d4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -684,7 +684,9 @@ static void *display_thread(void *arg)
 	delay_msecs = top->delay_secs * MSEC_PER_SEC;
 	set_term_quiet_input(&save);
 	/* trash return*/
-	getc(stdin);
+	clearerr(stdin);
+	if (poll(&stdin_poll, 1, 0) > 0)
+		getc(stdin);
 
 	while (!done) {
 		perf_top__print_sym_table(top);
-- 
2.21.1

Re: [PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

[Arnaldo Carvalho de Melo]

Em Thu, Mar 05, 2020 at 10:37:12AM +0200, Tommi Rantala escreveu:
> Since glibc 2.28 when running 'perf top --stdio', input handling no
> longer works, but hitting any key always just prints the "Mapped keys"
> help text.
> 
> To fix it, call clearerr() in the display_thread() loop to clear any EOF
> sticky errors, as instructed in the glibc NEWS file
> (https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS):
> 
>  * All stdio functions now treat end-of-file as a sticky condition.  If you
>    read from a file until EOF, and then the file is enlarged by another
>    process, you must call clearerr or another function with the same effect
>    (e.g. fseek, rewind) before you can read the additional data.  This
>    corrects a longstanding C99 conformance bug.  It is most likely to affect
>    programs that use stdio to read interactive input from a terminal.
>    (Bug #1190.)

Thanks for fixing this, I had stumbled on it at some point, but since I
mostly use the TUI interface, it fell thru the cracks.

Do you prefer it over the TUI one?

Thanks, tested and applied.

- Arnaldo
 
> Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> ---
>  tools/perf/builtin-top.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index f6dd1a63f159e..d2539b793f9d4 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -684,7 +684,9 @@ static void *display_thread(void *arg)
>  	delay_msecs = top->delay_secs * MSEC_PER_SEC;
>  	set_term_quiet_input(&save);
>  	/* trash return*/
> -	getc(stdin);
> +	clearerr(stdin);
> +	if (poll(&stdin_poll, 1, 0) > 0)
> +		getc(stdin);
>  
>  	while (!done) {
>  		perf_top__print_sym_table(top);
> -- 
> 2.21.1
> 

-- 

- Arnaldo

Re: [PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

[Rantala, Tommi T. (Nokia - FI/Espoo)]

On Thu, 2020-03-05 at 11:49 -0300, Arnaldo Carvalho de Melo wrote:
> Thanks for fixing this, I had stumbled on it at some point, but since
> I mostly use the TUI interface, it fell thru the cracks.
> 
> Do you prefer it over the TUI one?

The stdio interface is important for us, as we're building perf without s-
lang. To reduce dependencies I assume... But the TUI interface is
certainly nice, so maybe we should just include s-lang to get it.

-Tommi

> Thanks, tested and applied.
> 
> - Arnaldo
>  
> > Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> > ---
> >  tools/perf/builtin-top.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index f6dd1a63f159e..d2539b793f9d4 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -684,7 +684,9 @@ static void *display_thread(void *arg)
> >  	delay_msecs = top->delay_secs * MSEC_PER_SEC;
> >  	set_term_quiet_input(&save);
> >  	/* trash return*/
> > -	getc(stdin);
> > +	clearerr(stdin);
> > +	if (poll(&stdin_poll, 1, 0) > 0)
> > +		getc(stdin);
> >  
> >  	while (!done) {
> >  		perf_top__print_sym_table(top);
> > -- 
> > 2.21.1
> > 

[tip: perf/urgent] perf top: Fix stdio interface input handling with glibc 2.28+

[tip-bot2 for Tommi Rantala]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     29b4f5f188571c112713c35cc87eefb46efee612
Gitweb:        https://git.kernel.org/tip/29b4f5f188571c112713c35cc87eefb46efee612
Author:        Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate:    Thu, 05 Mar 2020 10:37:12 +02:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 06 Mar 2020 08:30:47 -03:00

perf top: Fix stdio interface input handling with glibc 2.28+

Since glibc 2.28 when running 'perf top --stdio', input handling no
longer works, but hitting any key always just prints the "Mapped keys"
help text.

To fix it, call clearerr() in the display_thread() loop to clear any EOF
sticky errors, as instructed in the glibc NEWS file
(https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS):

 * All stdio functions now treat end-of-file as a sticky condition.  If you
   read from a file until EOF, and then the file is enlarged by another
   process, you must call clearerr or another function with the same effect
   (e.g. fseek, rewind) before you can read the additional data.  This
   corrects a longstanding C99 conformance bug.  It is most likely to affect
   programs that use stdio to read interactive input from a terminal.
   (Bug #1190.)

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200305083714.9381-2-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f6dd1a6..d2539b7 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -684,7 +684,9 @@ repeat:
 	delay_msecs = top->delay_secs * MSEC_PER_SEC;
 	set_term_quiet_input(&save);
 	/* trash return*/
-	getc(stdin);
+	clearerr(stdin);
+	if (poll(&stdin_poll, 1, 0) > 0)
+		getc(stdin);
 
 	while (!done) {
 		perf_top__print_sym_table(top);

[PATCH 2/3] perf bench futex-wake: Restore thread count default to online CPU count

[Tommi Rantala]

Since commit 3b2323c2c1c4 ("perf bench futex: Use cpumaps") the default
number of threads the benchmark uses got changed from number of online
CPUs to zero:

  $ perf bench futex wake
  # Running 'futex/wake' benchmark:
  Run summary [PID 15930]: blocking on 0 threads (at [private] futex 0x558b8ee4bfac), waking up 1 at a time.
  [Run 1]: Wokeup 0 of 0 threads in 0.0000 ms
  [...]
  [Run 10]: Wokeup 0 of 0 threads in 0.0000 ms
  Wokeup 0 of 0 threads in 0.0004 ms (+-40.82%)

Restore the old behavior by grabbing the number of online CPUs via
cpu->nr:

  $ perf bench futex wake
  # Running 'futex/wake' benchmark:
  Run summary [PID 18356]: blocking on 8 threads (at [private] futex 0xb3e62c), waking up 1 at a time.
  [Run 1]: Wokeup 8 of 8 threads in 0.0260 ms
  [...]
  [Run 10]: Wokeup 8 of 8 threads in 0.0270 ms
  Wokeup 8 of 8 threads in 0.0419 ms (+-24.35%)

Fixes: 3b2323c2c1c4 ("perf bench futex: Use cpumaps")
Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/bench/futex-wake.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
index df810096abfef..58906e9499bb0 100644
--- a/tools/perf/bench/futex-wake.c
+++ b/tools/perf/bench/futex-wake.c
@@ -43,7 +43,7 @@ static bool done = false, silent = false, fshared = false;
 static pthread_mutex_t thread_lock;
 static pthread_cond_t thread_parent, thread_worker;
 static struct stats waketime_stats, wakeup_stats;
-static unsigned int ncpus, threads_starting, nthreads = 0;
+static unsigned int threads_starting, nthreads = 0;
 static int futex_flag = 0;
 
 static const struct option options[] = {
@@ -141,7 +141,7 @@ int bench_futex_wake(int argc, const char **argv)
 	sigaction(SIGINT, &act, NULL);
 
 	if (!nthreads)
-		nthreads = ncpus;
+		nthreads = cpu->nr;
 
 	worker = calloc(nthreads, sizeof(*worker));
 	if (!worker)
-- 
2.21.1

Re: [PATCH 2/3] perf bench futex-wake: Restore thread count default to online CPU count

[Arnaldo Carvalho de Melo]

Em Thu, Mar 05, 2020 at 10:37:13AM +0200, Tommi Rantala escreveu:
> Since commit 3b2323c2c1c4 ("perf bench futex: Use cpumaps") the default
> number of threads the benchmark uses got changed from number of online
> CPUs to zero:
> 
>   $ perf bench futex wake
>   # Running 'futex/wake' benchmark:
>   Run summary [PID 15930]: blocking on 0 threads (at [private] futex 0x558b8ee4bfac), waking up 1 at a time.
>   [Run 1]: Wokeup 0 of 0 threads in 0.0000 ms
>   [...]
>   [Run 10]: Wokeup 0 of 0 threads in 0.0000 ms
>   Wokeup 0 of 0 threads in 0.0004 ms (+-40.82%)
> 
> Restore the old behavior by grabbing the number of online CPUs via
> cpu->nr:
> 
>   $ perf bench futex wake
>   # Running 'futex/wake' benchmark:
>   Run summary [PID 18356]: blocking on 8 threads (at [private] futex 0xb3e62c), waking up 1 at a time.
>   [Run 1]: Wokeup 8 of 8 threads in 0.0260 ms
>   [...]
>   [Run 10]: Wokeup 8 of 8 threads in 0.0270 ms
>   Wokeup 8 of 8 threads in 0.0419 ms (+-24.35%)
> 
> Fixes: 3b2323c2c1c4 ("perf bench futex: Use cpumaps")

Thanks, tested and applied.

- Arnaldo

> Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> ---
>  tools/perf/bench/futex-wake.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
> index df810096abfef..58906e9499bb0 100644
> --- a/tools/perf/bench/futex-wake.c
> +++ b/tools/perf/bench/futex-wake.c
> @@ -43,7 +43,7 @@ static bool done = false, silent = false, fshared = false;
>  static pthread_mutex_t thread_lock;
>  static pthread_cond_t thread_parent, thread_worker;
>  static struct stats waketime_stats, wakeup_stats;
> -static unsigned int ncpus, threads_starting, nthreads = 0;
> +static unsigned int threads_starting, nthreads = 0;
>  static int futex_flag = 0;
>  
>  static const struct option options[] = {
> @@ -141,7 +141,7 @@ int bench_futex_wake(int argc, const char **argv)
>  	sigaction(SIGINT, &act, NULL);
>  
>  	if (!nthreads)
> -		nthreads = ncpus;
> +		nthreads = cpu->nr;
>  
>  	worker = calloc(nthreads, sizeof(*worker));
>  	if (!worker)
> -- 
> 2.21.1
> 

-- 

- Arnaldo

Re: [PATCH 2/3] perf bench futex-wake: Restore thread count default to online CPU count

[Davidlohr Bueso]

On Thu, 05 Mar 2020, Arnaldo Carvalho de Melo wrote:

>Em Thu, Mar 05, 2020 at 10:37:13AM +0200, Tommi Rantala escreveu:
>> Since commit 3b2323c2c1c4 ("perf bench futex: Use cpumaps") the default
>> number of threads the benchmark uses got changed from number of online
>> CPUs to zero:
>>
>>   $ perf bench futex wake
>>   # Running 'futex/wake' benchmark:
>>   Run summary [PID 15930]: blocking on 0 threads (at [private] futex 0x558b8ee4bfac), waking up 1 at a time.
>>   [Run 1]: Wokeup 0 of 0 threads in 0.0000 ms
>>   [...]
>>   [Run 10]: Wokeup 0 of 0 threads in 0.0000 ms
>>   Wokeup 0 of 0 threads in 0.0004 ms (+-40.82%)
>>
>> Restore the old behavior by grabbing the number of online CPUs via
>> cpu->nr:
>>
>>   $ perf bench futex wake
>>   # Running 'futex/wake' benchmark:
>>   Run summary [PID 18356]: blocking on 8 threads (at [private] futex 0xb3e62c), waking up 1 at a time.
>>   [Run 1]: Wokeup 8 of 8 threads in 0.0260 ms
>>   [...]
>>   [Run 10]: Wokeup 8 of 8 threads in 0.0270 ms
>>   Wokeup 8 of 8 threads in 0.0419 ms (+-24.35%)
>>
>> Fixes: 3b2323c2c1c4 ("perf bench futex: Use cpumaps")
>
>Thanks, tested and applied.

Thanks!

Re: [PATCH 2/3] perf bench futex-wake: Restore thread count default to online CPU count

[Arnaldo Melo]

On March 5, 2020 12:34:34 PM GMT-03:00, Davidlohr Bueso <dave@stgolabs.net> wrote:
>On Thu, 05 Mar 2020, Arnaldo Carvalho de Melo wrote:
>
>>Em Thu, Mar 05, 2020 at 10:37:13AM +0200, Tommi Rantala escreveu:
>>> Since commit 3b2323c2c1c4 ("perf bench futex: Use cpumaps") the
>default
>>> number of threads the benchmark uses got changed from number of
>online
>>> CPUs to zero:
>>>
>>>   $ perf bench futex wake
>>>   # Running 'futex/wake' benchmark:
>>>   Run summary [PID 15930]: blocking on 0 threads (at [private] futex
>0x558b8ee4bfac), waking up 1 at a time.
>>>   [Run 1]: Wokeup 0 of 0 threads in 0.0000 ms
>>>   [...]
>>>   [Run 10]: Wokeup 0 of 0 threads in 0.0000 ms
>>>   Wokeup 0 of 0 threads in 0.0004 ms (+-40.82%)
>>>
>>> Restore the old behavior by grabbing the number of online CPUs via
>>> cpu->nr:
>>>
>>>   $ perf bench futex wake
>>>   # Running 'futex/wake' benchmark:
>>>   Run summary [PID 18356]: blocking on 8 threads (at [private] futex
>0xb3e62c), waking up 1 at a time.
>>>   [Run 1]: Wokeup 8 of 8 threads in 0.0260 ms
>>>   [...]
>>>   [Run 10]: Wokeup 8 of 8 threads in 0.0270 ms
>>>   Wokeup 8 of 8 threads in 0.0419 ms (+-24.35%)
>>>
>>> Fixes: 3b2323c2c1c4 ("perf bench futex: Use cpumaps")
>>
>>Thanks, tested and applied.
>
>Thanks!

Taking that as an Acked-by, ok?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[tip: perf/urgent] perf bench futex-wake: Restore thread count default to online CPU count

[tip-bot2 for Tommi Rantala]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     f649bd9dd5d5004543bbc3c50b829577b49f5d75
Gitweb:        https://git.kernel.org/tip/f649bd9dd5d5004543bbc3c50b829577b49f5d75
Author:        Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate:    Thu, 05 Mar 2020 10:37:13 +02:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 06 Mar 2020 08:30:47 -03:00

perf bench futex-wake: Restore thread count default to online CPU count

Since commit 3b2323c2c1c4 ("perf bench futex: Use cpumaps") the default
number of threads the benchmark uses got changed from number of online
CPUs to zero:

  $ perf bench futex wake
  # Running 'futex/wake' benchmark:
  Run summary [PID 15930]: blocking on 0 threads (at [private] futex 0x558b8ee4bfac), waking up 1 at a time.
  [Run 1]: Wokeup 0 of 0 threads in 0.0000 ms
  [...]
  [Run 10]: Wokeup 0 of 0 threads in 0.0000 ms
  Wokeup 0 of 0 threads in 0.0004 ms (+-40.82%)

Restore the old behavior by grabbing the number of online CPUs via
cpu->nr:

  $ perf bench futex wake
  # Running 'futex/wake' benchmark:
  Run summary [PID 18356]: blocking on 8 threads (at [private] futex 0xb3e62c), waking up 1 at a time.
  [Run 1]: Wokeup 8 of 8 threads in 0.0260 ms
  [...]
  [Run 10]: Wokeup 8 of 8 threads in 0.0270 ms
  Wokeup 8 of 8 threads in 0.0419 ms (+-24.35%)

Fixes: 3b2323c2c1c4 ("perf bench futex: Use cpumaps")
Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lore.kernel.org/lkml/20200305083714.9381-3-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/bench/futex-wake.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
index df81009..58906e9 100644
--- a/tools/perf/bench/futex-wake.c
+++ b/tools/perf/bench/futex-wake.c
@@ -43,7 +43,7 @@ static bool done = false, silent = false, fshared = false;
 static pthread_mutex_t thread_lock;
 static pthread_cond_t thread_parent, thread_worker;
 static struct stats waketime_stats, wakeup_stats;
-static unsigned int ncpus, threads_starting, nthreads = 0;
+static unsigned int threads_starting, nthreads = 0;
 static int futex_flag = 0;
 
 static const struct option options[] = {
@@ -141,7 +141,7 @@ int bench_futex_wake(int argc, const char **argv)
 	sigaction(SIGINT, &act, NULL);
 
 	if (!nthreads)
-		nthreads = ncpus;
+		nthreads = cpu->nr;
 
 	worker = calloc(nthreads, sizeof(*worker));
 	if (!worker)

[PATCH 3/3] perf bench: Clear struct sigaction before sigaction() syscall

[Tommi Rantala]

Avoid garbage in sigaction structs used in sigaction() syscalls.
Valgrind is complaining about it.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/bench/epoll-ctl.c           | 1 +
 tools/perf/bench/epoll-wait.c          | 1 +
 tools/perf/bench/futex-hash.c          | 1 +
 tools/perf/bench/futex-lock-pi.c       | 1 +
 tools/perf/bench/futex-requeue.c       | 1 +
 tools/perf/bench/futex-wake-parallel.c | 1 +
 tools/perf/bench/futex-wake.c          | 1 +
 7 files changed, 7 insertions(+)

diff --git a/tools/perf/bench/epoll-ctl.c b/tools/perf/bench/epoll-ctl.c
index bb617e5688412..63e2520017d81 100644
--- a/tools/perf/bench/epoll-ctl.c
+++ b/tools/perf/bench/epoll-ctl.c
@@ -313,6 +313,7 @@ int bench_epoll_ctl(int argc, const char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/epoll-wait.c b/tools/perf/bench/epoll-wait.c
index 7af694437f4ea..5336e628b404c 100644
--- a/tools/perf/bench/epoll-wait.c
+++ b/tools/perf/bench/epoll-wait.c
@@ -427,6 +427,7 @@ int bench_epoll_wait(int argc, const char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-hash.c b/tools/perf/bench/futex-hash.c
index 8ba0c3330a9a2..c441aa446c7f8 100644
--- a/tools/perf/bench/futex-hash.c
+++ b/tools/perf/bench/futex-hash.c
@@ -137,6 +137,7 @@ int bench_futex_hash(int argc, const char **argv)
 	if (!cpu)
 		goto errmem;
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-lock-pi.c b/tools/perf/bench/futex-lock-pi.c
index d0cae8125423f..27c6e1944cbed 100644
--- a/tools/perf/bench/futex-lock-pi.c
+++ b/tools/perf/bench/futex-lock-pi.c
@@ -161,6 +161,7 @@ int bench_futex_lock_pi(int argc, const char **argv)
 	if (!cpu)
 		err(EXIT_FAILURE, "calloc");
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-requeue.c b/tools/perf/bench/futex-requeue.c
index a00a6891447ab..7a15c2e610228 100644
--- a/tools/perf/bench/futex-requeue.c
+++ b/tools/perf/bench/futex-requeue.c
@@ -128,6 +128,7 @@ int bench_futex_requeue(int argc, const char **argv)
 	if (!cpu)
 		err(EXIT_FAILURE, "cpu_map__new");
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-wake-parallel.c b/tools/perf/bench/futex-wake-parallel.c
index a053cf2b70397..cd2b81a845acb 100644
--- a/tools/perf/bench/futex-wake-parallel.c
+++ b/tools/perf/bench/futex-wake-parallel.c
@@ -234,6 +234,7 @@ int bench_futex_wake_parallel(int argc, const char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
index 58906e9499bb0..2dfcef3e371e4 100644
--- a/tools/perf/bench/futex-wake.c
+++ b/tools/perf/bench/futex-wake.c
@@ -136,6 +136,7 @@ int bench_futex_wake(int argc, const char **argv)
 	if (!cpu)
 		err(EXIT_FAILURE, "calloc");
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
-- 
2.21.1

Re: [PATCH 3/3] perf bench: Clear struct sigaction before sigaction() syscall

[Arnaldo Carvalho de Melo]

Em Thu, Mar 05, 2020 at 10:37:14AM +0200, Tommi Rantala escreveu:
> Avoid garbage in sigaction structs used in sigaction() syscalls.
> Valgrind is complaining about it.

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> ---
>  tools/perf/bench/epoll-ctl.c           | 1 +
>  tools/perf/bench/epoll-wait.c          | 1 +
>  tools/perf/bench/futex-hash.c          | 1 +
>  tools/perf/bench/futex-lock-pi.c       | 1 +
>  tools/perf/bench/futex-requeue.c       | 1 +
>  tools/perf/bench/futex-wake-parallel.c | 1 +
>  tools/perf/bench/futex-wake.c          | 1 +
>  7 files changed, 7 insertions(+)
> 
> diff --git a/tools/perf/bench/epoll-ctl.c b/tools/perf/bench/epoll-ctl.c
> index bb617e5688412..63e2520017d81 100644
> --- a/tools/perf/bench/epoll-ctl.c
> +++ b/tools/perf/bench/epoll-ctl.c
> @@ -313,6 +313,7 @@ int bench_epoll_ctl(int argc, const char **argv)
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> diff --git a/tools/perf/bench/epoll-wait.c b/tools/perf/bench/epoll-wait.c
> index 7af694437f4ea..5336e628b404c 100644
> --- a/tools/perf/bench/epoll-wait.c
> +++ b/tools/perf/bench/epoll-wait.c
> @@ -427,6 +427,7 @@ int bench_epoll_wait(int argc, const char **argv)
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> diff --git a/tools/perf/bench/futex-hash.c b/tools/perf/bench/futex-hash.c
> index 8ba0c3330a9a2..c441aa446c7f8 100644
> --- a/tools/perf/bench/futex-hash.c
> +++ b/tools/perf/bench/futex-hash.c
> @@ -137,6 +137,7 @@ int bench_futex_hash(int argc, const char **argv)
>  	if (!cpu)
>  		goto errmem;
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> diff --git a/tools/perf/bench/futex-lock-pi.c b/tools/perf/bench/futex-lock-pi.c
> index d0cae8125423f..27c6e1944cbed 100644
> --- a/tools/perf/bench/futex-lock-pi.c
> +++ b/tools/perf/bench/futex-lock-pi.c
> @@ -161,6 +161,7 @@ int bench_futex_lock_pi(int argc, const char **argv)
>  	if (!cpu)
>  		err(EXIT_FAILURE, "calloc");
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> diff --git a/tools/perf/bench/futex-requeue.c b/tools/perf/bench/futex-requeue.c
> index a00a6891447ab..7a15c2e610228 100644
> --- a/tools/perf/bench/futex-requeue.c
> +++ b/tools/perf/bench/futex-requeue.c
> @@ -128,6 +128,7 @@ int bench_futex_requeue(int argc, const char **argv)
>  	if (!cpu)
>  		err(EXIT_FAILURE, "cpu_map__new");
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> diff --git a/tools/perf/bench/futex-wake-parallel.c b/tools/perf/bench/futex-wake-parallel.c
> index a053cf2b70397..cd2b81a845acb 100644
> --- a/tools/perf/bench/futex-wake-parallel.c
> +++ b/tools/perf/bench/futex-wake-parallel.c
> @@ -234,6 +234,7 @@ int bench_futex_wake_parallel(int argc, const char **argv)
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
> index 58906e9499bb0..2dfcef3e371e4 100644
> --- a/tools/perf/bench/futex-wake.c
> +++ b/tools/perf/bench/futex-wake.c
> @@ -136,6 +136,7 @@ int bench_futex_wake(int argc, const char **argv)
>  	if (!cpu)
>  		err(EXIT_FAILURE, "calloc");
>  
> +	memset(&act, 0, sizeof(act));
>  	sigfillset(&act.sa_mask);
>  	act.sa_sigaction = toggle_done;
>  	sigaction(SIGINT, &act, NULL);
> -- 
> 2.21.1
> 

-- 

- Arnaldo

[tip: perf/urgent] perf bench: Clear struct sigaction before sigaction() syscall

[tip-bot2 for Tommi Rantala]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     7b919a53102d81cd2e310b4941ac51c465d249ca
Gitweb:        https://git.kernel.org/tip/7b919a53102d81cd2e310b4941ac51c465d249ca
Author:        Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate:    Thu, 05 Mar 2020 10:37:14 +02:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 06 Mar 2020 08:30:47 -03:00

perf bench: Clear struct sigaction before sigaction() syscall

Avoid garbage in sigaction structs used in sigaction() syscalls.
Valgrind is complaining about it.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Changbin Du <changbin.du@intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lore.kernel.org/lkml/20200305083714.9381-4-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/bench/epoll-ctl.c           | 1 +
 tools/perf/bench/epoll-wait.c          | 1 +
 tools/perf/bench/futex-hash.c          | 1 +
 tools/perf/bench/futex-lock-pi.c       | 1 +
 tools/perf/bench/futex-requeue.c       | 1 +
 tools/perf/bench/futex-wake-parallel.c | 1 +
 tools/perf/bench/futex-wake.c          | 1 +
 7 files changed, 7 insertions(+)

diff --git a/tools/perf/bench/epoll-ctl.c b/tools/perf/bench/epoll-ctl.c
index a7526c0..cadc18d 100644
--- a/tools/perf/bench/epoll-ctl.c
+++ b/tools/perf/bench/epoll-ctl.c
@@ -312,6 +312,7 @@ int bench_epoll_ctl(int argc, const char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/epoll-wait.c b/tools/perf/bench/epoll-wait.c
index d1c5cb5..f938c58 100644
--- a/tools/perf/bench/epoll-wait.c
+++ b/tools/perf/bench/epoll-wait.c
@@ -426,6 +426,7 @@ int bench_epoll_wait(int argc, const char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-hash.c b/tools/perf/bench/futex-hash.c
index 2177686..65eebe0 100644
--- a/tools/perf/bench/futex-hash.c
+++ b/tools/perf/bench/futex-hash.c
@@ -137,6 +137,7 @@ int bench_futex_hash(int argc, const char **argv)
 	if (!cpu)
 		goto errmem;
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-lock-pi.c b/tools/perf/bench/futex-lock-pi.c
index 30d9712..89fd8f3 100644
--- a/tools/perf/bench/futex-lock-pi.c
+++ b/tools/perf/bench/futex-lock-pi.c
@@ -160,6 +160,7 @@ int bench_futex_lock_pi(int argc, const char **argv)
 	if (!cpu)
 		err(EXIT_FAILURE, "calloc");
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-requeue.c b/tools/perf/bench/futex-requeue.c
index a00a689..7a15c2e 100644
--- a/tools/perf/bench/futex-requeue.c
+++ b/tools/perf/bench/futex-requeue.c
@@ -128,6 +128,7 @@ int bench_futex_requeue(int argc, const char **argv)
 	if (!cpu)
 		err(EXIT_FAILURE, "cpu_map__new");
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-wake-parallel.c b/tools/perf/bench/futex-wake-parallel.c
index a053cf2..cd2b81a 100644
--- a/tools/perf/bench/futex-wake-parallel.c
+++ b/tools/perf/bench/futex-wake-parallel.c
@@ -234,6 +234,7 @@ int bench_futex_wake_parallel(int argc, const char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);
diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
index 58906e9..2dfcef3 100644
--- a/tools/perf/bench/futex-wake.c
+++ b/tools/perf/bench/futex-wake.c
@@ -136,6 +136,7 @@ int bench_futex_wake(int argc, const char **argv)
 	if (!cpu)
 		err(EXIT_FAILURE, "calloc");
 
+	memset(&act, 0, sizeof(act));
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
 	sigaction(SIGINT, &act, NULL);