On Mon, 09 Oct 2023, Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > On Sun, Oct 08, 2023 at 12:28:46AM +0900, Masahiro Yamada wrote: >> On Fri, Oct 6, 2023 at 9:35 PM Jani Nikula <jani.nikula@xxxxxxxxx> wrote: >> > >> > The kernel top level Makefile, and recently scripts/Makefile.extrawarn, >> > have included -Wall, and the disables -Wno-format-security and >> > $(call cc-disable-warning,frame-address,) for a very long time. They're >> > redundant in our local subdir-ccflags-y and can be dropped. >> > >> > Cc: Arnd Bergmann <arnd@xxxxxxxx> >> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> >> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> >> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> >> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> >> >> I made a similar suggestion in the past >> https://lore.kernel.org/dri-devel/20190515043753.9853-1-yamada.masahiro@xxxxxxxxxxxxx/ >> >> So, I am glad that Intel has decided to de-duplicate the flags. >> >> >> >> I think you can drop more flags. >> >> For example, >> >> subdir-ccflags-y += -Wno-sign-compare >> >> >> It is set by scripts/Makefile.extrawarn >> unless W=3 is passed. >> >> >> If W=3 is set by a user, -Wsign-compare should be warned >> as it is the user's request. >> >> >> drivers/gpu/drm/i915/Makefile negates W=3. >> There is no good reason to do so. >> >> >> Same applied to >> >> >> subdir-ccflags-y += -Wno-shift-negative-value > > As I point out in my review of the second patch [1], I am not sure these > should be dropped because -Wextra turns these warnings back on, at least > for clang according to this build report [2] and my own testing, so they > need to be disabled again. Yeah. The focus is on enabling W=1 warnings by default for i915. I get that the disables we have to add to achieve that also disable some W=2 and W=3 warnings. But taking all of that into account requires duplicating even more of Makefile.extrawarn (checking for warning levels, maintaining parity with the different levels, etc.). I guess we could check if KBUILD_EXTRA_WARN does not have any of 1, 2, or 3, but very few places outside of the build system look at KBUILD_EXTRA_WARN, so feels wrong. BR, Jani. > > [1]: https://lore.kernel.org/20231006174550.GC3359308@dev-arch.thelio-3990X/ > [2]: https://lore.kernel.org/202310070011.Fji48IBk-lkp@xxxxxxxxx/ > > Cheers, > Nathan. -- Jani Nikula, Intel