2020-06-19 13:37:04

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH 1/2] libsubcmd: Fix OPT_CALLBACK_SET()

Any option macro with _SET suffix should set opt->set variable which
is not happening for OPT_CALLBACK_SET(). This is causing issues with
perf record --switch-output-event. Fix that.

Before:
# ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
--switch-output-event syscalls:sys_enter_mmap
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.297 MB perf.data (657 samples) ]

After:

$ ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
--switch-output-event syscalls:sys_enter_mmap
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144542 ]
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144608 ]
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144660 ]
^C[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144784 ]
[ perf record: Woken up 0 times to write data ]
[ perf record: Dump perf.data.2020061918144803 ]
[ perf record: Captured and wrote 0.419 MB perf.data.<timestamp> ]

Fixes: 636eb4d001b1 ("libsubcmd: Introduce OPT_CALLBACK_SET()")
Signed-off-by: Ravi Bangoria <[email protected]>
---
tools/lib/subcmd/parse-options.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index dbb9efbf718a..39ebf6192016 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -237,6 +237,9 @@ static int get_value(struct parse_opt_ctx_t *p,
return err;

case OPTION_CALLBACK:
+ if (opt->set)
+ *(bool *)opt->set = true;
+
if (unset)
return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
if (opt->flags & PARSE_OPT_NOARG)
--
2.17.1


2020-06-19 13:37:13

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH 2/2] perf record: Fix --switch-output=time documentation

perf record man page uses word 'size' while describing
--switch-output=time option. Fix that.

Signed-off-by: Ravi Bangoria <[email protected]>
---
tools/perf/Documentation/perf-record.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index fa8a5fcd27ab..5256f5e12dde 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -540,7 +540,7 @@ based on 'mode' value:
"signal" - when receiving a SIGUSR2 (default value) or
<size> - when reaching the size threshold, size is expected to
be a number with appended unit character - B/K/M/G
- <time> - when reaching the time threshold, size is expected to
+ <time> - when reaching the time threshold, time is expected to
be a number with appended unit character - s/m/h/d

Note: the precision of the size threshold hugely depends
--
2.17.1

2020-07-17 03:43:17

by Ravi Bangoria

[permalink] [raw]
Subject: Re: [PATCH 1/2] libsubcmd: Fix OPT_CALLBACK_SET()

Hi Arnaldo,

Can you please consider this trivial fix.

Ravi

On 6/19/20 7:04 PM, Ravi Bangoria wrote:
> Any option macro with _SET suffix should set opt->set variable which
> is not happening for OPT_CALLBACK_SET(). This is causing issues with
> perf record --switch-output-event. Fix that.
>
> Before:
> # ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
> --switch-output-event syscalls:sys_enter_mmap
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.297 MB perf.data (657 samples) ]
>
> After:
>
> $ ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
> --switch-output-event syscalls:sys_enter_mmap
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2020061918144542 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2020061918144608 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2020061918144660 ]
> ^C[ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2020061918144784 ]
> [ perf record: Woken up 0 times to write data ]
> [ perf record: Dump perf.data.2020061918144803 ]
> [ perf record: Captured and wrote 0.419 MB perf.data.<timestamp> ]
>
> Fixes: 636eb4d001b1 ("libsubcmd: Introduce OPT_CALLBACK_SET()")
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> tools/lib/subcmd/parse-options.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
> index dbb9efbf718a..39ebf6192016 100644
> --- a/tools/lib/subcmd/parse-options.c
> +++ b/tools/lib/subcmd/parse-options.c
> @@ -237,6 +237,9 @@ static int get_value(struct parse_opt_ctx_t *p,
> return err;
>
> case OPTION_CALLBACK:
> + if (opt->set)
> + *(bool *)opt->set = true;
> +
> if (unset)
> return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
> if (opt->flags & PARSE_OPT_NOARG)
>

2020-07-17 12:35:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] libsubcmd: Fix OPT_CALLBACK_SET()

Em Fri, Jul 17, 2020 at 09:10:45AM +0530, Ravi Bangoria escreveu:
> Hi Arnaldo,
>
> Can you please consider this trivial fix.

Trivial but important, sorry for taking so long, applied to perf/urgent,
aimed at v5.8,

- Arnaldo

> Ravi
>
> On 6/19/20 7:04 PM, Ravi Bangoria wrote:
> > Any option macro with _SET suffix should set opt->set variable which
> > is not happening for OPT_CALLBACK_SET(). This is causing issues with
> > perf record --switch-output-event. Fix that.
> >
> > Before:
> > # ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
> > --switch-output-event syscalls:sys_enter_mmap
> > ^C[ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.297 MB perf.data (657 samples) ]
> >
> > After:
> >
> > $ ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
> > --switch-output-event syscalls:sys_enter_mmap
> > [ perf record: dump data: Woken up 1 times ]
> > [ perf record: Dump perf.data.2020061918144542 ]
> > [ perf record: dump data: Woken up 1 times ]
> > [ perf record: Dump perf.data.2020061918144608 ]
> > [ perf record: dump data: Woken up 1 times ]
> > [ perf record: Dump perf.data.2020061918144660 ]
> > ^C[ perf record: dump data: Woken up 1 times ]
> > [ perf record: Dump perf.data.2020061918144784 ]
> > [ perf record: Woken up 0 times to write data ]
> > [ perf record: Dump perf.data.2020061918144803 ]
> > [ perf record: Captured and wrote 0.419 MB perf.data.<timestamp> ]
> >
> > Fixes: 636eb4d001b1 ("libsubcmd: Introduce OPT_CALLBACK_SET()")
> > Signed-off-by: Ravi Bangoria <[email protected]>
> > ---
> > tools/lib/subcmd/parse-options.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
> > index dbb9efbf718a..39ebf6192016 100644
> > --- a/tools/lib/subcmd/parse-options.c
> > +++ b/tools/lib/subcmd/parse-options.c
> > @@ -237,6 +237,9 @@ static int get_value(struct parse_opt_ctx_t *p,
> > return err;
> > case OPTION_CALLBACK:
> > + if (opt->set)
> > + *(bool *)opt->set = true;
> > +
> > if (unset)
> > return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
> > if (opt->flags & PARSE_OPT_NOARG)
> >

--

- Arnaldo