linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Build failure: -Wno-unused-const-variable DNE on old GCC
@ 2016-01-07 18:54 Brian Norris
  2016-01-07 19:37 ` Joe Perches
  2016-01-07 20:25 ` Arnd Bergmann
  0 siblings, 2 replies; 17+ messages in thread
From: Brian Norris @ 2016-01-07 18:54 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Anton Blanchard, Ian Munsie, Michael Neuling, linuxppc-dev,
	linux-kernel, Michal Marek

Hi,

I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed
that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable
behaviour change") breaks my builds, because the
-Wno-unused-const-variable doesn't exist on GCC 4.6.3.

  drivers/misc/cxl/base.c: At top level:
  cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]

Any thoughts on how to best fix this? I'd like not to have to scrounge
up a new cross compiler just for build tests.

Regards,
Brian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 18:54 Build failure: -Wno-unused-const-variable DNE on old GCC Brian Norris
@ 2016-01-07 19:37 ` Joe Perches
  2016-01-07 19:44   ` Michal Marek
  2016-01-07 20:25 ` Arnd Bergmann
  1 sibling, 1 reply; 17+ messages in thread
From: Joe Perches @ 2016-01-07 19:37 UTC (permalink / raw)
  To: Brian Norris, Michael Ellerman
  Cc: Anton Blanchard, Ian Munsie, Michael Neuling, linuxppc-dev,
	linux-kernel, Michal Marek

On Thu, 2016-01-07 at 10:54 -0800, Brian Norris wrote:
> Hi,
> 
> I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed
> that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable
> behaviour change") breaks my builds, because the
> -Wno-unused-const-variable doesn't exist on GCC 4.6.3.
> 
>   drivers/misc/cxl/base.c: At top level:
>   cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]
> 
> Any thoughts on how to best fix this? I'd like not to have to scrounge
> up a new cross compiler just for build tests.

drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable

You could take that -Wno-unused-const-variable out of the
Makefile or maybe add something like:

$(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)

or whatever gcc version actually added that unused-const-variable check

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 19:37 ` Joe Perches
@ 2016-01-07 19:44   ` Michal Marek
  2016-01-07 19:57     ` Joe Perches
  0 siblings, 1 reply; 17+ messages in thread
From: Michal Marek @ 2016-01-07 19:44 UTC (permalink / raw)
  To: Joe Perches, Brian Norris, Michael Ellerman
  Cc: Anton Blanchard, Ian Munsie, Michael Neuling, linuxppc-dev, linux-kernel

Dne 7.1.2016 v 20:37 Joe Perches napsal(a):
> On Thu, 2016-01-07 at 10:54 -0800, Brian Norris wrote:
>> Hi,
>>
>> I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed
>> that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable
>> behaviour change") breaks my builds, because the
>> -Wno-unused-const-variable doesn't exist on GCC 4.6.3.
>>
>>   drivers/misc/cxl/base.c: At top level:
>>   cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]
>>
>> Any thoughts on how to best fix this? I'd like not to have to scrounge
>> up a new cross compiler just for build tests.
> 
> drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable
> 
> You could take that -Wno-unused-const-variable out of the
> Makefile or maybe add something like:
> 
> $(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)
> 
> or whatever gcc version actually added that unused-const-variable check

We have cc-disable-warning for this.

