linux-kernel - Re: [PATCH v2 13/16] perf bench: Remove reference to cmd

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fU7n6iumZuNaUcS7VjsG9uF3ib-mP_6cvUGExKaY-CNQw@mail.gmail.com>
Date: Tue, 22 Oct 2024 10:15:37 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
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>, 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>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v2 13/16] perf bench: Remove reference to cmd_inject

On Mon, Oct 21, 2024 at 11:44 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Tue, Oct 15, 2024 at 09:24:12PM -0700, Ian Rogers wrote:
> > Avoid `perf bench internals inject-build-id` referencing the
> > cmd_inject sub-command that requires perf-bench to backward reference
> > internals of builtins. Replace the reference to cmd_inject with a call
> > to main. To avoid python.c needing to link with something providing
> > main, drop the libperf-bench library from the python shared object.
>
> Looks like a clever trick!  But I guess by removing libperf-bench from
> the python object, you can remove the reference to cmd_inject, right?

So this change is looking to clean up cmd_inject yes, an alternative
thing to do is to remove libperf-bench.a as a python dependency but
that would use an unused stub. I'm looking to remove all the stubs.
Once you start cleaning up the cmd_inject stub then you get where this
patch is.

Wrt cmd_inject have 4 choices:
1) status quo - have libperf-bench.a be depended on by builtin-bench
but have a cycle where libperf-bench.a depends on builtin-inject. This
can yield build issues I've wrestled with in the past and in general
this shouldn't be done.
2) this patch - have a dependency from libperf-bench.a to main in
perf.c, which is less of an internal dependency on the perf.c/builtin
code but is still an annoying cycle. It's not much better than (1) but
at least it shouldn't require a #include "../builtin.h" which is
obviously bad. That #include isn't in the bench code as it re-declares
the function, again bad. The patch here declares main which isn't
great, but at least the declaration of main should not be subject to
change whereas the builtins could be.
3) move perf inject into util or some other 3rd library. Out of scope
for this change, but also not something I think we want.
4) have `perf bench internals inject-build-id` fork/exec the perf
command. My preference but out of scope here.

So I went with 2 to improve things but without doing some larger
rewrite. The cleanups all relate to removing cyclic dependencies to
cmd_inject hence 1 patch.

Thanks,
Ian