* [PATCH] perf synthetic-events: save 4kb from 2 stack frames
@ 2020-03-27 6:36 Ian Rogers
2020-03-27 13:23 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2020-03-27 6:36 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel
Cc: Stephane Eranian, Ian Rogers
Reduce the scope of PATH_MAX sized char buffers so that the compiler can
overlap their usage.
perf_event__synthesize_mmap_events before 'sub $0x45b8,%rsp' after
'sub $0x35b8,%rsp'.
perf_event__get_comm_ids before 'sub $0x2028,%rsp' after 'sub $0x1028,%rsp'.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/synthetic-events.c | 46 ++++++++++++++++++------------
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 3f28af39f9c6..9ff54707bb30 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -70,7 +70,6 @@ int perf_tool__process_synth_event(struct perf_tool *tool,
static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
pid_t *tgid, pid_t *ppid)
{
- char filename[PATH_MAX];
char bf[4096];
int fd;
size_t size = 0;
@@ -80,12 +79,16 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
*tgid = -1;
*ppid = -1;
- snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
+ {
+ char filename[PATH_MAX];
- fd = open(filename, O_RDONLY);
- if (fd < 0) {
- pr_debug("couldn't open %s\n", filename);
- return -1;
+ snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
+
+ fd = open(filename, O_RDONLY);
+ if (fd < 0) {
+ pr_debug("couldn't open %s\n", filename);
+ return -1;
+ }
}
n = read(fd, bf, sizeof(bf) - 1);
@@ -280,7 +283,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
struct machine *machine,
bool mmap_data)
{
- char filename[PATH_MAX];
FILE *fp;
unsigned long long t;
bool truncation = false;
@@ -292,18 +294,22 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
if (machine__is_default_guest(machine))
return 0;
- snprintf(filename, sizeof(filename), "%s/proc/%d/task/%d/maps",
- machine->root_dir, pid, pid);
+#define FILENAME_FMT_STRING "%s/proc/%d/task/%d/maps"
+ {
+ char filename[PATH_MAX];
- fp = fopen(filename, "r");
- if (fp == NULL) {
- /*
- * We raced with a task exiting - just return:
- */
- pr_debug("couldn't open %s\n", filename);
- return -1;
- }
+ snprintf(filename, sizeof(filename), FILENAME_FMT_STRING,
+ machine->root_dir, pid, pid);
+ fp = fopen(filename, "r");
+ if (fp == NULL) {
+ /*
+ * We raced with a task exiting - just return:
+ */
+ pr_debug("couldn't open %s\n", filename);
+ return -1;
+ }
+ }
event->header.type = PERF_RECORD_MMAP2;
t = rdclock();
@@ -320,10 +326,10 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
break;
if ((rdclock() - t) > timeout) {
- pr_warning("Reading %s time out. "
+ pr_warning("Reading " FILENAME_FMT_STRING " time out. "
"You may want to increase "
"the time limit by --proc-map-timeout\n",
- filename);
+ machine->root_dir, pid, pid);
truncation = true;
goto out;
}
@@ -412,6 +418,8 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
fclose(fp);
return rc;
+
+#undef FILENAME_FMT_STRING
}
int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t process,
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf synthetic-events: save 4kb from 2 stack frames
2020-03-27 6:36 [PATCH] perf synthetic-events: save 4kb from 2 stack frames Ian Rogers
@ 2020-03-27 13:23 ` Jiri Olsa
2020-03-27 17:33 ` Ian Rogers
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2020-03-27 13:23 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-kernel,
Stephane Eranian
On Thu, Mar 26, 2020 at 11:36:51PM -0700, Ian Rogers wrote:
> Reduce the scope of PATH_MAX sized char buffers so that the compiler can
> overlap their usage.
>
> perf_event__synthesize_mmap_events before 'sub $0x45b8,%rsp' after
> 'sub $0x35b8,%rsp'.
>
> perf_event__get_comm_ids before 'sub $0x2028,%rsp' after 'sub $0x1028,%rsp'.
nice catch.. is this actualy problem somewhere? I thought
we don't need to care that much about this in user space
jirka
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/synthetic-events.c | 46 ++++++++++++++++++------------
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 3f28af39f9c6..9ff54707bb30 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -70,7 +70,6 @@ int perf_tool__process_synth_event(struct perf_tool *tool,
> static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
> pid_t *tgid, pid_t *ppid)
> {
> - char filename[PATH_MAX];
> char bf[4096];
> int fd;
> size_t size = 0;
> @@ -80,12 +79,16 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
> *tgid = -1;
> *ppid = -1;
>
> - snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
> + {
> + char filename[PATH_MAX];
>
> - fd = open(filename, O_RDONLY);
> - if (fd < 0) {
> - pr_debug("couldn't open %s\n", filename);
> - return -1;
> + snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
> +
> + fd = open(filename, O_RDONLY);
> + if (fd < 0) {
> + pr_debug("couldn't open %s\n", filename);
> + return -1;
> + }
> }
>
> n = read(fd, bf, sizeof(bf) - 1);
> @@ -280,7 +283,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> struct machine *machine,
> bool mmap_data)
> {
> - char filename[PATH_MAX];
> FILE *fp;
> unsigned long long t;
> bool truncation = false;
> @@ -292,18 +294,22 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> if (machine__is_default_guest(machine))
> return 0;
>
> - snprintf(filename, sizeof(filename), "%s/proc/%d/task/%d/maps",
> - machine->root_dir, pid, pid);
> +#define FILENAME_FMT_STRING "%s/proc/%d/task/%d/maps"
> + {
> + char filename[PATH_MAX];
>
> - fp = fopen(filename, "r");
> - if (fp == NULL) {
> - /*
> - * We raced with a task exiting - just return:
> - */
> - pr_debug("couldn't open %s\n", filename);
> - return -1;
> - }
> + snprintf(filename, sizeof(filename), FILENAME_FMT_STRING,
> + machine->root_dir, pid, pid);
>
> + fp = fopen(filename, "r");
> + if (fp == NULL) {
> + /*
> + * We raced with a task exiting - just return:
> + */
> + pr_debug("couldn't open %s\n", filename);
> + return -1;
> + }
> + }
> event->header.type = PERF_RECORD_MMAP2;
> t = rdclock();
>
> @@ -320,10 +326,10 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> break;
>
> if ((rdclock() - t) > timeout) {
> - pr_warning("Reading %s time out. "
> + pr_warning("Reading " FILENAME_FMT_STRING " time out. "
> "You may want to increase "
> "the time limit by --proc-map-timeout\n",
> - filename);
> + machine->root_dir, pid, pid);
> truncation = true;
> goto out;
> }
> @@ -412,6 +418,8 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>
> fclose(fp);
> return rc;
> +
> +#undef FILENAME_FMT_STRING
> }
>
> int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t process,
> --
> 2.25.1.696.g5e7596f4ac-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] perf synthetic-events: save 4kb from 2 stack frames
2020-03-27 13:23 ` Jiri Olsa
@ 2020-03-27 17:33 ` Ian Rogers
0 siblings, 0 replies; 3+ messages in thread
From: Ian Rogers @ 2020-03-27 17:33 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, LKML,
Stephane Eranian
On Fri, Mar 27, 2020 at 6:24 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Mar 26, 2020 at 11:36:51PM -0700, Ian Rogers wrote:
> > Reduce the scope of PATH_MAX sized char buffers so that the compiler can
> > overlap their usage.
> >
> > perf_event__synthesize_mmap_events before 'sub $0x45b8,%rsp' after
> > 'sub $0x35b8,%rsp'.
> >
> > perf_event__get_comm_ids before 'sub $0x2028,%rsp' after 'sub $0x1028,%rsp'.
>
> nice catch.. is this actualy problem somewhere? I thought
> we don't need to care that much about this in user space
I did perhaps a cleaner v2 here:
https://lore.kernel.org/lkml/20200327172914.28603-1-irogers@google.com/T/#u
I have some local changes that required some more arrays in
perf_event__synthesize_mmap_events, this hit a frame limit size with
our compiler set up. This change was the simplest workaround. I agree
this patch isn't essential but it should generally benefit performance
a little.
Thanks,
Ian
> jirka
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/synthetic-events.c | 46 ++++++++++++++++++------------
> > 1 file changed, 27 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > index 3f28af39f9c6..9ff54707bb30 100644
> > --- a/tools/perf/util/synthetic-events.c
> > +++ b/tools/perf/util/synthetic-events.c
> > @@ -70,7 +70,6 @@ int perf_tool__process_synth_event(struct perf_tool *tool,
> > static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
> > pid_t *tgid, pid_t *ppid)
> > {
> > - char filename[PATH_MAX];
> > char bf[4096];
> > int fd;
> > size_t size = 0;
> > @@ -80,12 +79,16 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
> > *tgid = -1;
> > *ppid = -1;
> >
> > - snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
> > + {
> > + char filename[PATH_MAX];
> >
> > - fd = open(filename, O_RDONLY);
> > - if (fd < 0) {
> > - pr_debug("couldn't open %s\n", filename);
> > - return -1;
> > + snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
> > +
> > + fd = open(filename, O_RDONLY);
> > + if (fd < 0) {
> > + pr_debug("couldn't open %s\n", filename);
> > + return -1;
> > + }
> > }
> >
> > n = read(fd, bf, sizeof(bf) - 1);
> > @@ -280,7 +283,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> > struct machine *machine,
> > bool mmap_data)
> > {
> > - char filename[PATH_MAX];
> > FILE *fp;
> > unsigned long long t;
> > bool truncation = false;
> > @@ -292,18 +294,22 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> > if (machine__is_default_guest(machine))
> > return 0;
> >
> > - snprintf(filename, sizeof(filename), "%s/proc/%d/task/%d/maps",
> > - machine->root_dir, pid, pid);
> > +#define FILENAME_FMT_STRING "%s/proc/%d/task/%d/maps"
> > + {
> > + char filename[PATH_MAX];
> >
> > - fp = fopen(filename, "r");
> > - if (fp == NULL) {
> > - /*
> > - * We raced with a task exiting - just return:
> > - */
> > - pr_debug("couldn't open %s\n", filename);
> > - return -1;
> > - }
> > + snprintf(filename, sizeof(filename), FILENAME_FMT_STRING,
> > + machine->root_dir, pid, pid);
> >
> > + fp = fopen(filename, "r");
> > + if (fp == NULL) {
> > + /*
> > + * We raced with a task exiting - just return:
> > + */
> > + pr_debug("couldn't open %s\n", filename);
> > + return -1;
> > + }
> > + }
> > event->header.type = PERF_RECORD_MMAP2;
> > t = rdclock();
> >
> > @@ -320,10 +326,10 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> > break;
> >
> > if ((rdclock() - t) > timeout) {
> > - pr_warning("Reading %s time out. "
> > + pr_warning("Reading " FILENAME_FMT_STRING " time out. "
> > "You may want to increase "
> > "the time limit by --proc-map-timeout\n",
> > - filename);
> > + machine->root_dir, pid, pid);
> > truncation = true;
> > goto out;
> > }
> > @@ -412,6 +418,8 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> >
> > fclose(fp);
> > return rc;
> > +
> > +#undef FILENAME_FMT_STRING
> > }
> >
> > int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t process,
> > --
> > 2.25.1.696.g5e7596f4ac-goog
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-27 17:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-27 6:36 [PATCH] perf synthetic-events: save 4kb from 2 stack frames Ian Rogers
2020-03-27 13:23 ` Jiri Olsa
2020-03-27 17:33 ` Ian Rogers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox