All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
	kernel test robot <lkp@intel.com>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	kbuild-all@lists.01.org,
	Linux Memory Management List <linux-mm@kvack.org>,
	Quentin Perret <qperret@google.com>,
	Lukasz Luba <lukasz.luba@arm.com>
Subject: Re: [linux-next:master 6632/9522] include/linux/pm_opp.h:458:58: warning: unused parameter 'dev'
Date: Mon, 23 Aug 2021 13:28:09 -0700	[thread overview]
Message-ID: <YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain> (raw)
In-Reply-To: <CAK8P3a1o6uvDo_ttPFOw_pRdtok3-9=dFMLToyUxSmTxs0YfOg@mail.gmail.com>

On Mon, Aug 23, 2021 at 11:07:13AM +0200, Arnd Bergmann wrote:
> On Mon, Aug 23, 2021 at 5:15 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 21-08-21, 03:30, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > > head:   86ed57fd8c93fdfaabb4f58e78455180fa7d8a84
> > > commit: c17495b01b72b53bd290f442d39b060e015c7aea [6632/9522] cpufreq: Add callback to register with energy model
> > > config: i386-randconfig-a016-20210820 (attached as .config)
> > > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d9c5613e856cf2addfbf892fc4c1ce9ef9feceaa)
> > > reproduce (this is a W=1 build):
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c17495b01b72b53bd290f442d39b060e015c7aea
> > >         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > >         git fetch --no-tags linux-next master
> > >         git checkout c17495b01b72b53bd290f442d39b060e015c7aea
> > >         # save the attached .config to linux build tree
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > >    static inline void pm_vt_switch_unregister(struct device *dev)
> > >                                                              ^
> > >    In file included from drivers/gpu/drm/i915/gt/intel_llc.c:6:
> > >    In file included from include/linux/cpufreq.h:12:
> > >    In file included from include/linux/cpu.h:17:
> > >    In file included from include/linux/node.h:18:
> > >    include/linux/device.h:653:46: warning: unused parameter 'dev' [-Wunused-parameter]
> > >    static inline int dev_to_node(struct device *dev)
> >
> > False positives ? These are mostly inline dummies, which simply return
> > errors. Their parameters aren't supposed to be used.
> 
> It's a clang-14 W=1 build, probably something went wrong with the clang specific
> warning flags there. I think we do want "-Wunused -Wno-unused-parameter". Not
> sure what changed compared to older clang builds.

W=1 already does this:

$ sed -n '23,25p' scripts/Makefile.extrawarn
ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)

KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter

