On Sun, Nov 5, 2023 at 9:01 PM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Sun, Nov 5, 2023, at 12:54, Yafang Shao wrote: > > On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@xxxxxxxx> wrote: > >> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote: > >> > To address this, we should also suppress the "-Wmissing-prototypes" warning > >> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as > >> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0. > >> > >> Not sure why these need to be suppressed like this at all, > >> can't you just add the prototype somewhere? > > > > BPF kfuncs are intended for use within BPF programs, and they should > > not be called from other parts of the kernel. Consequently, it is not > > appropriate to include their prototypes in a kernel header file. > > How does the caller in the BPF program get the prototype? BPF programs will get the prototype directly from BTF, for example, see also the prototypes declared using "__ksym" in tools/testing/selftests/bpf/progs/ for examples. > > >> > @@ -131,14 +131,14 @@ > >> > #define __diag_str(s) __diag_str1(s) > >> > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) > >> > > >> > -#if GCC_VERSION >= 80000 > >> > -#define __diag_GCC_8(s) __diag(s) > >> > +#if GCC_VERSION >= 50100 > >> > +#define __diag_GCC_5(s) __diag(s) > >> > #else > >> > -#define __diag_GCC_8(s) > >> > +#define __diag_GCC_5(s) > >> > #endif > >> > > >> > >> This breaks all uses of __diag_ignore that specify > >> version 8 directly. Just add the macros for each version > >> from 5 to 14 here. > > > > It seems that __diag_GCC_8() or __diag_GCC() are not directly used > > anywhere in the kernel, right? > > I see three instances: > > drivers/net/ethernet/renesas/sh_eth.c:__diag_ignore(GCC, 8, "-Woverride-init", > include/linux/compat.h: __diag_ignore(GCC, 8, "-Wattribute-alias", include/linux/syscalls.h: __diag_ignore(GCC, 8, "-Wattribute-alias", Thanks for pointing them out. > > The override-init one should probably use version 5 as well, > but I think the -Wattribute-alias ones require GCC 8 and otherwise > cause a warning about an unknown warning option. Right. -Wattribute-alias requires GCC 8. > > __diag_ignore_all() would also be wrong for the override-init > because the option has a different name in clang > (-Winitializer-overrides). > > > Therefore it won't break anything if we just replace __diag_GCC_8() > > with __diag_GCC_5(). > > It may be cumbersome to add the macrocs for every GCC version if they > > aren't actively used. > > For the _all variant, I would prefer to completely remove > the version logic and just use __diag() directly. I think the > entire point of this is that it is used on all supported > versions. Good suggestion. will do it. -- Regards Yafang