Michal

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 19:44   ` Michal Marek
@ 2016-01-07 19:57     ` Joe Perches
  2016-01-07 20:18       ` Brian Norris
  0 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2016-01-07 19:57 UTC (permalink / raw)
  To: Michal Marek, Brian Norris, Michael Ellerman
  Cc: Anton Blanchard, Ian Munsie, Michael Neuling, linuxppc-dev, linux-kernel

On Thu, 2016-01-07 at 20:44 +0100, Michal Marek wrote:
> Dne 7.1.2016 v 20:37 Joe Perches napsal(a):
> > On Thu, 2016-01-07 at 10:54 -0800, Brian Norris wrote:
> > > I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed
> > > that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable
> > > behaviour change") breaks my builds, because the
> > > -Wno-unused-const-variable doesn't exist on GCC 4.6.3.
> > >   drivers/misc/cxl/base.c: At top level:
> > >   cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]
> > > Any thoughts on how to best fix this? I'd like not to have to scrounge
> > > up a new cross compiler just for build tests.
> > drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable
> > You could take that -Wno-unused-const-variable out of the
> > Makefile or maybe add something like:
> > 
> > $(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)
> > 
> > or whatever gcc version actually added that unused-const-variable check
> 
> We have cc-disable-warning for this.

Thanks Michal.

Perhaps most uses of -Werror without some CONFIG_<FOO> guard
should be removed or replaced by some other mechanism.

$ git grep -E "=\s*\-Werror" | grep -v CONFIG
[...]
arch/alpha/lib/Makefile:ccflags-y := -Werror
arch/alpha/mm/Makefile:ccflags-y := -Werror
arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
arch/mips/Kbuild:subdir-ccflags-y := -Werror
arch/sh/cchips/hd6446x/Makefile:ccflags-y := -Werror
arch/sh/kernel/Makefile:ccflags-y := -Werror
arch/sh/lib/Makefile:ccflags-y := -Werror
arch/sh/mm/Makefile:ccflags-y := -Werror
arch/sparc/kernel/Makefile:ccflags-y := -Werror
arch/sparc/lib/Makefile:ccflags-y := -Werror
arch/sparc/mm/Makefile:ccflags-y := -Werror
arch/sparc/prom/Makefile:ccflags := -Werror
drivers/gpu/drm/tilcdc/Makefile:	ccflags-y += -Werror
drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable
drivers/scsi/aic7xxx/Makefile:ccflags-y += -Werror
drivers/scsi/lpfc/Makefile:ccflags-y += -Werror

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 19:57     ` Joe Perches
@ 2016-01-07 20:18       ` Brian Norris
  2016-01-07 20:38         ` [PATCH] misc: cxl: fix build for GCC 4.6.x Brian Norris
  2016-01-30 14:20         ` Build failure: -Wno-unused-const-variable DNE on old GCC Maciej W. Rozycki
  0 siblings, 2 replies; 17+ messages in thread
From: Brian Norris @ 2016-01-07 20:18 UTC (permalink / raw)
  To: Joe Perches
  Cc: Michal Marek, Michael Ellerman, Anton Blanchard, Ian Munsie,
	Michael Neuling, linuxppc-dev, linux-kernel, Olof Johansson

On Thu, Jan 07, 2016 at 11:57:31AM -0800, Joe Perches wrote:
> On Thu, 2016-01-07 at 20:44 +0100, Michal Marek wrote:
> > Dne 7.1.2016 v 20:37 Joe Perches napsal(a):
> > > On Thu, 2016-01-07 at 10:54 -0800, Brian Norris wrote:
> > > > I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed
> > > > that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable
> > > > behaviour change") breaks my builds, because the
> > > > -Wno-unused-const-variable doesn't exist on GCC 4.6.3.
> > > >   drivers/misc/cxl/base.c: At top level:
> > > >   cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]
> > > > Any thoughts on how to best fix this? I'd like not to have to scrounge
> > > > up a new cross compiler just for build tests.
> > > drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable
> > > You could take that -Wno-unused-const-variable out of the
> > > Makefile or maybe add something like:
> > > 
> > > $(call cc-ifversion, -ge, 0530, -Wno-unused-const-variable)
> > > 
> > > or whatever gcc version actually added that unused-const-variable check
> > 
> > We have cc-disable-warning for this.
> 
> Thanks Michal.

Cool, thanks! I'll send a patch to use that instead.

> Perhaps most uses of -Werror without some CONFIG_<FOO> guard
> should be removed or replaced by some other mechanism.

+1000. I'd personally like to see all one-off uses of -Werror removed.

> $ git grep -E "=\s*\-Werror" | grep -v CONFIG
> [...]
> arch/alpha/lib/Makefile:ccflags-y := -Werror
> arch/alpha/mm/Makefile:ccflags-y := -Werror
> arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
> arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
> arch/mips/Kbuild:subdir-ccflags-y := -Werror

^^ I always patch this one out when build-testing MIPS, since I like to
turn up warning levels (e.g., W=1), but not kill the build entirely.

Regards,
Brian

> arch/sh/cchips/hd6446x/Makefile:ccflags-y := -Werror
> arch/sh/kernel/Makefile:ccflags-y := -Werror
> arch/sh/lib/Makefile:ccflags-y := -Werror
> arch/sh/mm/Makefile:ccflags-y := -Werror
> arch/sparc/kernel/Makefile:ccflags-y := -Werror
> arch/sparc/lib/Makefile:ccflags-y := -Werror
> arch/sparc/mm/Makefile:ccflags-y := -Werror
> arch/sparc/prom/Makefile:ccflags := -Werror
> drivers/gpu/drm/tilcdc/Makefile:	ccflags-y += -Werror
> drivers/misc/cxl/Makefile:ccflags-y := -Werror -Wno-unused-const-variable
> drivers/scsi/aic7xxx/Makefile:ccflags-y += -Werror
> drivers/scsi/lpfc/Makefile:ccflags-y += -Werror
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 18:54 Build failure: -Wno-unused-const-variable DNE on old GCC Brian Norris
  2016-01-07 19:37 ` Joe Perches
