2013-06-19 01:02:31

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 1/3] perf record: support duration option to run during specified time

Currently, there is no method to quit at specified time later.
We are used to using 'sleep N' as command argument if we need it,
but explicitly supporting this feature maybe makes sense.

Cc: Namhyung Kim <[email protected]>
Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..19c65b5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -29,6 +29,7 @@
#include <sched.h>
#include <sys/mman.h>

+static unsigned int duration;
#ifndef HAVE_ON_EXIT
#ifndef ATEXIT_MAX
#define ATEXIT_MAX 32
@@ -194,7 +195,7 @@ static void perf_record__sig_exit(int exit_status __maybe_unused, void *arg)
psignal(WTERMSIG(status), rec->progname);
}

- if (signr == -1 || signr == SIGUSR1)
+ if (signr == -1 || signr == SIGUSR1 || signr == SIGALRM)
return;

signal(signr, SIG_DFL);
@@ -404,6 +405,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
signal(SIGCHLD, sig_handler);
signal(SIGINT, sig_handler);
signal(SIGUSR1, sig_handler);
+ signal(SIGALRM, sig_handler);

if (!output_name) {
if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
@@ -471,6 +473,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
goto out_delete_session;
}

+ alarm(duration);
if (forks) {
err = perf_evlist__prepare_workload(evsel_list, &opts->target,
argv, opts->pipe_output,
@@ -955,6 +958,7 @@ const struct option record_options[] = {
parse_branch_stack),
OPT_BOOLEAN('W', "weight", &record.opts.sample_weight,
"sample by weight (on special events only)"),
+ OPT_UINTEGER(0, "duration", &duration, "run during specified seconds"),
OPT_END()
};

--
1.7.9.5


2013-06-19 01:02:37

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 2/3] tools lib lk: Fix for cross build

Currently, lib lk doesn't use CROSS_COMPILE environment variable,
so cross build is always failed. This is a quick fix for this problem.

Cc: Namhyung Kim <[email protected]>
Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/tools/lib/lk/Makefile b/tools/lib/lk/Makefile
index 926cbf3..91e5174 100644
--- a/tools/lib/lk/Makefile
+++ b/tools/lib/lk/Makefile
@@ -1,5 +1,20 @@
include ../../scripts/Makefile.include

+# Makefiles suck: This macro sets a default value of $(2) for the
+# variable named by $(1), unless the variable has been set by
+# environment or command line. This is necessary for CC and AR
+# because make sets default values, so the simpler ?= approach
+# won't work as expected.
+define allow-override
+ $(if $(or $(findstring environment,$(origin $(1))),\
+ $(findstring command line,$(origin $(1)))),,\
+ $(eval $(1) = $(2)))
+endef
+
+# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
+$(call allow-override,CC,$(CROSS_COMPILE)gcc)
+$(call allow-override,AR,$(CROSS_COMPILE)ar)
+
# guard against environment variables
LIB_H=
LIB_OBJS=
--
1.7.9.5

2013-06-19 01:02:57

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 3/3] perf tools: include termios.h explicitly

Building perf for android is failed, because it can't find definition of
struct winsize. This definition is in termios.h, so I add this header
to util.h to solve the problem. It is missed by commit '2c803e52' which
moves get_term_dimensions() from builtin-top.c to util.c, but, missed to
move termios.h header.

Cc: David Ahern <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index a45710b..f2c6483 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -72,6 +72,7 @@
#include "types.h"
#include <sys/ttydefaults.h>
#include <lk/debugfs.h>
+#include <termios.h>

extern const char *graph_line;
extern const char *graph_dotted_line;
@@ -274,6 +275,5 @@ void dump_stack(void);

extern unsigned int page_size;

-struct winsize;
void get_term_dimensions(struct winsize *ws);
#endif /* GIT_COMPAT_UTIL_H */
--
1.7.9.5

2013-06-19 11:33:35

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf tools: include termios.h explicitly


* Joonsoo Kim <[email protected]> wrote:

> Building perf for android is failed, because it can't find definition of
> struct winsize. This definition is in termios.h, so I add this header
> to util.h to solve the problem. It is missed by commit '2c803e52' which
> moves get_term_dimensions() from builtin-top.c to util.c, but, missed to
> move termios.h header.
>
> Cc: David Ahern <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Joonsoo Kim <[email protected]>

