perf test pmu: Remove unused test_pmus

[PATCH] perf test pmu: Remove unused test_pmus

Posted by linux@treblig.org 1 year, 6 months ago

From: "Dr. David Alan Gilbert" <linux@treblig.org>

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@treblig.org>
---
 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

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

Posted by Ian Rogers 1 year, 6 months ago

On Sat, Jul 27, 2024 at 10:59 AM <linux@treblig.org> wrote:
>
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
>
> 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@treblig.org>

Strange that the compiler doesn't warn about unused stuff like this,
we get unused variables within a function and unused static
functions...

Reviewed-by: Ian Rogers <irogers@google.com>

Possibly:
Fixes: aa1551f299ba ("perf test pmu: Refactor format test and exposed
test APIs")

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
>

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

Posted by Arnaldo Carvalho de Melo 1 year, 6 months ago

On Mon, Jul 29, 2024 at 07:52:53AM -0700, Ian Rogers wrote:
> On Sat, Jul 27, 2024 at 10:59 AM <linux@treblig.org> wrote:
> >
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> >
> > 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@treblig.org>
> 
> Strange that the compiler doesn't warn about unused stuff like this,
> we get unused variables within a function and unused static
> functions...
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks, applied to tmp.perf-tools-next,

- Arnaldo
 
> Possibly:
> Fixes: aa1551f299ba ("perf test pmu: Refactor format test and exposed
> test APIs")
> 
> 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
> >

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

Posted by Dr. David Alan Gilbert 1 year, 6 months ago

* Ian Rogers (irogers@google.com) wrote:
> On Sat, Jul 27, 2024 at 10:59 AM <linux@treblig.org> wrote:
> >
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> >
> > 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@treblig.org>
> 
> 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

> Reviewed-by: Ian Rogers <irogers@google.com>

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.

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   |_______/

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

Posted by Ian Rogers 1 year, 6 months ago

On Mon, Jul 29, 2024 at 8:07 AM Dr. David Alan Gilbert
<linux@treblig.org> wrote:
>
> * Ian Rogers (irogers@google.com) wrote:
> > On Sat, Jul 27, 2024 at 10:59 AM <linux@treblig.org> wrote:
> > >
> > > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > >
> > > 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@treblig.org>
> >
> > 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@google.com>
>
> 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   |_______/