@ 2016-01-07 20:25 ` Arnd Bergmann
  2016-01-07 22:51   ` Daniel Axtens
  2016-01-08  1:33   ` Ian Munsie
  1 sibling, 2 replies; 17+ messages in thread
From: Arnd Bergmann @ 2016-01-07 20:25 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Brian Norris, Michael Ellerman, Michael Neuling, Anton Blanchard,
	linux-kernel, Michal Marek, Ian Munsie

On Thursday 07 January 2016 10:54:06 Brian Norris wrote:
> 
> I'm using a GCC 4.6.3 compiler for some compile tests, and I noticed
> that commit 2cd55c68c0a4 ("cxl: Fix build failure due to -Wunused-variable
> behaviour change") breaks my builds, because the
> -Wno-unused-const-variable doesn't exist on GCC 4.6.3.
> 
>   drivers/misc/cxl/base.c: At top level:
>   cc1: error: unrecognized command line option "-Wno-unused-const-variable" [-Werror]
> 
> Any thoughts on how to best fix this? I'd like not to have to scrounge
> up a new cross compiler just for build tests.
> 

This should do:

diff --git a/drivers/misc/cxl/Makefile b/drivers/misc/cxl/Makefile
index 6982f603fadc..add2cc17ed91 100644
--- a/drivers/misc/cxl/Makefile
+++ b/drivers/misc/cxl/Makefile
@@ -1,4 +1,4 @@
-ccflags-y := -Werror -Wno-unused-const-variable
+ccflags-y := -Werror $(call cc-disable-warning,unused-const-variable)
 
 cxl-y				+= main.o file.o irq.o fault.o native.o
 cxl-y				+= context.o sysfs.o debugfs.o pci.o trace.o


Alternatively, remove the -Werror. We occasionally get people that add this
flag to a Makefile, but it tends to cause more trouble whenever a new
gcc version arrives.

	Arnd

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] misc: cxl: fix build for GCC 4.6.x
  2016-01-07 20:18       ` Brian Norris
