linux-kernel - Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std

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]

Date:   Tue, 28 Sep 2021 10:22:36 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     John Garry <john.garry@...wei.com>
Cc:     Like Xu <like.xu.linux@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like
 arch_std_events

Em Tue, Sep 28, 2021 at 10:16:48AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Sep 28, 2021 at 01:49:20PM +0100, John Garry escreveu:
> > On 28/09/2021 12:52, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu:
> > > > From: Like Xu <likexu@...cent.com>
> > > > 
> > > > The compiler reports that free_sys_event_tables() is dead code. But
> > > > according to the semantics, the "LIST_HEAD(arch_std_events)" should
> > > > also be released, just like we do with 'arch_std_events' in the main().
> > > 
> > > Thanks, applied.
> > > 
> > > - Arnaldo
> > > 
> > 
> > If not too late:
> > Reviewed-by: John Garry <john.garry@...wei.com>
> 
> Not too late, collected.
>  
> > I think that it could be a good idea to raise gcc warning level to detect
> > unused static functions, like this was
> 
> Agreed, but we already have:
> 
> CORE_CFLAGS += -Wall
> CORE_CFLAGS += -Wextra
> 
> We can se it for this specific case with:
> 
> $ make V=1 -k BUILD_BPF_SKEL=1 CORESIGHT=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin | grep jevents
> make -f /var/home/acme/git/perf/tools/build/Makefile.build dir=pmu-events obj=jevents
>   gcc -Wp,-MD,/tmp/build/perf/pmu-events/.jevents.o.d -Wp,-MT,/tmp/build/perf/pmu-events/jevents.o  -D"BUILD_STR(s)=#s" -I/var/home/acme/git/perf/tools/include  -c -o /tmp/build/perf/pmu-events/jevents.o pmu-events/jevents.c
>    ld -r -o /tmp/build/perf/pmu-events/jevents-in.o  /tmp/build/perf/pmu-events/json.o /tmp/build/perf/pmu-events/jsmn.o /tmp/build/perf/pmu-events/jevents.o
> gcc /tmp/build/perf/pmu-events/jevents-in.o -o /tmp/build/perf/pmu-events/jevents
> /tmp/build/perf/pmu-events/jevents x86 pmu-events/arch /tmp/build/perf/pmu-events/pmu-events.c 1
> jevents: Processing mapfile pmu-events/arch/x86/mapfile.csv
> 
> Humm... no "-Wall -Wextra" there... lemme try to fix it

With this:

⬢[acme@...lbox perf]$ git diff
diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
index a055dee6a46af77e..ea7107630bf4327f 100644
--- a/tools/perf/pmu-events/Build
+++ b/tools/perf/pmu-events/Build
@@ -1,7 +1,7 @@
 hostprogs := jevents

 jevents-y      += json.o jsmn.o jevents.o
-HOSTCFLAGS_jevents.o   = -I$(srctree)/tools/include
+HOSTCFLAGS_jevents.o   = -I$(srctree)/tools/include -Wall -Wextra
 pmu-events-y   += pmu-events.o
 JDIR           =  pmu-events/arch/$(SRCARCH)
 JSON           =  $(shell [ -d $(JDIR) ] &&                            \
⬢[acme@...lbox perf]$

I get this before applying Xu's patch:

  LINK    /tmp/build/perf/libbpf.a
pmu-events/jevents.c: In function ‘save_arch_std_events’:
pmu-events/jevents.c:473:39: warning: unused parameter ‘data’ [-Wunused-parameter]
  473 | static int save_arch_std_events(void *data, struct json_event *je)
      |                                 ~~~~~~^~~~
At top level:
pmu-events/jevents.c:93:13: warning: ‘free_sys_event_tables’ defined but not used [-Wunused-function]
   93 | static void free_sys_event_tables(void)
      |             ^~~~~~~~~~~~~~~~~~~~~


-------------------------------------

I'll add this to perf/core, as this isn't a strict fix, so can wait for
v5.16.

- Arnaldo