The problem is the same one as commit fa63da2ab046 ("arm64: Don't
unconditionally add -Wno-psabi to KBUILD_CFLAGS") but just with an '-f'
flag instead of a '-W' flag.

This config has CONFIG_MEFFICEON=y, which adds a few '-falign-...' flags
to cflags-y, namely '-falign-jumps=0', which is not supported by clang:

clang-14: warning: optimization flag '-falign-jumps=0' is not supported [-Wignored-optimization-argument]

As a result, all subsequent cc-{disable-warning,option} calls fail because the
command always fails with an unknown flag in the mix since we added -Werror to
cc-{disable-warning,option} to work around this exact situation where clang
only emits a warning rather than an error for unknown flags.  I improved this
in commit 589834b3a009 ("kbuild: Add -Werror=unknown-warning-option to
CLANG_FLAGS") so that we get alerted of failed flags before this but I guess I
missed this one :).

i915 enables '-Wall -Wextra' then disables a few warnings (including
'-Wunused-parameter') with cc-disable-warning, which does not work, so we get
all of these warnings as a result.

I think fixing this once and for all is a three pronged approach:

1. '-falign-jumps=' should not be added unconditionally, as it is not
   supported in clang (as it warns rather than errors about...).
   '-falign-loops=' falls into the same category, as it is only
   supported on clang-14 and newer.

   Something like this should do it (no point in making GCC pay the
   cc-option cost):

diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index cd3056759880..1db40f14719d 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -25,11 +25,16 @@ cflags-$(CONFIG_MK6)                += -march=k6
 # They make zero difference whatsosever to performance at this time.
 cflags-$(CONFIG_MK7)           += -march=athlon
 cflags-$(CONFIG_MK8)           += $(call cc-option,-march=k8,-march=athlon)
-cflags-$(CONFIG_MCRUSOE)       += -march=i686 -falign-functions=0 -falign-jumps=0 -falign-loops=0
-cflags-$(CONFIG_MEFFICEON)     += -march=i686 $(call tune,pentium3) -falign-functions=0 -falign-jumps=0 -falign-loops=0
+ifdef CONFIG_CC_IS_CLANG
+align := -falign-functions=0 $(call cc-option,-falign-jumps=0) $(call cc-option,-falign-loops=0)
+else
+align := -falign-functions=0 -falign-jumps=0 -falign-loops=0
+endif
+cflags-$(CONFIG_MCRUSOE)       += -march=i686 $(align)
+cflags-$(CONFIG_MEFFICEON)     += -march=i686 $(call tune,pentium3) $(align)
 cflags-$(CONFIG_MWINCHIPC6)    += $(call cc-option,-march=winchip-c6,-march=i586)
 cflags-$(CONFIG_MWINCHIP3D)    += $(call cc-option,-march=winchip2,-march=i586)
-cflags-$(CONFIG_MCYRIXIII)     += $(call cc-option,-march=c3,-march=i486) -falign-functions=0 -falign-jumps=0 -falign-loops=0
+cflags-$(CONFIG_MCYRIXIII)     += $(call cc-option,-march=c3,-march=i486) $(align)
 cflags-$(CONFIG_MVIAC3_2)      += $(call cc-option,-march=c3-2,-march=i686)
 cflags-$(CONFIG_MVIAC7)                += -march=i686
 cflags-$(CONFIG_MCORE2)                += -march=i686 $(call tune,core2)

2. i915 should not be using cc-disable-warning for most of the flags
   that they are, as they are supported by both compilers and the rest
   of the kernel assumes this. I plan to send a series to turn on
   -Wsometimes-uninitialized for i195 shortly so I will send this diff
   as a prior change in that series.

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 642a5b5a1b81..9f05b3b18816 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -13,15 +13,16 @@
 # will most likely get a sudden build breakage... Hopefully we will fix
 # new warnings before CI updates!
 subdir-ccflags-y := -Wall -Wextra
-subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
-subdir-ccflags-y += $(call cc-disable-warning, type-limits)
-subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
 subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
-# clang warnings
-subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
-subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
 subdir-ccflags-y += $(call cc-disable-warning, frame-address)
+ifdef CONFIG_CC_IS_CLANG
+subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-sometimes-uninitialized
+subdir-ccflags-y += -Wno-initializer-overrides
+endif
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror

 # Fine grained warnings disable

3. We should not allow unknown flags to get added to the command line
   without our knowledge. Bugs like this are annoying for several
   parties so we should minimize them as much as possible. I will
   propose this in the same series as the first diff, which should avoid
   the initial issue altogether (the second step is more of an
   optimization for the future).

diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
index f88ceb3d076e..94c9455adf59 100644
--- a/scripts/Makefile.clang
+++ b/scripts/Makefile.clang
@@ -12,7 +12,9 @@ CLANG_TARGET_FLAGS_s390               := s390x-linux-gnu
 CLANG_TARGET_FLAGS_x86         := x86_64-linux-gnu
 CLANG_TARGET_FLAGS             := $(CLANG_TARGET_FLAGS_$(SRCARCH))

+# Make clang behave more like gcc when it encounters an unknown '-W' or '-f' flag.
 TENTATIVE_CLANG_FLAGS := -Werror=unknown-warning-option
+TENTATIVE_CLANG_FLAGS += -Werror=ignored-optimization-argument

 ifeq ($(CROSS_COMPILE),)
 ifeq ($(CLANG_TARGET_FLAGS),)

Sorry for the wall of text, hopefully it all makes sense.

Cheers,
Nathan


WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <nathan@kernel.org>
To: kbuild-all@lists.01.org
Subject: Re: [linux-next:master 6632/9522] include/linux/pm_opp.h:458:58: warning: unused parameter 'dev'
Date: Mon, 23 Aug 2021 13:28:09 -0700	[thread overview]
Message-ID: <YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain> (raw)
In-Reply-To: <CAK8P3a1o6uvDo_ttPFOw_pRdtok3-9=dFMLToyUxSmTxs0YfOg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 8482 bytes --]