@ 2016-01-07 20:38         ` Brian Norris
  2016-01-08  2:12           ` Michael Ellerman
  2016-01-30 14:20         ` Build failure: -Wno-unused-const-variable DNE on old GCC Maciej W. Rozycki
  1 sibling, 1 reply; 17+ messages in thread
From: Brian Norris @ 2016-01-07 20:38 UTC (permalink / raw)
  To: Joe Perches
  Cc: Michal Marek, Michael Ellerman, Anton Blanchard, Ian Munsie,
	Michael Neuling, linuxppc-dev, linux-kernel, Olof Johansson,
	Arnd Bergmann

GCC 4.6.3 does not support -Wno-unused-const-variable. Instead, use the
kbuild infrastructure that checks if this options exists.

Also drop -Werror, since it's harmful, if forced on the user. New GCC's,
or higher warning verbosities (e.g., W=1) can easily kill the build
where they shouldn't.

Suggested-by: Michal Marek <mmarek@suse.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---

On Thu, Jan 07, 2016 at 12:18:26PM -0800, Brian Norris wrote:
> On Thu, Jan 07, 2016 at 11:57:31AM -0800, Joe Perches wrote:
> > On Thu, 2016-01-07 at 20:44 +0100, Michal Marek wrote:
> > > We have cc-disable-warning for this.
> > 
> > Thanks Michal.
> 
> Cool, thanks! I'll send a patch to use that instead.

 drivers/misc/cxl/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/cxl/Makefile b/drivers/misc/cxl/Makefile
index 6982f603fadc..0163d4d1fd1e 100644
--- a/drivers/misc/cxl/Makefile
+++ b/drivers/misc/cxl/Makefile
@@ -1,4 +1,4 @@
-ccflags-y := -Werror -Wno-unused-const-variable
+ccflags-y := $(call cc-disable-warning, unused-const-variable)
 
 cxl-y				+= main.o file.o irq.o fault.o native.o
 cxl-y				+= context.o sysfs.o debugfs.o pci.o trace.o
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 20:25 ` Arnd Bergmann
@ 2016-01-07 22:51   ` Daniel Axtens
  2016-01-07 23:02     ` Brian Norris
  2016-01-08  1:33   ` Ian Munsie
  1 sibling, 1 reply; 17+ messages in thread
From: Daniel Axtens @ 2016-01-07 22:51 UTC (permalink / raw)
  To: Arnd Bergmann, linuxppc-dev
  Cc: Brian Norris, Michael Ellerman, Michael Neuling, Anton Blanchard,
	linux-kernel, Michal Marek, Ian Munsie

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

Hi Arnd,

Thanks for your patch.
Acked-by: Daniel Axtens <dja@axtens.net>

> Alternatively, remove the -Werror. We occasionally get people that add this
> flag to a Makefile, but it tends to cause more trouble whenever a new
> gcc version arrives.

Speaking up as the person who added -Werror to cxl, I'd really rather
it stayed. There are a number of reasons I think this. Here's the first
three that came to mind.

 - cxl is powerpc specific (and always will be for deep seated hardware
   reasons), and is handled through the powerpc tree. arch/powerpc
   compiles with -Werror, and as part of the powerpc ecosystem, cxl
   should too.

 - It forces cxl developers to a higher standard. cxl has already had
   more than it's fair share of incredibly difficult to debug issues,
   so any way we can reduce the risk of errors going in makes our lives
   (and our end-users lives) better.

 - I am (and I'm quite confident the other cxl people are) quite happy to
   send patches to fix build-breaking issues such as this. Indeed, I
   would have, except you sent it during the Australian night :)

If it's really super-duper important we can consider putting it behind a
config guard, but I'd really rather not.

Regards,
Daniel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 859 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 22:51   ` Daniel Axtens
@ 2016-01-07 23:02     ` Brian Norris
  2016-01-08  1:31       ` Ian Munsie
  0 siblings, 1 reply; 17+ messages in thread
From: Brian Norris @ 2016-01-07 23:02 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: Arnd Bergmann, linuxppc-dev, Michael Ellerman, Michael Neuling,
	Anton Blanchard, linux-kernel, Michal Marek, Ian Munsie

On Fri, Jan 08, 2016 at 09:51:35AM +1100, Daniel Axtens wrote:
> 
> > Alternatively, remove the -Werror. We occasionally get people that add this
> > flag to a Makefile, but it tends to cause more trouble whenever a new
> > gcc version arrives.

^^ Your reasons below don't really address this point. No matter how
well you patch a later kernel release, you can't fix a problem in an
existing kernel release that is triggered by a new warning in a new
compiler. This shouldn't cause a build failure.

> Speaking up as the person who added -Werror to cxl, I'd really rather
> it stayed. There are a number of reasons I think this. Here's the first
> three that came to mind.
> 
>  - cxl is powerpc specific (and always will be for deep seated hardware
>    reasons), and is handled through the powerpc tree. arch/powerpc
>    compiles with -Werror, and as part of the powerpc ecosystem, cxl
>    should too.
> 
>  - It forces cxl developers to a higher standard. cxl has already had
>    more than it's fair share of incredibly difficult to debug issues,
>    so any way we can reduce the risk of errors going in makes our lives
>    (and our end-users lives) better.

One problem with this point: not all warnings are under the purview of
cxl developers. For instance, if I turn up warning verbosity (W=1), then
the *header* files start producing plenty of warnings. Should this break
the build? Your code didn't change, and you can't fix those errors.

That is a real use case for me daily: I turn the warning verbosity up on
my compile tests, then (smart)diff the build logs before and after
new patches. That way, I can see what new warnings (even potentially
false positive ones) are introduced. I can't do that if every random
developer wants to stick -Werror in their Makefile.

