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]

Message-ID: <bd98c9f3-de67-7ca5-534c-f7fd6cc69915@huawei.com>
Date:   Tue, 28 Sep 2021 21:30:16 +0100
From:   John Garry <john.garry@...wei.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
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

On 28/09/2021 18:56, Arnaldo Carvalho de Melo wrote:
>>>   ‘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.
>> Hi Arnaldo,
>>
>> OK, would you also consider reusing CFLAGS:
>>
>> --- a/tools/perf/pmu-events/Build
>> +++ b/tools/perf/pmu-events/Build
>> @@ -9,10 +9,12 @@ JSON          =  $(shell [ -d $(JDIR) ] &&
>> \
>> JDIR_TEST      =  pmu-events/arch/test
>> JSON_TEST      =  $(shell [ -d $(JDIR_TEST) ] &&                       \
>>                         find $(JDIR_TEST) -name '*.json')
>> -
>> +HOSTCFLAGS_jevents += $(CFLAGS)
> Humm, we have to check if CFLAGS doesn't come with cross-build options,
> i.e. IIRC we have to use HOSTCFLAGS instead. Unsure if there is some
> *CFLAGS variable that gets the common part, where these -Wall and
> -Wextra, -Werror could go.

not sure. As I see, the bulk of flags we have in CFLAGS comes from 
EXTRA_WARNINGS in scripts/Makefile.include; but CFLAGS seems to also 
include EXTRA_CLAGS, which are for cross-builds (see perf/Makefile.config)

So maybe we just need to use EXTRA_WARNINGS for HOST_CFLAGS

>   
>> I tried it, and there are more things to fix for jevents.o. Let me know your
>> preference and if any help required to fix any errors up.
> I fixed the one I found, see below, I'll test build what I have in
> perf/core and push it, then you can continue from there, after checking
> this HOSTCFLAGS/CFLAGS issue.
> 
> - Arnaldo
> 
>>>From 0e46c8307574a8e2dac8d7ba97e0f6f4bbee67a5 Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo<acme@...hat.com>
> Date: Tue, 28 Sep 2021 14:15:01 -0300
> Subject: [PATCH 1/1] perf jevents: Add __maybe_unused attribute to unused
>   function arg
> 
> The tools/perf/pmu-events/jevents.c file isn't being compiled with
> -Werror and -Wextra, which will be the case soon, so before we turn
> those compiler flags on, fix what it would flag.
> 
> Cc: Alexander Shishkin<alexander.shishkin@...ux.intel.com>
> Cc: Ingo Molnar<mingo@...hat.com>
> Cc: Jiri Olsa<jolsa@...hat.com>
> Cc: Like Xu<like.xu.linux@...il.com>
> Cc: Mark Rutland<mark.rutland@....com>
> Cc: Namhyung Kim<namhyung@...nel.org>
> Cc: Peter Zijlstra<peterz@...radead.org>
> Signed-off-by: Arnaldo Carvalho de Melo<acme@...hat.com>
> To: John Garry<john.garry@...wei.com>

thanks.

Reviewed-by: John Garry <john.garry@...wei.com>

> ---
>   tools/perf/pmu-events/jevents.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 6731b3cf0c2fc9b7..323e1dfe2436c049 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -45,6 +45,7 @@
>   #include <sys/resource.h>		/* getrlimit */
>   #include <ftw.h>
>   #include <sys/stat.h>
> +#include <linux/compiler.h>
>   #include <linux/list.h>
>   #include "jsmn.h"
>   #include "json.h"
> @@ -470,7 +471,7 @@ static void free_arch_std_events(void)
>   	}
>   }
>   
> -static int save_arch_std_events(void *data, struct json_event *je)
> +static int save_arch_std_events(void *data __maybe_unused, struct json_event *je)
>   {
>   	struct event_struct *es;