linux-kernel - Re: [PATCH v6 15/22] perf lock: Move common lock contention code to new file

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: <ZzzEE_kAjmXnvwtj@google.com>
Date: Tue, 19 Nov 2024 09:00:03 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.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>,
	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,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH v6 15/22] perf lock: Move common lock contention code to
 new file

On Mon, Nov 18, 2024 at 05:03:41PM -0800, Ian Rogers wrote:
> On Mon, Nov 18, 2024 at 4:23 PM Namhyung Kim <namhyung@...nel.org> wrote:
[SNIP]
> > On Fri, Nov 08, 2024 at 10:18:02PM -0800, Ian Rogers wrote:
> > > +#ifndef HAVE_BPF_SKEL
> > > +int lock_contention_prepare(struct lock_contention *con __maybe_unused)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +int lock_contention_start(void)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +int lock_contention_stop(void)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +int lock_contention_finish(struct lock_contention *con __maybe_unused)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +int lock_contention_read(struct lock_contention *con __maybe_unused)
> > > +{
> > > +     return 0;
> > > +}
> > > +#endif  /* !HAVE_BPF_SKEL */
> >
> > I still think it's the convention to have them in a header file as
> > static inline functions and reduce the #ifdef in the .c file.
> 
> Shouldn't minimizing ifdefs, and associated cognitive load, in header
> files be the priority given they are #included many times while the .c
> file is only compiled once?
> Shouldn't a goal of the header file be to abstract away things like
> HAVE_BPF_SKEL?
> I'm not clear what the goal of having the functions in the header
> files is, performance? The code isn't going to run anyway. I feel
> lock_contention.h is smaller and easier to read like this but I also
> don't care enough to fight. I did this change here as
> lock_contention.h was being brought into python.c for the sake of
> stubbing out functions that the header file was also subbing out for
> !BPF_HAVE_SKEL. A single stub felt like progress.

I think it may have the empty functions in the binary if we keep the
functions in the .c file whereas compilers would optimize away them if
they are static inline functions.

Thanks,
Namhyung