>  - I am (and I'm quite confident the other cxl people are) quite happy to
>    send patches to fix build-breaking issues such as this. Indeed, I
>    would have, except you sent it during the Australian night :)
> 
> If it's really super-duper important we can consider putting it behind a
> config guard, but I'd really rather not.

I think there are plenty of reasons to either remove -Werror, or make it
configurable. Some of them are detailed above.

Maybe you can gate the -Werror on CONFIG_PPC_WERROR, just like the rest
of PowerPC?

Brian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 23:02     ` Brian Norris
@ 2016-01-08  1:31       ` Ian Munsie
  2016-01-08  2:07         ` Brian Norris
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Munsie @ 2016-01-08  1:31 UTC (permalink / raw)
  To: Brian Norris
  Cc: Daniel Axtens, Arnd Bergmann, linuxppc-dev, Michael Ellerman,
	Michael Neuling, Anton Blanchard, linux-kernel, Michal Marek

Excerpts from Brian Norris's message of 2016-01-08 10:02:25 +1100:
> >  - It forces cxl developers to a higher standard. cxl has already had
> >    more than it's fair share of incredibly difficult to debug issues,
> >    so any way we can reduce the risk of errors going in makes our lives
> >    (and our end-users lives) better.
> 
> One problem with this point: not all warnings are under the purview of
> cxl developers. For instance, if I turn up warning verbosity (W=1), then
> the *header* files start producing plenty of warnings. Should this break
> the build? Your code didn't change, and you can't fix those errors.

That's a good point, but the specific warnings that we suppressed in the
new compiler are in drivers/misc/cxl/cxl.h, which is an internal header
that should only ever be included by the cxl driver. We do have some
headers elsewhere which are included by other drivers, the generic ppc
architecture code and userspace, but these are all warning free and
won't be affected by the -Werror when included from elsewhere.

> That is a real use case for me daily: I turn the warning verbosity up on
> my compile tests, then (smart)diff the build logs before and after
> new patches. That way, I can see what new warnings (even potentially
> false positive ones) are introduced. I can't do that if every random
> developer wants to stick -Werror in their Makefile.

Makes sense.

> I think there are plenty of reasons to either remove -Werror, or make it
> configurable. Some of them are detailed above.
>
> Maybe you can gate the -Werror on CONFIG_PPC_WERROR, just like the rest
> of PowerPC?

Agreed.

@Daniel - since you added the -Werror do you want to do this, or shall
I?

Cheers,
-Ian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 20:25 ` Arnd Bergmann
  2016-01-07 22:51   ` Daniel Axtens
@ 2016-01-08  1:33   ` Ian Munsie
  1 sibling, 0 replies; 17+ messages in thread
From: Ian Munsie @ 2016-01-08  1:33 UTC (permalink / raw)
  To: Arnd Bergmann, Michael Ellerman
  Cc: linuxppc-dev, Brian Norris, Michael Neuling, Anton Blanchard,
	linux-kernel, Michal Marek

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

