Messages in this thread Patch in this message | | | Date | Tue, 28 Sep 2021 10:22:36 -0300 | From | Arnaldo Carvalho de Melo <> | 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@tencent.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@huawei.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@toolbox 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@toolbox 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
| |