Linux-KBuild Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: netdev <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Rohit Maheshwari <rohitm@chelsio.com>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today
Date: Fri, 2 Oct 2020 03:44:11 +0200
Message-ID: <20201002014411.GG4067422@lunn.ch> (raw)
In-Reply-To: <CAKwvOdnVC8F1=QT03W5Zh9pJdTxxNfRcqXeob5_b4CXycvG1+g@mail.gmail.com>

On Thu, Oct 01, 2020 at 04:09:43PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 30, 2020 at 6:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > There is a movement to try to make more and more of /drivers W=1
> > clean. But it will only stay clean if new warnings are quickly
> > detected and fixed, ideally by the developer adding the new code.
> >
> > To allow subdirectories to sign up to being W=1 clean for a given
> > definition of W=1, export the current set of additional compile flags
> > using the symbol KBUILD_CFLAGS_W1_20200930. Subdirectory Makefiles can
> > then use:
> >
> > subdir-ccflags-y := $(KBUILD_CFLAGS_W1_20200930)
> >
> > To indicate they want to W=1 warnings as defined on 20200930.
> >
> > Additional warnings can be added to the W=1 definition. This will not
> > affect KBUILD_CFLAGS_W1_20200930 and hence no additional warnings will
> > start appearing unless W=1 is actually added to the command
> > line. Developers can then take their time to fix any new W=1 warnings,
> > and then update to the latest KBUILD_CFLAGS_W1_<DATESTAMP> symbol.
> 
> I'm not a fan of this approach.  Are DATESTAMP configs just going to
> keep being added? Is it going to complicate isolating the issue from a
> randconfig build?  If we want more things to build warning-free at
> W=1, then why don't we start moving warnings from W=1 into the
> default, until this is no W=1 left?  That way we're cutting down on
> the kernel's configuration combinatorial explosion, rather than adding
> to it?

Hi Nick

I don't see randconfig being an issue. driver/net/ethernet would
always be build W=1, by some stable definition of W=1. randconfig
would not enable or disable additional warnings. It to make it clear,
KBUILD_CFLAGS_W1_20200930 is not a Kconfig option you can select. It
is a Makefile constant, a list of warnings which define what W=1 means
on that specific day. See patch 1/2.

I see a few issues with moving individual warnings from W=1 to the
default:

One of the comments for v1 of this patchset is that we cannot
introduce new warnings in the build. The complete tree needs to clean
of a particularly warning, before it can be added to the default list.
But that is not how people are cleaning up code, nor how the
infrastructure is designed. Those doing the cleanup are not take the
first from the list, -Wextra and cleanup up the whole tree for that
one warnings. They are rather enabling W=1 on a subdirectory, and
cleanup up all warnings on that subdirectory. So using this approach,
in order to move a warning from W=1 to the default, we are going to
have to get the entire tree W=1 clean, and move them all the warnings
are once.

People generally don't care about the tree as a whole. They care about
their own corner. The idea of fixing one warning thought the whole
tree is 'slicing and dicing' the kernel the wrong way. As we have seen
with the recent work with W=1, the more natural way to slice/dice the
kernel is by subdirectories.

I do however understand the fear that we end up with lots of
KBUILD_CFLAGS_W1_<DATESTAMP> constants. So i looked at the git history
of script/Makefile.extrawarn. These are historically the symbols we
would have, if we started this idea 1/1/2018:

KBUILD_CFLAGS_W1_20200326    # CLANG only change
KBUILD_CFLAGS_W1_20190907
KBUILD_CFLAGS_W1_20190617    # CLANG only change
KBUILD_CFLAGS_W1_20190614    # CLANG only change
KBUILD_CFLAGS_W1_20190509
KBUILD_CFLAGS_W1_20180919
KBUILD_CFLAGS_W1_20180111

So not too many.

   Andrew

  reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01  1:12 [RFC PATCH net-next v2 0/2] driver/net/ethernet W=1 by default Andrew Lunn
2020-10-01  1:12 ` [PATCH net-next v2 1/2] Makefile.extrawarn: Add symbol for W=1 warnings for today Andrew Lunn
2020-10-01 23:09   ` Nick Desaulniers
2020-10-02  1:44     ` Andrew Lunn [this message]
2020-10-02 12:20       ` Arnd Bergmann
2020-10-02 12:51         ` Andrew Lunn
2020-10-02 13:15           ` Arnd Bergmann
2020-10-12  1:00         ` Masahiro Yamada
2020-10-12  8:05           ` Arnd Bergmann
2020-10-05 17:31       ` Nick Desaulniers
2020-10-05 19:49         ` Andrew Lunn
2020-10-05 20:03           ` Arnd Bergmann
2020-10-05 21:08             ` Andrew Lunn
2020-10-11 13:03               ` Masahiro Yamada
2020-10-12  8:11                 ` Arnd Bergmann
2020-10-16 14:12                 ` Andrew Lunn
     [not found]                   ` <CAK8P3a1nBhmf1PQwHHbEjiVgRTXi4UuJAbwuK92CKEbR=yKGWw@mail.gmail.com>
2020-10-17 14:57                     ` Andrew Lunn
2020-10-02 11:08   ` kernel test robot
2020-10-01  1:12 ` [PATCH net-next v2 2/2] driver/net/ethernet: Sign up for W=1 as defined on 20200930 Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201002014411.GG4067422@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=arnd@arndb.de \
    --cc=clang-built-linux@googlegroups.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=rohitm@chelsio.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-KBuild Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kbuild/0 linux-kbuild/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kbuild linux-kbuild/ https://lore.kernel.org/linux-kbuild \
		linux-kbuild@vger.kernel.org
	public-inbox-index linux-kbuild

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kbuild


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git