As suggested by Brian we might also gate the -Werror with
CONFIG_PPC_WERROR, but that can be in a separate commit.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-08  1:31       ` Ian Munsie
@ 2016-01-08  2:07         ` Brian Norris
  2016-01-08  2:16           ` Michael Ellerman
  0 siblings, 1 reply; 17+ messages in thread
From: Brian Norris @ 2016-01-08  2:07 UTC (permalink / raw)
  To: Ian Munsie
  Cc: Daniel Axtens, Arnd Bergmann, linuxppc-dev, Michael Ellerman,
	Michael Neuling, Anton Blanchard, linux-kernel, Michal Marek

On Fri, Jan 08, 2016 at 12:31:54PM +1100, Ian Munsie wrote:
> Excerpts from Brian Norris's message of 2016-01-08 10:02:25 +1100:
> > >  - It forces cxl developers to a higher standard. cxl has already had
> > >    more than it's fair share of incredibly difficult to debug issues,
> > >    so any way we can reduce the risk of errors going in makes our lives
> > >    (and our end-users lives) better.
> > 
> > One problem with this point: not all warnings are under the purview of
> > cxl developers. For instance, if I turn up warning verbosity (W=1), then

(BTW, I think most of these files are currently clean with W=1, but not
W=2)

> > the *header* files start producing plenty of warnings. Should this break
> > the build? Your code didn't change, and you can't fix those errors.
> 
> That's a good point, but the specific warnings that we suppressed in the
> new compiler are in drivers/misc/cxl/cxl.h, which is an internal header
> that should only ever be included by the cxl driver. We do have some
> headers elsewhere which are included by other drivers, the generic ppc
> architecture code and userspace, but these are all warning free and
> won't be affected by the -Werror when included from elsewhere.

I was referring to when extra warnings are turned on, not just the
default. So this fails spectacularly:

  $ export ARCH=powerpc
  $ make ppc64_defconfig
  $ make drivers/misc/cxl/api.o W=2
    CC [M]  drivers/misc/cxl/api.o
  In file included from include/linux/bitops.h:36:0,
                   from include/linux/kernel.h:10,
                   from include/linux/list.h:8,
                   from include/linux/pci.h:25,
                   from drivers/misc/cxl/api.c:10:
  ./arch/powerpc/include/asm/bitops.h:226:94: error: declaration of 'ffs' shadows a built-in function [-Werror=shadow]
  In file included from include/linux/atomic.h:562:0,
                   from include/linux/mutex.h:18,
                   from include/linux/kernfs.h:13,
                   from include/linux/sysfs.h:15,
                   from include/linux/kobject.h:21,
                   from include/linux/pci.h:28,
                   from drivers/misc/cxl/api.c:10:
  include/asm-generic/atomic-long.h: In function 'atomic_long_read_acquire':
  include/asm-generic/atomic-long.h:45:231: error: nested extern declaration of '__compiletime_assert_45' [-Werror=nested-externs]
  In file included from include/linux/atomic.h:562:0,
                   from include/linux/mutex.h:18,
                   from include/linux/kernfs.h:13,
                   from include/linux/sysfs.h:15,
                   from include/linux/kobject.h:21,
                   from include/linux/pci.h:28,
                   from drivers/misc/cxl/api.c:10:
  include/asm-generic/atomic-long.h: In function 'atomic_long_set_release':
  include/asm-generic/atomic-long.h:57:1: error: nested extern declaration of '__compiletime_assert_57' [-Werror=nested-externs]
  In file included from include/linux/ktime.h:25:0,
                   from include/linux/rcupdate.h:47,
                   from include/linux/idr.h:18,
                   from include/linux/kernfs.h:14,
                   from include/linux/sysfs.h:15,
                   from include/linux/kobject.h:21,
                   from include/linux/pci.h:28,
                   from drivers/misc/cxl/api.c:10:
  include/linux/jiffies.h: In function 'jiffies_to_timespec':
  include/linux/jiffies.h:422:131: error: declaration of 'jiffies' shadows a global declaration [-Werror=shadow]
  include/linux/jiffies.h:78:65: error: shadowed declaration is here [-Werror=shadow]
  In file included from include/linux/kernfs.h:16:0,
                   from include/linux/sysfs.h:15,
                   from include/linux/kobject.h:21,
                   from include/linux/pci.h:28,
                   from drivers/misc/cxl/api.c:10:
  [...many more failures...]
  cc1: all warnings being treated as errors
  make[1]: *** [drivers/misc/cxl/api.o] Error 1
  make: *** [drivers/misc/cxl/api.o] Error 2


I doubt you plan to fix all those. So making -Werror configurable seems like
the only way forward, then. (Glad you agreed!)

Regards,
Brian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] misc: cxl: fix build for GCC 4.6.x
  2016-01-07 20:38         ` [PATCH] misc: cxl: fix build for GCC 4.6.x Brian Norris
@ 2016-01-08  2:12           ` Michael Ellerman
  0 siblings, 0 replies; 17+ messages in thread
From: Michael Ellerman @ 2016-01-08  2:12 UTC (permalink / raw)
  To: Brian Norris, Joe Perches
  Cc: Michal Marek, Anton Blanchard, Ian Munsie, Michael Neuling,
	linuxppc-dev, linux-kernel, Olof Johansson, Arnd Bergmann

On Thu, 2016-01-07 at 12:38 -0800, Brian Norris wrote:

> GCC 4.6.3 does not support -Wno-unused-const-variable. Instead, use the
> kbuild infrastructure that checks if this options exists.

Thanks.

> Also drop -Werror, since it's harmful, if forced on the user. New GCC's,
> or higher warning verbosities (e.g., W=1) can easily kill the build
> where they shouldn't.

But no thanks.

Please resend with just the cc-disable-warning change.

cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-08  2:07         ` Brian Norris
@ 2016-01-08  2:16           ` Michael Ellerman
  2016-01-08 10:14             ` David Laight
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Ellerman @ 2016-01-08  2:16 UTC (permalink / raw)
  To: Brian Norris, Ian Munsie
  Cc: Daniel Axtens, Arnd Bergmann, linuxppc-dev, Michael Neuling,
	Anton Blanchard, linux-kernel, Michal Marek

