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@redhat.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@google.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
>