| 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: <Z60JmMKq-Do-VBUn@x1>
Date: Wed, 12 Feb 2025 21:50:32 +0100
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org,
Howard Chu <howardchu95@...il.com>,
Wander Costa <wcosta@...hat.com>
Subject: Re: [PATCH v3 1/4] perf trace: Allocate syscall stats only if
summary is on
On Wed, Feb 05, 2025 at 12:54:40PM -0800, Namhyung Kim wrote:
> The syscall stats are used only when summary is requested. Let's avoid
> unnecessary operations. While at it, let's pass 'trace' pointer
> directly instead of passing 'output' file pointer and 'summary' option
> in the 'trace' separately.
root@...ber:~# perf probe -x ~/bin/perf intlist__new
Added new event:
probe_perf:intlist_new (on intlist__new in /home/acme/bin/perf)
You can now use it in all perf tools, such as:
perf record -e probe_perf:intlist_new -aR sleep 1
root@...ber:~# perf trace -e probe_perf:intlist_new perf trace -e *nanosleep sleep 1
0.000 (1000.184 ms): sleep/574971 clock_nanosleep(rqtp: { .tv_sec: 1, .tv_nsec: 0 }, rmtp: 0x7ffda05c1be0) = 0
root@...ber:~# perf trace -e probe_perf:intlist_new perf trace -e *nanosleep --summary sleep 1
0.000 :574984/574984 probe_perf:intlist_new(__probe_ip: 6861748)
Summary of events:
sleep (574985), 2 events, 25.0%
syscall calls errors total min avg max stddev
(msec) (msec) (msec) (msec) (%)
--------------- -------- ------ -------- --------- --------- --------- ------
clock_nanosleep 1 0 1000.205 1000.205 1000.205 1000.205 0.00%
root@...ber:~#
I'm trying to convince a colleague to have this short streamlined to
something like:
# perf trace -e ~/bin/perf/intlist_new()/ perf trace -e *nanosleep sleep 1
Or some other event syntax that allows us to specify compactly and
unambiguously that we want to put a probe if one isn't there yet and
inside the () to specify which of the arguments we want collected, etc,
to save the 'perf probe' step, adding the probe and removing it if it
isn't there or reusing a pre-existing, compatible one.
Anyway, for the patch tested:
Tested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
- Arnaldo
> Acked-by: Howard Chu <howardchu95@...il.com>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/builtin-trace.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index ac97632f13dc8f7c..7e0324a2e9182088 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1522,13 +1522,14 @@ struct thread_trace {
> struct intlist *syscall_stats;
> };
>
> -static struct thread_trace *thread_trace__new(void)
> +static struct thread_trace *thread_trace__new(struct trace *trace)
> {
> struct thread_trace *ttrace = zalloc(sizeof(struct thread_trace));
>
> if (ttrace) {
> ttrace->files.max = -1;
> - ttrace->syscall_stats = intlist__new(NULL);
> + if (trace->summary)
> + ttrace->syscall_stats = intlist__new(NULL);
> }
>
> return ttrace;
> @@ -1550,7 +1551,7 @@ static void thread_trace__delete(void *pttrace)
> free(ttrace);
> }
>
> -static struct thread_trace *thread__trace(struct thread *thread, FILE *fp)
> +static struct thread_trace *thread__trace(struct thread *thread, struct trace *trace)
> {
> struct thread_trace *ttrace;
>
> @@ -1558,7 +1559,7 @@ static struct thread_trace *thread__trace(struct thread *thread, FILE *fp)
> goto fail;
>
> if (thread__priv(thread) == NULL)
> - thread__set_priv(thread, thread_trace__new());
> + thread__set_priv(thread, thread_trace__new(trace));
>
> if (thread__priv(thread) == NULL)
> goto fail;
> @@ -1568,7 +1569,7 @@ static struct thread_trace *thread__trace(struct thread *thread, FILE *fp)
>
> return ttrace;
> fail:
> - color_fprintf(fp, PERF_COLOR_RED,
> + color_fprintf(trace->output, PERF_COLOR_RED,
> "WARNING: not enough memory, dropping samples!\n");
> return NULL;
> }
> @@ -2622,7 +2623,7 @@ static int trace__sys_enter(struct trace *trace, struct evsel *evsel,
> return -1;
>
> thread = machine__findnew_thread(trace->host, sample->pid, sample->tid);
> - ttrace = thread__trace(thread, trace->output);
> + ttrace = thread__trace(thread, trace);
> if (ttrace == NULL)
> goto out_put;
>
> @@ -2699,7 +2700,7 @@ static int trace__fprintf_sys_enter(struct trace *trace, struct evsel *evsel,
> return -1;
>
> thread = machine__findnew_thread(trace->host, sample->pid, sample->tid);
> - ttrace = thread__trace(thread, trace->output);
> + ttrace = thread__trace(thread, trace);
> /*
> * We need to get ttrace just to make sure it is there when syscall__scnprintf_args()
> * and the rest of the beautifiers accessing it via struct syscall_arg touches it.
> @@ -2771,7 +2772,7 @@ static int trace__sys_exit(struct trace *trace, struct evsel *evsel,
> return -1;
>
> thread = machine__findnew_thread(trace->host, sample->pid, sample->tid);
> - ttrace = thread__trace(thread, trace->output);
> + ttrace = thread__trace(thread, trace);
> if (ttrace == NULL)
> goto out_put;
>
> @@ -2960,7 +2961,7 @@ static int trace__sched_stat_runtime(struct trace *trace, struct evsel *evsel,
> struct thread *thread = machine__findnew_thread(trace->host,
> sample->pid,
> sample->tid);
> - struct thread_trace *ttrace = thread__trace(thread, trace->output);
> + struct thread_trace *ttrace = thread__trace(thread, trace);
>
> if (ttrace == NULL)
> goto out_dump;
> @@ -3214,7 +3215,7 @@ static int trace__pgfault(struct trace *trace,
> }
> }
>
> - ttrace = thread__trace(thread, trace->output);
> + ttrace = thread__trace(thread, trace);
> if (ttrace == NULL)
> goto out_put;
>
> --
> 2.48.1.502.g6dc24dfdaf-goog
Powered by blists - more mailing lists