On Mon, Aug 23, 2021 at 11:07:13AM +0200, Arnd Bergmann wrote:
> On Mon, Aug 23, 2021 at 5:15 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 21-08-21, 03:30, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > > head:   86ed57fd8c93fdfaabb4f58e78455180fa7d8a84
> > > commit: c17495b01b72b53bd290f442d39b060e015c7aea [6632/9522] cpufreq: Add callback to register with energy model
> > > config: i386-randconfig-a016-20210820 (attached as .config)
> > > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d9c5613e856cf2addfbf892fc4c1ce9ef9feceaa)
> > > reproduce (this is a W=1 build):
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c17495b01b72b53bd290f442d39b060e015c7aea
> > >         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > >         git fetch --no-tags linux-next master
> > >         git checkout c17495b01b72b53bd290f442d39b060e015c7aea
> > >         # save the attached .config to linux build tree
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > >    static inline void pm_vt_switch_unregister(struct device *dev)
> > >                                                              ^
> > >    In file included from drivers/gpu/drm/i915/gt/intel_llc.c:6:
> > >    In file included from include/linux/cpufreq.h:12:
> > >    In file included from include/linux/cpu.h:17:
> > >    In file included from include/linux/node.h:18:
> > >    include/linux/device.h:653:46: warning: unused parameter 'dev' [-Wunused-parameter]
> > >    static inline int dev_to_node(struct device *dev)
> >
> > False positives ? These are mostly inline dummies, which simply return
> > errors. Their parameters aren't supposed to be used.
> 
> It's a clang-14 W=1 build, probably something went wrong with the clang specific
> warning flags there. I think we do want "-Wunused -Wno-unused-parameter". Not
> sure what changed compared to older clang builds.

W=1 already does this:

$ sed -n '23,25p' scripts/Makefile.extrawarn
ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)

KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter

