| 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: <ZyPbggG8jC9MYQbo@x1>
Date: Thu, 31 Oct 2024 16:33:22 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
James Clark <james.clark@...aro.org>,
Howard Chu <howardchu95@...il.com>,
Athira Jajeev <atrajeev@...ux.vnet.ibm.com>,
Michael Petlan <mpetlan@...hat.com>,
Veronika Molnarova <vmolnaro@...hat.com>,
Dapeng Mi <dapeng1.mi@...ux.intel.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Ilya Leoshkevich <iii@...ux.ibm.com>,
Colin Ian King <colin.i.king@...il.com>,
Weilin Wang <weilin.wang@...el.com>,
Andi Kleen <ak@...ux.intel.com>,
Josh Poimboeuf <jpoimboe@...hat.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v5 09/21] perf script: Move script_fetch_insn to
trace-event-scripting.c
On Wed, Oct 30, 2024 at 06:42:40PM -0700, Ian Rogers wrote:
> Add native_arch as a parameter to script_fetch_insn rather than
> relying on the builtin-script value that won't be initialized for the
> dlfilter and python Context use cases. Assume both of those cases are
> running natively.
Adrian, can you take a look? Maybe have some machine__native_arch() that
would use the perf_env struct, uname, etc?
So, as you noticed builtin-script.c does it, sets a global variable
uname(&uts);
if (data.is_pipe) { /* Assume pipe_mode indicates native_arch */
native_arch = true;
} else if (session->header.env.arch) {
if (!strcmp(uts.machine, session->header.env.arch))
native_arch = true;
else if (!strcmp(uts.machine, "x86_64") &&
!strcmp(session->header.env.arch, "i386"))
native_arch = true;
}
So instead of doing it we should move that global variable to
env->native_arch and then use it?
Your patch doesn't introduce problems and shows this is a limitation, so
no problem with it being merged:
Acked-by: Arnaldo Carvalho de Melo <acme@...hat.com>
But maybe this would be the time to try to fix this? Adrian,
cross-platform use of dlfilter seems not to be something you would be
interested on, but using a python script to process something from
another arch is interesting, as we advertise cross-platorm perf.data
file analysis as a feature for perf.
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/builtin-script.c | 15 +--------------
> .../perf/scripts/python/Perf-Trace-Util/Context.c | 2 +-
> tools/perf/util/dlfilter.c | 3 ++-
> tools/perf/util/python.c | 6 ------
> tools/perf/util/trace-event-scripting.c | 14 ++++++++++++++
> tools/perf/util/trace-event.h | 2 +-
> 6 files changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 11c0ee8c1afc..22d78a9d8f27 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1586,19 +1586,6 @@ static int perf_sample__fprintf_callindent(struct perf_sample *sample,
> return len + dlen;
> }
>
> -__weak void arch_fetch_insn(struct perf_sample *sample __maybe_unused,
> - struct thread *thread __maybe_unused,
> - struct machine *machine __maybe_unused)
> -{
> -}
> -
> -void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
> - struct machine *machine)
> -{
> - if (sample->insn_len == 0 && native_arch)
> - arch_fetch_insn(sample, thread, machine);
> -}
> -
> static int perf_sample__fprintf_insn(struct perf_sample *sample,
> struct evsel *evsel,
> struct perf_event_attr *attr,
> @@ -1608,7 +1595,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> {
> int printed = 0;
>
> - script_fetch_insn(sample, thread, machine);
> + script_fetch_insn(sample, thread, machine, native_arch);
>
> if (PRINT_FIELD(INSNLEN))
> printed += fprintf(fp, " ilen: %d", sample->insn_len);
> diff --git a/tools/perf/scripts/python/Perf-Trace-Util/Context.c b/tools/perf/scripts/python/Perf-Trace-Util/Context.c
> index d742daaa5d5a..60dcfe56d4d9 100644
> --- a/tools/perf/scripts/python/Perf-Trace-Util/Context.c
> +++ b/tools/perf/scripts/python/Perf-Trace-Util/Context.c
> @@ -93,7 +93,7 @@ static PyObject *perf_sample_insn(PyObject *obj, PyObject *args)
> if (c->sample->ip && !c->sample->insn_len && thread__maps(c->al->thread)) {
> struct machine *machine = maps__machine(thread__maps(c->al->thread));
>
> - script_fetch_insn(c->sample, c->al->thread, machine);
> + script_fetch_insn(c->sample, c->al->thread, machine, /*native_arch=*/true);
> }
> if (!c->sample->insn_len)
> Py_RETURN_NONE; /* N.B. This is a return statement */
> diff --git a/tools/perf/util/dlfilter.c b/tools/perf/util/dlfilter.c
> index 7d180bdaedbc..ddacef881af2 100644
> --- a/tools/perf/util/dlfilter.c
> +++ b/tools/perf/util/dlfilter.c
> @@ -234,7 +234,8 @@ static const __u8 *dlfilter__insn(void *ctx, __u32 *len)
> struct machine *machine = maps__machine(thread__maps(al->thread));
>
> if (machine)
> - script_fetch_insn(d->sample, al->thread, machine);
> + script_fetch_insn(d->sample, al->thread, machine,
> + /*native_arch=*/true);
> }
> }
>
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 3d938fe2de6a..22edadd64e5f 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1317,12 +1317,6 @@ struct kwork_work *perf_kwork_add_work(struct perf_kwork *kwork __maybe_unused,
> return NULL;
> }
>
> -void script_fetch_insn(struct perf_sample *sample __maybe_unused,
> - struct thread *thread __maybe_unused,
> - struct machine *machine __maybe_unused)
> -{
> -}
> -
> int perf_sample__sprintf_flags(u32 flags __maybe_unused, char *str __maybe_unused,
> size_t sz __maybe_unused)
> {
> diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
> index ad62d8e5a368..beac456260ae 100644
> --- a/tools/perf/util/trace-event-scripting.c
> +++ b/tools/perf/util/trace-event-scripting.c
> @@ -13,6 +13,7 @@
> #include <traceevent/event-parse.h>
> #endif
>
> +#include "archinsn.h"
> #include "debug.h"
> #include "trace-event.h"
> #include "evsel.h"
> @@ -269,3 +270,16 @@ void setup_perl_scripting(void)
> }
> #endif
> #endif
> +
> +__weak void arch_fetch_insn(struct perf_sample *sample __maybe_unused,
> + struct thread *thread __maybe_unused,
> + struct machine *machine __maybe_unused)
> +{
> +}
> +
> +void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
> + struct machine *machine, bool native_arch)
> +{
> + if (sample->insn_len == 0 && native_arch)
> + arch_fetch_insn(sample, thread, machine);
> +}
> diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
> index 81fceaf297ba..e0bb42e6509e 100644
> --- a/tools/perf/util/trace-event.h
> +++ b/tools/perf/util/trace-event.h
> @@ -120,7 +120,7 @@ struct scripting_ops *script_spec__lookup(const char *spec);
> int script_spec__for_each(int (*cb)(struct scripting_ops *ops, const char *spec));
>
> void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
> - struct machine *machine);
> + struct machine *machine, bool native_arch);
>
> void setup_perl_scripting(void);
> void setup_python_scripting(void);
> --
> 2.47.0.163.g1226f6d8fa-goog
>
Powered by blists - more mailing lists