Is this something we need for v3.10 as well?

Thanks,

Ingo

2013-06-19 12:16:42

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf tools: include termios.h explicitly

Hi Ingo,

On Wed, Jun 19, 2013 at 8:33 PM, Ingo Molnar <[email protected]> wrote:
>
> * Joonsoo Kim <[email protected]> wrote:
>
>> Building perf for android is failed, because it can't find definition of
>> struct winsize. This definition is in termios.h, so I add this header
>> to util.h to solve the problem. It is missed by commit '2c803e52' which
>> moves get_term_dimensions() from builtin-top.c to util.c, but, missed to
>> move termios.h header.
>>
>> Cc: David Ahern <[email protected]>
>> Cc: Namhyung Kim <[email protected]>
>> Signed-off-by: Joonsoo Kim <[email protected]>
>
> Is this something we need for v3.10 as well?
>
> Thanks,
>
> Ingo

I think it's good to go to v3.10 and even v3.9 as well:

$ git name-rev --tags --refs=refs/tags/v3.* 2c803e52
2c803e52 tags/v3.9-rc1~173^2~13^2~22

--
Thanks,
Namhyung

2013-06-19 14:52:12

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf tools: include termios.h explicitly

On 6/18/13 7:02 PM, Joonsoo Kim wrote:
> Building perf for android is failed, because it can't find definition of
> struct winsize. This definition is in termios.h, so I add this header
> to util.h to solve the problem. It is missed by commit '2c803e52' which
> moves get_term_dimensions() from builtin-top.c to util.c, but, missed to
> move termios.h header.

Thank you for fixing this.

David

>
> Cc: David Ahern <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Joonsoo Kim <[email protected]>
>
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index a45710b..f2c6483 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -72,6 +72,7 @@
> #include "types.h"
> #include <sys/ttydefaults.h>
> #include <lk/debugfs.h>
> +#include <termios.h>
>
> extern const char *graph_line;
> extern const char *graph_dotted_line;
> @@ -274,6 +275,5 @@ void dump_stack(void);
>
> extern unsigned int page_size;
>
> -struct winsize;
> void get_term_dimensions(struct winsize *ws);
> #endif /* GIT_COMPAT_UTIL_H */
>

2013-06-20 01:45:18

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf tools: include termios.h explicitly

On Wed, Jun 19, 2013 at 09:16:20PM +0900, Namhyung Kim wrote:
> Hi Ingo,
>
> On Wed, Jun 19, 2013 at 8:33 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Joonsoo Kim <[email protected]> wrote:
> >
> >> Building perf for android is failed, because it can't find definition of
> >> struct winsize. This definition is in termios.h, so I add this header
> >> to util.h to solve the problem. It is missed by commit '2c803e52' which
> >> moves get_term_dimensions() from builtin-top.c to util.c, but, missed to
> >> move termios.h header.
> >>
> >> Cc: David Ahern <[email protected]>
> >> Cc: Namhyung Kim <[email protected]>
> >> Signed-off-by: Joonsoo Kim <[email protected]>
> >
> > Is this something we need for v3.10 as well?
> >
> > Thanks,
> >
> > Ingo
>
> I think it's good to go to v3.10 and even v3.9 as well:
>
> $ git name-rev --tags --refs=refs/tags/v3.* 2c803e52
> 2c803e52 tags/v3.9-rc1~173^2~13^2~22

Agreed.

>
> --
> Thanks,
> Namhyung
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-06-29 00:27:33

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf record: support duration option to run during specified time