The problem is the same one as commit fa63da2ab046 ("arm64: Don't
unconditionally add -Wno-psabi to KBUILD_CFLAGS") but just with an '-f'
flag instead of a '-W' flag.

This config has CONFIG_MEFFICEON=y, which adds a few '-falign-...' flags
to cflags-y, namely '-falign-jumps=0', which is not supported by clang:

clang-14: warning: optimization flag '-falign-jumps=0' is not supported [-Wignored-optimization-argument]

As a result, all subsequent cc-{disable-warning,option} calls fail because the
command always fails with an unknown flag in the mix since we added -Werror to
cc-{disable-warning,option} to work around this exact situation where clang
only emits a warning rather than an error for unknown flags.  I improved this
in commit 589834b3a009 ("kbuild: Add -Werror=unknown-warning-option to
CLANG_FLAGS") so that we get alerted of failed flags before this but I guess I
missed this one :).

i915 enables '-Wall -Wextra' then disables a few warnings (including
'-Wunused-parameter') with cc-disable-warning, which does not work, so we get
all of these warnings as a result.

I think fixing this once and for all is a three pronged approach:

1. '-falign-jumps=' should not be added unconditionally, as it is not
   supported in clang (as it warns rather than errors about...).
   '-falign-loops=' falls into the same category, as it is only
   supported on clang-14 and newer.

   Something like this should do it (no point in making GCC pay the
   cc-option cost):

diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index cd3056759880..1db40f14719d 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -25,11 +25,16 @@ cflags-$(CONFIG_MK6)                += -march=k6
 # They make zero difference whatsosever to performance at this time.
 cflags-$(CONFIG_MK7)           += -march=athlon
 cflags-$(CONFIG_MK8)           += $(call cc-option,-march=k8,-march=athlon)
-cflags-$(CONFIG_MCRUSOE)       += -march=i686 -falign-functions=0 -falign-jumps=0 -falign-loops=0
-cflags-$(CONFIG_MEFFICEON)     += -march=i686 $(call tune,pentium3) -falign-functions=0 -falign-jumps=0 -falign-loops=0
+ifdef CONFIG_CC_IS_CLANG
+align := -falign-functions=0 $(call cc-option,-falign-jumps=0) $(call cc-option,-falign-loops=0)
+else
+align := -falign-functions=0 -falign-jumps=0 -falign-loops=0
+endif
+cflags-$(CONFIG_MCRUSOE)       += -march=i686 $(align)
+cflags-$(CONFIG_MEFFICEON)     += -march=i686 $(call tune,pentium3) $(align)
 cflags-$(CONFIG_MWINCHIPC6)    += $(call cc-option,-march=winchip-c6,-march=i586)
 cflags-$(CONFIG_MWINCHIP3D)    += $(call cc-option,-march=winchip2,-march=i586)
-cflags-$(CONFIG_MCYRIXIII)     += $(call cc-option,-march=c3,-march=i486) -falign-functions=0 -falign-jumps=0 -falign-loops=0
+cflags-$(CONFIG_MCYRIXIII)     += $(call cc-option,-march=c3,-march=i486) $(align)
 cflags-$(CONFIG_MVIAC3_2)      += $(call cc-option,-march=c3-2,-march=i686)
 cflags-$(CONFIG_MVIAC7)                += -march=i686
 cflags-$(CONFIG_MCORE2)                += -march=i686 $(call tune,core2)

2. i915 should not be using cc-disable-warning for most of the flags
   that they are, as they are supported by both compilers and the rest
   of the kernel assumes this. I plan to send a series to turn on
   -Wsometimes-uninitialized for i195 shortly so I will send this diff
   as a prior change in that series.

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 642a5b5a1b81..9f05b3b18816 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -13,15 +13,16 @@
 # will most likely get a sudden build breakage... Hopefully we will fix
 # new warnings before CI updates!
 subdir-ccflags-y := -Wall -Wextra
-subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
-subdir-ccflags-y += $(call cc-disable-warning, type-limits)
-subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
 subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
-# clang warnings
-subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
-subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
 subdir-ccflags-y += $(call cc-disable-warning, frame-address)
+ifdef CONFIG_CC_IS_CLANG
+subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-sometimes-uninitialized
+subdir-ccflags-y += -Wno-initializer-overrides
+endif
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror

 # Fine grained warnings disable

3. We should not allow unknown flags to get added to the command line
   without our knowledge. Bugs like this are annoying for several
   parties so we should minimize them as much as possible. I will
   propose this in the same series as the first diff, which should avoid
   the initial issue altogether (the second step is more of an
   optimization for the future).

diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
index f88ceb3d076e..94c9455adf59 100644
--- a/scripts/Makefile.clang
+++ b/scripts/Makefile.clang
@@ -12,7 +12,9 @@ CLANG_TARGET_FLAGS_s390               := s390x-linux-gnu
 CLANG_TARGET_FLAGS_x86         := x86_64-linux-gnu
 CLANG_TARGET_FLAGS             := $(CLANG_TARGET_FLAGS_$(SRCARCH))

+# Make clang behave more like gcc when it encounters an unknown '-W' or '-f' flag.
 TENTATIVE_CLANG_FLAGS := -Werror=unknown-warning-option
+TENTATIVE_CLANG_FLAGS += -Werror=ignored-optimization-argument

 ifeq ($(CROSS_COMPILE),)
 ifeq ($(CLANG_TARGET_FLAGS),)

Sorry for the wall of text, hopefully it all makes sense.

Cheers,
Nathan

  reply	other threads:[~2021-08-23 20:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20 19:30 [linux-next:master 6632/9522] include/linux/pm_opp.h:458:58: warning: unused parameter 'dev' kernel test robot
2021-08-20 19:30 ` kernel test robot
2021-08-23  3:15 ` Viresh Kumar
2021-08-23  3:15   ` Viresh Kumar
2021-08-23  9:07   ` Arnd Bergmann
2021-08-23  9:07     ` Arnd Bergmann
2021-08-23 20:28     ` Nathan Chancellor [this message]
2021-08-23 20:28       ` Nathan Chancellor

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=YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain \
    --to=nathan@kernel.org \
    --cc=arnd@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=lukasz.luba@arm.com \
    --cc=qperret@google.com \
    --cc=viresh.kumar@linaro.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.