On Thu, 2016-01-07 at 18:07 -0800, Brian Norris wrote:
> On Fri, Jan 08, 2016 at 12:31:54PM +1100, Ian Munsie wrote:
> > Excerpts from Brian Norris's message of 2016-01-08 10:02:25 +1100:
> > > >  - It forces cxl developers to a higher standard. cxl has already had
> > > >    more than it's fair share of incredibly difficult to debug issues,
> > > >    so any way we can reduce the risk of errors going in makes our lives
> > > >    (and our end-users lives) better.
> > > 
> > > One problem with this point: not all warnings are under the purview of
> > > cxl developers. For instance, if I turn up warning verbosity (W=1), then
> 
> (BTW, I think most of these files are currently clean with W=1, but not W=2)

> > > the *header* files start producing plenty of warnings. Should this break
> > > the build? Your code didn't change, and you can't fix those errors.
> > 
> > That's a good point, but the specific warnings that we suppressed in the
> > new compiler are in drivers/misc/cxl/cxl.h, which is an internal header
> > that should only ever be included by the cxl driver. We do have some
> > headers elsewhere which are included by other drivers, the generic ppc
> > architecture code and userspace, but these are all warning free and
> > won't be affected by the -Werror when included from elsewhere.
> 
> I was referring to when extra warnings are turned on, not just the
> default. So this fails spectacularly:
> 
>   $ export ARCH=powerpc
>   $ make ppc64_defconfig
>   $ make drivers/misc/cxl/api.o W=2
>     CC [M]  drivers/misc/cxl/api.o
>   In file included from include/linux/bitops.h:36:0,
>                    from include/linux/kernel.h:10,
>                    from include/linux/list.h:8,
>                    from include/linux/pci.h:25,
>                    from drivers/misc/cxl/api.c:10:
>   ./arch/powerpc/include/asm/bitops.h:226:94: error: declaration of 'ffs' shadows a built-in function [-Werror=shadow]

>
> I doubt you plan to fix all those. So making -Werror configurable seems like
> the only way forward, then. (Glad you agreed!)

Yep, I'm happy to make Werror configurable.

cxl can probably just use the existing PPC_WERROR.

cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-08  2:16           ` Michael Ellerman
@ 2016-01-08 10:14             ` David Laight
  0 siblings, 0 replies; 17+ messages in thread
From: David Laight @ 2016-01-08 10:14 UTC (permalink / raw)
  To: 'Michael Ellerman', Brian Norris, Ian Munsie
  Cc: Michael Neuling, Arnd Bergmann, Anton Blanchard, linux-kernel,
	Michal Marek, linuxppc-dev, Daniel Axtens

From: Michael Ellerman
> Sent: 08 January 2016 02:17
> > I doubt you plan to fix all those. So making -Werror configurable seems like
> > the only way forward, then. (Glad you agreed!)
> 
> Yep, I'm happy to make Werror configurable.
> 
> cxl can probably just use the existing PPC_WERROR.

Personally I think the development builds should be done with
both error -Werror and higher levels of warnings, even if the
ones that users tend to do ignore more warnings.

Clearly some warnings that some versions of gcc have included
in -Wall are just a PITA (like warning for __DATE__ and __TIME__).