Joonsoo Kim [[email protected]] wrote:
| Currently, there is no method to quit at specified time later.
| We are used to using 'sleep N' as command argument if we need it,
| but explicitly supporting this feature maybe makes sense.
|
| Cc: Namhyung Kim <[email protected]>
| Signed-off-by: Joonsoo Kim <[email protected]>
|
| diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
| index cdf58ec..19c65b5 100644
| --- a/tools/perf/builtin-record.c
| +++ b/tools/perf/builtin-record.c
| @@ -29,6 +29,7 @@
| #include <sched.h>
| #include <sys/mman.h>
|
| +static unsigned int duration;
| #ifndef HAVE_ON_EXIT
| #ifndef ATEXIT_MAX
| #define ATEXIT_MAX 32
| @@ -194,7 +195,7 @@ static void perf_record__sig_exit(int exit_status __maybe_unused, void *arg)
| psignal(WTERMSIG(status), rec->progname);
| }
|
| - if (signr == -1 || signr == SIGUSR1)
| + if (signr == -1 || signr == SIGUSR1 || signr == SIGALRM)
| return;
|
| signal(signr, SIG_DFL);
| @@ -404,6 +405,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
| signal(SIGCHLD, sig_handler);
| signal(SIGINT, sig_handler);
| signal(SIGUSR1, sig_handler);
| + signal(SIGALRM, sig_handler);
|
| if (!output_name) {
| if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
| @@ -471,6 +473,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
| goto out_delete_session;
| }
|
| + alarm(duration);
| if (forks) {
| err = perf_evlist__prepare_workload(evsel_list, &opts->target,
| argv, opts->pipe_output,
| @@ -955,6 +958,7 @@ const struct option record_options[] = {
| parse_branch_stack),
| OPT_BOOLEAN('W', "weight", &record.opts.sample_weight,
| "sample by weight (on special events only)"),
| + OPT_UINTEGER(0, "duration", &duration, "run during specified seconds"),

nit: A slight ambiguity in the help text. Would you mind changing it to:

"run for specified duration (seconds)" ?

Sukadev

2013-07-01 05:46:22

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf record: support duration option to run during specified time

On Fri, Jun 28, 2013 at 05:27:24PM -0700, Sukadev Bhattiprolu wrote:
> Joonsoo Kim [[email protected]] wrote:
> | Currently, there is no method to quit at specified time later.
> | We are used to using 'sleep N' as command argument if we need it,
> | but explicitly supporting this feature maybe makes sense.
> |
> | Cc: Namhyung Kim <[email protected]>
> | Signed-off-by: Joonsoo Kim <[email protected]>
> |
> | diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> | index cdf58ec..19c65b5 100644
> | --- a/tools/perf/builtin-record.c
> | +++ b/tools/perf/builtin-record.c
> | @@ -29,6 +29,7 @@
> | #include <sched.h>
> | #include <sys/mman.h>
> |
> | +static unsigned int duration;
> | #ifndef HAVE_ON_EXIT
> | #ifndef ATEXIT_MAX
> | #define ATEXIT_MAX 32
> | @@ -194,7 +195,7 @@ static void perf_record__sig_exit(int exit_status __maybe_unused, void *arg)
> | psignal(WTERMSIG(status), rec->progname);
> | }
> |
> | - if (signr == -1 || signr == SIGUSR1)
> | + if (signr == -1 || signr == SIGUSR1 || signr == SIGALRM)
> | return;
> |
> | signal(signr, SIG_DFL);
> | @@ -404,6 +405,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
> | signal(SIGCHLD, sig_handler);
> | signal(SIGINT, sig_handler);
> | signal(SIGUSR1, sig_handler);
> | + signal(SIGALRM, sig_handler);
> |
> | if (!output_name) {
> | if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
> | @@ -471,6 +473,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
> | goto out_delete_session;
> | }
> |
> | + alarm(duration);
> | if (forks) {
> | err = perf_evlist__prepare_workload(evsel_list, &opts->target,
> | argv, opts->pipe_output,
> | @@ -955,6 +958,7 @@ const struct option record_options[] = {
> | parse_branch_stack),
> | OPT_BOOLEAN('W', "weight", &record.opts.sample_weight,
> | "sample by weight (on special events only)"),
> | + OPT_UINTEGER(0, "duration", &duration, "run during specified seconds"),
>
> nit: A slight ambiguity in the help text. Would you mind changing it to:
>
> "run for specified duration (seconds)" ?
>
> Sukadev

Hello, Sukadev.
Okay.

Here goes v2 with your suggestion.
---------------->8------------------------------
>From b65e29b33cb250d4a4be103ce5afae29f6bdc11a Mon Sep 17 00:00:00 2001
From: Joonsoo Kim <[email protected]>
Date: Thu, 13 Jun 2013 18:06:21 +0900
Subject: [PATCH v2] perf, tools: support duration option to run during specified
time

Currently, there is no method to quit at specified time later.
We are used to using 'sleep N' as command argument if we need it,
but explicitly supporting this feature maybe makes sense.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..1384118 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -29,6 +29,7 @@
#include <sched.h>
#include <sys/mman.h>

+static unsigned int duration;
#ifndef HAVE_ON_EXIT
#ifndef ATEXIT_MAX
#define ATEXIT_MAX 32
@@ -194,7 +195,7 @@ static void perf_record__sig_exit(int exit_status __maybe_unused, void *arg)
psignal(WTERMSIG(status), rec->progname);
}

- if (signr == -1 || signr == SIGUSR1)
+ if (signr == -1 || signr == SIGUSR1 || signr == SIGALRM)
return;

signal(signr, SIG_DFL);
@@ -404,6 +405,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
signal(SIGCHLD, sig_handler);
signal(SIGINT, sig_handler);
signal(SIGUSR1, sig_handler);
+ signal(SIGALRM, sig_handler);

if (!output_name) {
if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
@@ -471,6 +473,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
goto out_delete_session;
}

+ alarm(duration);
if (forks) {
err = perf_evlist__prepare_workload(evsel_list, &opts->target,
argv, opts->pipe_output,
@@ -955,6 +958,8 @@ const struct option record_options[] = {
parse_branch_stack),
OPT_BOOLEAN('W', "weight", &record.opts.sample_weight,
"sample by weight (on special events only)"),
+ OPT_UINTEGER(0, "duration", &duration,
+ "run for specified duration (seconds)"),
OPT_END()
};

--
1.7.9.5

Subject: [tip:perf/urgent] perf tools: Include termios.h explicitly

Commit-ID: 756bbc84e30b57ca1010b550ea30594fd0b0494f
Gitweb: http://git.kernel.org/tip/756bbc84e30b57ca1010b550ea30594fd0b0494f
Author: Joonsoo Kim <[email protected]>
AuthorDate: Wed, 19 Jun 2013 10:02:30 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 8 Jul 2013 17:36:05 -0300

perf tools: Include termios.h explicitly

Building perf for android fails because it can't find the definition of
struct winsize.

This definition is in termios.h, so I add this header to util.h to solve
the problem.

It is missed by commit '2c803e52' which moves get_term_dimensions() from
builtin-top.c to util.c, but missed to move termios.h header.

Signed-off-by: Joonsoo Kim <[email protected]>
Acked-by: David Ahern <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/util.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 7a484c9..2732fad 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -72,6 +72,7 @@
#include "types.h"
#include <sys/ttydefaults.h>
#include <lk/debugfs.h>
+#include <termios.h>

extern const char *graph_line;
extern const char *graph_dotted_line;
@@ -274,6 +275,5 @@ void dump_stack(void);

extern unsigned int page_size;

-struct winsize;
void get_term_dimensions(struct winsize *ws);
#endif /* GIT_COMPAT_UTIL_H */

Subject: [tip:perf/urgent] tools lib lk: Fix for cross build

Commit-ID: 079787f209416416383c74ea5d5044be2d586f5e
Gitweb: http://git.kernel.org/tip/079787f209416416383c74ea5d5044be2d586f5e
Author: Joonsoo Kim <[email protected]>
AuthorDate: Wed, 19 Jun 2013 10:02:29 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 8 Jul 2013 17:36:17 -0300

tools lib lk: Fix for cross build

Currently, lib lk doesn't use CROSS_COMPILE environment variable, so
cross build always fails.

This is a quick fix for this problem.

Signed-off-by: Joonsoo Kim <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/lk/Makefile | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/tools/lib/lk/Makefile b/tools/lib/lk/Makefile
index 2c5a197..f0ecc0a 100644
--- a/tools/lib/lk/Makefile
+++ b/tools/lib/lk/Makefile
@@ -3,6 +3,21 @@ include ../../scripts/Makefile.include
CC = $(CROSS_COMPILE)gcc
AR = $(CROSS_COMPILE)ar

+# Makefiles suck: This macro sets a default value of $(2) for the
+# variable named by $(1), unless the variable has been set by
+# environment or command line. This is necessary for CC and AR
+# because make sets default values, so the simpler ?= approach
+# won't work as expected.
+define allow-override
+ $(if $(or $(findstring environment,$(origin $(1))),\
+ $(findstring command line,$(origin $(1)))),,\
+ $(eval $(1) = $(2)))
+endef
+
+# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
+$(call allow-override,CC,$(CROSS_COMPILE)gcc)
+$(call allow-override,AR,$(CROSS_COMPILE)ar)
+
# guard against environment variables
LIB_H=
LIB_OBJS=