Re: [Intel-gfx] [RFC] drm: enable W=1 warnings by default across the subsystem

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 30 Nov 2023, Javier Martinez Canillas <javierm@redhat.com> wrote:
> Maxime Ripard <mripard@kernel.org> writes:
>
>> Hi,
>>
>> On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote:
>>> On Wed, 29 Nov 2023, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
>>> > Cc: Nathan Chancellor <nathan@kernel.org>
>>> >
>>> > On 11/29/23 13:12, Jani Nikula wrote:
>>> >> At least the i915 and amd drivers enable a bunch more compiler warnings
>>> >> than the kernel defaults.
>>> >> 
>>> >> Extend the W=1 warnings to the entire drm subsystem by default. Use the
>>> >> copy-pasted warnings from scripts/Makefile.extrawarn with
>>> >> s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
>>> >> up with them in the future.
>>> >> 
>>> >> This is similar to the approach currently used in i915.
>>> >> 
>>> >> Some of the -Wextra warnings do need to be disabled, just like in
>>> >> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
>>> >> builds, depending on the warning.
>>> >
>>> > I think this should go in after drm-misc-next has a clean build (for
>>> > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
>>> > lot of build configs.
>>> 
>>> Oh, I'm absolutely not suggesting this should be merged before known
>>> warnings have been addressed one way or another. Either by fixing them
>>> or by disabling said warning in driver local Makefiles, depending on the
>>> case.
>>
>> I'm all for it, but yeah, we need some easy way to opt-in/opt-out. Some
>> drivers are pretty much unmaintained now and are likely to never fix
>> those warnings.

Then I'd go for enabling in drm level and disabling individual warnings
in the driver Makefile level if they won't get fixed.

> Maybe add a Kconfig symbol for it instead of making unconditional?
>
> Something like:
>
> +# Unconditionally enable W=1 warnings locally
> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
> +subdir-ccflags-$(CONFIG_DRM_EXTRA_CHECKS)  += -Wextra -Wunused -Wno-unused-parameter
> ...

Then we'll have a ping pong of people not using W=1 or
CONFIG_DRM_EXTRA_CHECKS introducing warnings, and people using them
fixing the warnings...

I really do think making it unconditional is the only way.


BR,
Jani.


-- 
Jani Nikula, Intel