| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fVc6kRpaKEnA+OeKmK8Zr8KF+JpUw4_mZvesLtHyHA0Ew@mail.gmail.com>
Date: Fri, 27 Mar 2020 10:33:07 -0700
From: Ian Rogers <irogers@...gle.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Namhyung Kim <namhyung@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH] perf synthetic-events: save 4kb from 2 stack frames
On Fri, Mar 27, 2020 at 6:24 AM Jiri Olsa <jolsa@...hat.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@...gle.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
> >
>
Powered by blists - more mailing lists