The kernel headers do need fixing so that they pass compilation
with options like -Wwrite-strings (linux/mm.h doesn't).

	David

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-07 20:18       ` Brian Norris
  2016-01-07 20:38         ` [PATCH] misc: cxl: fix build for GCC 4.6.x Brian Norris
@ 2016-01-30 14:20         ` Maciej W. Rozycki
  2016-01-30 17:37           ` Joe Perches
  1 sibling, 1 reply; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-30 14:20 UTC (permalink / raw)
  To: Brian Norris
  Cc: Joe Perches, Michal Marek, Michael Ellerman, Anton Blanchard,
	Ian Munsie, Michael Neuling, linuxppc-dev, linux-kernel,
	Olof Johansson

On Thu, 7 Jan 2016, Brian Norris wrote:

> > Perhaps most uses of -Werror without some CONFIG_<FOO> guard
> > should be removed or replaced by some other mechanism.
> 
> +1000. I'd personally like to see all one-off uses of -Werror removed.
> 
> > $ git grep -E "=\s*\-Werror" | grep -v CONFIG
> > [...]
> > arch/alpha/lib/Makefile:ccflags-y := -Werror
> > arch/alpha/mm/Makefile:ccflags-y := -Werror
> > arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
> > arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
> > arch/mips/Kbuild:subdir-ccflags-y := -Werror
> 
> ^^ I always patch this one out when build-testing MIPS, since I like to
> turn up warning levels (e.g., W=1), but not kill the build entirely.

 The MIPS port switched on -Werror years ago, because people submitted 
awful code and couldn't be bothered unless the build crashed.  You're 
welcome to patch your own tree, however I maintain it was a very good 
decision, and TBH I think -Werror should be on globally.

  Maciej

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Build failure: -Wno-unused-const-variable DNE on old GCC
  2016-01-30 14:20         ` Build failure: -Wno-unused-const-variable DNE on old GCC Maciej W. Rozycki
@ 2016-01-30 17:37           ` Joe Perches
  0 siblings, 0 replies; 17+ messages in thread
From: Joe Perches @ 2016-01-30 17:37 UTC (permalink / raw)
  To: Maciej W. Rozycki, Brian Norris
  Cc: Michal Marek, Michael Ellerman, Anton Blanchard, Ian Munsie,
	Michael Neuling, linuxppc-dev, linux-kernel, Olof Johansson

On Sat, 2016-01-30 at 14:20 +0000, Maciej W. Rozycki wrote:
> On Thu, 7 Jan 2016, Brian Norris wrote:
> 
> > > Perhaps most uses of -Werror without some CONFIG_ guard
> > > should be removed or replaced by some other mechanism.
> > 
> > +1000. I'd personally like to see all one-off uses of -Werror removed.
> > 
> > > $ git grep -E "=\s*\-Werror" | grep -v CONFIG
> > > [...]
> > > arch/alpha/lib/Makefile:ccflags-y := -Werror
> > > arch/alpha/mm/Makefile:ccflags-y := -Werror
> > > arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
> > > arch/metag/oprofile/Makefile:ccflags-y	+= -Werror
> > > arch/mips/Kbuild:subdir-ccflags-y := -Werror
> > 
> > ^^ I always patch this one out when build-testing MIPS, since I like to
> > turn up warning levels (e.g., W=1), but not kill the build entirely.
> 
>  The MIPS port switched on -Werror years ago, because people submitted 
> awful code and couldn't be bothered unless the build crashed.  You're 
> welcome to patch your own tree, however I maintain it was a very good 
> decision, and TBH I think -Werror should be on globally.

That'd be easy to do with some CONFIG_COMPILER_ERROR type
rather than in specific directory Makefiles

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-01-30 17:37 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07 18:54 Build failure: -Wno-unused-const-variable DNE on old GCC Brian Norris
2016-01-07 19:37 ` Joe Perches
2016-01-07 19:44   ` Michal Marek
2016-01-07 19:57     ` Joe Perches
2016-01-07 20:18       ` Brian Norris
2016-01-07 20:38         ` [PATCH] misc: cxl: fix build for GCC 4.6.x Brian Norris
2016-01-08  2:12           ` Michael Ellerman
2016-01-30 14:20         ` Build failure: -Wno-unused-const-variable DNE on old GCC Maciej W. Rozycki
2016-01-30 17:37           ` Joe Perches
2016-01-07 20:25 ` Arnd Bergmann
2016-01-07 22:51   ` Daniel Axtens
2016-01-07 23:02     ` Brian Norris
2016-01-08  1:31       ` Ian Munsie
2016-01-08  2:07         ` Brian Norris
2016-01-08  2:16           ` Michael Ellerman
2016-01-08 10:14             ` David Laight
2016-01-08  1:33   ` Ian Munsie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).