Re: [PATCH] perf test pmu: Remove unused test_pmus

[Ian Rogers]

On Mon, Jul 29, 2024 at 8:07 AM Dr. David Alan Gilbert
<linux@xxxxxxxxxxx> wrote:
>
> * Ian Rogers (irogers@xxxxxxxxxx) wrote:
> > On Sat, Jul 27, 2024 at 10:59 AM <linux@xxxxxxxxxxx> wrote:
> > >
> > > From: "Dr. David Alan Gilbert" <linux@xxxxxxxxxxx>
> > >
> > > Commit aa1551f299ba ("perf test pmu: Refactor format test and exposed
> > > test APIs") added the 'test_pmus' list, but didn't use it.
> > > (It seems to put them on the other_pmus list?)
> > >
> > > Remove it.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert <linux@xxxxxxxxxxx>
> >
> > Strange that the compiler doesn't warn about unused stuff like this,
> > we get unused variables within a function and unused static
> > functions...
>
> The problem is that LIST_HEAD initialises the list to point to itself;
> so it *is* used - but only in it's own initialiser.
> I did file:
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115027

Nice, the bug already has a fix - good work! Hopefully clang can follow suit.

> > Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> Thanks,
>
> > Possibly:
> > Fixes: aa1551f299ba ("perf test pmu: Refactor format test and exposed
> > test APIs")
>
> Given it's got no actual effect other than a few bytes
> saved, I'm not sure it's worth the Fixes.

Ack. I mention it so the maintainers can make a decision about fixing
in 6.11 (still in rc stage) or whether to hold for 6.12. It seems
pretty safe either way, but for 6.11 probably better to have a fixes
tag. If the GCC warning becomes a thing there's a chance the code
won't build without the fix :-)

Thanks,
Ian

> Dave
>
> > Thanks,
> > Ian
> >
> > > ---
> > >  tools/perf/tests/pmu.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
> > > index 40132655ccd1..0b2f04a55d7b 100644
> > > --- a/tools/perf/tests/pmu.c
> > > +++ b/tools/perf/tests/pmu.c
> > > @@ -18,9 +18,6 @@
> > >  #include <sys/stat.h>
> > >  #include <sys/types.h>
> > >
> > > -/* Fake PMUs created in temp directory. */
> > > -static LIST_HEAD(test_pmus);
> > > -
> > >  /* Cleanup test PMU directory. */
> > >  static int test_pmu_put(const char *dir, struct perf_pmu *pmu)
> > >  {
> > > --
> > > 2.45.2
> > >
> --
>  -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> \        dave @ treblig.org |                               | In Hex /
>  \ _________________________|_____ http://www.treblig.org   |_______/