Re: [PATCH v2] drm/amd/display: enable more strict compile checks

From: Nathan Chancellor <nathan@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Hamza Mahfooz <hamza.mahfooz@amd.com>,
	Alex Deucher <alexdeucher@gmail.com>,
	Felix Kuehling <felix.kuehling@amd.com>,
	"Russell, Kent" <Kent.Russell@amd.com>,
	"Ho, Kenny" <Kenny.Ho@amd.com>,
	"Li, Sun peng (Leo)" <Sunpeng.Li@amd.com>,
	"Wentland, Harry" <Harry.Wentland@amd.com>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	"Siqueira, Rodrigo" <Rodrigo.Siqueira@amd.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
	Daniel Vetter <daniel@ffwll.ch>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>,
	David Airlie <airlied@gmail.com>,
	"Koenig, Christian" <Christian.Koenig@amd.com>
Subject: Re: [PATCH v2] drm/amd/display: enable more strict compile checks
Date: Thu, 25 May 2023 09:26:07 -0700	[thread overview]
Message-ID: <20230525162607.GA550162@dev-arch.thelio-3990X> (raw)
In-Reply-To: <202305250832.0127ABAC@keescook>

On Thu, May 25, 2023 at 08:37:07AM -0700, Kees Cook wrote:
> Hi!
> 
> On Wed, May 24, 2023 at 04:27:31PM -0400, Hamza Mahfooz wrote:
> > + Kees
> > 
> > On 5/24/23 15:50, Alex Deucher wrote:
> > > On Wed, May 24, 2023 at 3:46 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
> > > > 
> > > > Sure, I think we tried enabling warnings as errors before and had to
> > > > revert it because of weird compiler quirks or the variety of compiler
> > > > versions that need to be supported.
> > > > 
> > > > Alex, are you planning to upstream this, or is this only to enforce more
> > > > internal discipline about not ignoring warnings?
> > > 
> > > I'd like to upstream it.  Upstream already has CONFIG_WERROR as a
> > > config option, but it's been problematic to enable in CI because of
> > > various breakages outside of the driver and in different compilers.
> > > That said, I don't know how much trouble enabling it will cause with
> > > various compilers in the wild.
> 
> -Wmisleading-indentation is already part of -Wall, so this is globally
> enabled already.
> 
> -Wunused is enabled under W=1, and it's pretty noisy still. If you can
> get builds clean in drm, that'll be a good step towards getting it
> enabled globally. (A middle ground with less to clean up might be
> -Wunused-but-set-variable)
> 
> I agree about -Werror: just stick with CONFIG_WERROR instead.

There is also W=e, added by commit c77d06e70d59 ("kbuild: support W=e
to make build abort in case of warning") in 5.19, which works well for
building with configurations that do not have CONFIG_WERROR enabled and
avoiding dipping into menuconfig.

Unconditionally enabling -Werror with no way to turn it off is just
asking for problems over time with new compiler versions, either due to
new warnings in -Wall or warnings that have been improved or changed.
Should that still be desired, consider doing what i915 and PowerPC have
done and add a Kconfig option that can be disabled.

Cheers,
Nathan