All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Anvin <hpa@zytor.com>, Brian Gerst <brgerst@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Louis Langholtz <lou_langholtz@me.com>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare
Date: Tue, 12 May 2015 10:44:15 +0200	[thread overview]
Message-ID: <20150512084415.GA6535@gmail.com> (raw)
In-Reply-To: <CA+55aFxa5HHKJNdZrSg2XNY-im1-WyG=uLveqmZKYo_Zd=kosA@mail.gmail.com>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On May 11, 2015 05:44, "tip-bot for Ingo Molnar" <tipbot@zytor.com> wrote:
> >
> > So restore the warning and see what happens.
> 
> The gcc sign compare had been *totally* broken, I really don't want 
> to get a "let's see what happens" thing.
> 
> I do "allmodconfig" builds all the time, and actually check 
> warnings. [...]

Yeah, so what I failed to point out in the changelog is that I started 
doing that too a couple of weeks ago in my continuous integration 
kernel testing setup. There is a "baseline warnings count" gathered 
from your tree, so I can monitor changes in the level of warnings 
emitted:

 def64:    #   1, 8e70122d, Tue_May_12_09_38_25_CEST_2015:   0 kernels/hour, [ bzImage...    32 secs, modules...     4 secs, done ] (warns:   0)
 def32:    #   2, 8e70122d, Tue_May_12_09_38_47_CEST_2015: 156 kernels/hour, [ bzImage...    30 secs, modules...     3 secs, done ] (warns:   0)
 allno64:  #   3, 8e70122d, Tue_May_12_09_39_24_CEST_2015: 120 kernels/hour, [ bzImage...    22 secs, modules...     0 secs, done ] (warns:   2)
 allno32:  #   4, 8e70122d, Tue_May_12_09_39_58_CEST_2015: 114 kernels/hour, [ bzImage...    21 secs, modules...     0 secs, done ] (warns:   1)
 allyes64: #   5, 8e70122d, Tue_May_12_09_40_21_CEST_2015: 123 kernels/hour, [ bzImage...   204 secs, modules...     9 secs, done ] (warns:  10)
 allyes32: #   6, 8e70122d, Tue_May_12_09_40_44_CEST_2015: 128 kernels/hour, [ bzImage...   206 secs, modules...     9 secs, done ] (warns:  16)
 allmod64: #   7, 8e70122d, Tue_May_12_09_44_19_CEST_2015:  60 kernels/hour, [ bzImage...    62 secs, modules...   105 secs, done ] (warns:   6)
 allmod32: #   8, 8e70122d, Tue_May_12_09_47_56_CEST_2015:  44 kernels/hour, [ bzImage...    58 secs, modules...   103 secs, done ] (warns:  12)

the 'warns: 16' for example means that there are 16 warnings in that 
build. If a new warning is emitted by any of the trees I maintain, 
then I get a delta like this:

 def64:    #   1, 5e5f191d, Tue_May_12_08_55_14_CEST_2015:   0 kernels/hour, [ bzImage...    32 secs, modules...     5 secs, done ] (warns:   0)
 def32:    #   2, ed602bbb, Tue_May_12_08_55_39_CEST_2015: 138 kernels/hour, [ bzImage...    30 secs, modules...     3 secs, done ] (warns:   0)
 allno64:  #   3, ed602bbb, Tue_May_12_08_56_17_CEST_2015: 112 kernels/hour, [ bzImage...    22 secs, modules...     0 secs, done ] (warns:   2)
 allno32:  #   4, ed602bbb, Tue_May_12_08_56_51_CEST_2015: 110 kernels/hour, [ bzImage...    21 secs, modules...     0 secs, done ] (warns:   1)
 allyes64: #   5, ed602bbb, Tue_May_12_08_57_14_CEST_2015: 119 kernels/hour, [ bzImage...   199 secs, modules...     9 secs, done ] (warns:  11, delta: 1)
 allyes32: #   6, ed602bbb, Tue_May_12_08_57_37_CEST_2015: 125 kernels/hour, [ bzImage...   205 secs, modules...     9 secs, done ] (warns:  17, delta: 1)
 allmod64: #   7, ed602bbb, Tue_May_12_09_01_06_CEST_2015:  61 kernels/hour, [ bzImage...    60 secs, modules...   103 secs, done ] (warns:   7, delta: 1)
 allmod32: #   8, ed602bbb, Tue_May_12_09_04_41_CEST_2015:  44 kernels/hour, [ bzImage...    59 secs, modules...   105 secs, done ] (warns:  13, delta: 1)

(nicely color highligted so I cannot miss it and such.)

Btw., just some feedback, 'random' kernel configs still generate a ton 
of warnings:

 randconf: #   9, ed602bbb, Tue_May_12_09_07_25_CEST_2015:  39 kernels/hour, [ bzImage...    81 secs, modules...    21 secs, done ] (warns:   6)
 randconf: #  10, ed602bbb, Tue_May_12_09_10_10_CEST_2015:  36 kernels/hour, [ bzImage...    43 secs, modules...    20 secs, done ] (warns:  12)
 randconf: #  11, ed602bbb, Tue_May_12_09_11_52_CEST_2015:  36 kernels/hour, [ bzImage...    71 secs, modules...     0 secs, done ] (warns:   5)
 randconf: #  12, ed602bbb, Tue_May_12_09_12_56_CEST_2015:  37 kernels/hour, [ bzImage...    64 secs, modules...    28 secs, done ] (warns:  16)
 randconf: #  13, ed602bbb, Tue_May_12_09_14_07_CEST_2015:  38 kernels/hour, [ bzImage...   106 secs, modules...     0 secs, done ] (warns: 157)
 randconf: #  14, ed602bbb, Tue_May_12_09_15_40_CEST_2015:  38 kernels/hour, [ bzImage...   100 secs, modules...     0 secs, done ] (warns:  11)
 randconf: #  15, ed602bbb, Tue_May_12_09_17_27_CEST_2015:  37 kernels/hour, [ bzImage...    65 secs, modules...    28 secs, done ] (warns:  26)
 randconf: #  16, ed602bbb, Tue_May_12_09_19_08_CEST_2015:  37 kernels/hour, [ bzImage...    70 secs, modules...     0 secs, done ] (warns: 228)
 randconf: #  17, ed602bbb, Tue_May_12_09_20_42_CEST_2015:  37 kernels/hour, [ bzImage...    79 secs, modules...     0 secs, done ] (warns: 101)
 randconf: #  18, ed602bbb, Tue_May_12_09_21_53_CEST_2015:  38 kernels/hour, [ bzImage...    55 secs, modules...     0 secs, done ] (warns:   6)
 randconf: #  19, ed602bbb, Tue_May_12_09_23_13_CEST_2015:  38 kernels/hour, [ bzImage...    49 secs, modules...     0 secs, done ] (warns:   4)
 randconf: #  20, ed602bbb, Tue_May_12_09_24_08_CEST_2015:  39 kernels/hour, [ bzImage...    43 secs, modules... B  20 secs, done ] (warns:  28)
 randconf: #  21, ed602bbb, Tue_May_12_09_24_58_CEST_2015:  40 kernels/hour, [ bzImage... B  35 secs, modules...     0 secs, done ] (warns:  14)
 randconf: #  22, ed602bbb, Tue_May_12_09_26_02_CEST_2015:  40 kernels/hour, [ bzImage...    58 secs, modules...     0 secs, done ] (warns:  16)
 randconf: #  23, ed602bbb, Tue_May_12_09_26_38_CEST_2015:  42 kernels/hour, [ bzImage...    48 secs, modules... B  23 secs, done ] (warns:  35)
 randconf: #  24, ed602bbb, Tue_May_12_09_27_37_CEST_2015:  42 kernels/hour, [ bzImage...    40 secs, modules... B  15 secs, done ] (warns:  25)
 randconf: #  25, ed602bbb, Tue_May_12_09_28_49_CEST_2015:  42 kernels/hour, [ bzImage...    64 secs, modules...    19 secs, done ] (warns:   3)
 randconf: #  26, ed602bbb, Tue_May_12_09_29_45_CEST_2015:  43 kernels/hour, [ bzImage...    87 secs, modules...     0 secs, done ] (warns:  15)
 randconf: #  27, ed602bbb, Tue_May_12_09_31_09_CEST_2015:  43 kernels/hour, [ bzImage...    87 secs, modules...     0 secs, done ] (warns:   7)

(and 'B' marks known upstream build breakages.)

There are a few hotspots.

But in any case, your policy to police allmodconfig builds is a good 
starting point and has a positive effect.

Before I pushed out this -Wno-sign-compare change I made sure there 
are no extra warnings generated on the 8 key configs I monitor 
explicitly: [def|allno|allyes|allmod]config[64|32].

The "let's see what happens" in the changelog referred to the 
possibility that an older GCC version that what I use (4.9.2) emits so 
much crap that amounts to a regression and that I'll have to zap the 
commit. It did not refer to any random enabling of compiler warnings.

I absolutely failed at pointing out all this background work in the 
changelog though. Do you want me to rebase it to fix the changelog?

> [...] Right now I get something like six warnings, all from old 
> drivers that so dubious things, but that u can live with.

So the current warnings count is 0/0/2/1/10/16/6/12 on the main 
configs on your tree.

I also have a separate build system that uses GCC 5.0.1, there the 
warnings count is substantially higher:

 def64:    #   1, 5e5f191d, Tue_May_12_09_56_23_CEST_2015:   0 kernels/hour, [ bzImage...    47 secs, modules...     3 secs, done ] (warns:   4, delta: 4)
 def32:    #   2, 1e29025d, Tue_May_12_09_56_53_CEST_2015: 116 kernels/hour, [ bzImage...    46 secs, modules...     2 secs, done ] (warns:   4, delta: 4)
 allno64:  #   3, 1e29025d, Tue_May_12_09_57_44_CEST_2015:  87 kernels/hour, [ bzImage...    24 secs, modules...     0 secs, done ] (warns:   4, delta: 2)
 allno32:  #   4, 1e29025d, Tue_May_12_09_58_33_CEST_2015:  82 kernels/hour, [ bzImage...    24 secs, modules...     0 secs, done ] (warns:   3, delta: 2)
 allyes64: #   5, 1e29025d, Tue_May_12_09_58_58_CEST_2015:  92 kernels/hour, [ bzImage...   386 secs, modules...     9 secs, done ] (warns:  31, delta: 21)
 allyes32: #   6, 1e29025d, Tue_May_12_09_59_23_CEST_2015:  99 kernels/hour, [ bzImage...   344 secs, modules...    10 secs, done ] (warns:  36, delta: 20)
 allmod64: #   7, 1e29025d, Tue_May_12_10_05_59_CEST_2015:  37 kernels/hour, [ bzImage...    82 secs, modules...   250 secs, done ] (warns:  27, delta: 21)
 allmod32: #   8, 1e29025d, Tue_May_12_10_11_54_CEST_2015:  27 kernels/hour, [ bzImage...    75 secs, modules...   243 secs, done ] (warns:  32, delta: 20)

Here's the full list of warnings for allmod64:

make bzImage:

 include/linux/blkdev.h:624:26: warning: switch condition has boolean value [-Wswitch-bool]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:28:2: warning: implicit declaration of function ?pv_queued_spin_lock_slowpath? [-Wimplicit-function-declaration]
 ./arch/x86/include/asm/qspinlock.h:33:2: warning: implicit declaration of function ?pv_queued_spin_unlock? [-Wimplicit-function-declaration]

make modules:

 drivers/ata/pata_hpt366.c:376:9: warning: assignment discards ?const? qualifier from pointer target type [-Wdiscarded-array-qualifiers]
 drivers/ata/pata_hpt366.c:379:9: warning: assignment discards ?const? qualifier from pointer target type [-Wdiscarded-array-qualifiers]
 drivers/ata/pata_hpt366.c:382:9: warning: assignment discards ?const? qualifier from pointer target type [-Wdiscarded-array-qualifiers]
 drivers/hid/hid-input.c:1160:67: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/md/md.c:6375:26: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/gpu/drm/gma500/cdv_intel_dp.c:869:2: warning: ?i2c_dp_aux_add_bus? is deprecated [-Wdeprecated-declarations]
 drivers/mmc/host/sh_mmcif.c:401:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 drivers/mmc/host/sh_mmcif.c:402:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 drivers/isdn/hardware/eicon/message.c:6115:1: warning: the frame size of 2352 bytes is larger than 2048 bytes [-Wframe-larger-than=]
 drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not properly converted to the DMA API [-Wcpp]
 drivers/scsi/be2iscsi/be_main.c:3168:18: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 sound/pci/oxygen/oxygen_mixer.c:91:43: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/usb/renesas_usbhs/common.c:492:25: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 drivers/media/platform/s3c-camif/camif-capture.c:118:10: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/media/platform/s3c-camif/camif-capture.c:134:10: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
 drivers/net/wireless/brcm80211/brcmsmac/phy/phy_n.c:16065:1: warning: the frame size of 3200 bytes is larger than 2048 bytes [-Wframe-larger-than=]
 drivers/media/usb/cx231xx/cx231xx-cards.c:1110:1: warning: the frame size of 2064 bytes is larger than 2048 bytes [-Wframe-larger-than=]
 drivers/net/wireless/brcm80211/brcmsmac/phy/phy_n.c:17138:1: warning: the frame size of 2768 bytes is larger than 2048 bytes [-Wframe-larger-than=]

No new warnings were triggered by -Wno-sign-compare.

Six of the new warnings are generated due to -Wlogical-not-parentheses,
which looks like false positives in the cases of:

  drivers/hid/hid-input.c:1160
  drivers/md/md.c:6375

but it's actually pointing out what appears to be a real bug here:

  drivers/scsi/be2iscsi/be_main.c:3168

and I'm unsure about:

  sound/pci/oxygen/oxygen_mixer.c:91

but it sure looks weird. These are probably correct but are
looking weird:

  drivers/media/platform/s3c-camif/camif-capture.c:118
  drivers/media/platform/s3c-camif/camif-capture.c:134

and any kernel code that should be routine but makes reviewers
looks twice probably needs improving.

In any case, yes, I fully agree with your point and general policy 
that by default we cannot trust GCC's warnings, so I have tested this 
change as much as I could.

> Does this add to that? Because if it does, I'm not pulling the end 
> result.

Yeah, absolutely. I'll even zap the commit if it generates noise for 
older but still widely used GCC versions.

Thanks,

	Ingo

       reply	other threads:[~2015-05-12  8:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <tip-fd096a3bcd9718d979a7dd6253cd5b46fff66593@git.kernel.org>
     [not found] ` <CA+55aFxa5HHKJNdZrSg2XNY-im1-WyG=uLveqmZKYo_Zd=kosA@mail.gmail.com>
2015-05-12  8:44   ` Ingo Molnar [this message]
2015-05-12  8:53     ` [tip:x86/build] x86/build: Remove -Wno-sign-compare Ingo Molnar
2015-05-12 17:51       ` Waiman Long
2015-05-13  7:43     ` Valdis.Kletnieks
2015-05-13 10:22       ` Ingo Molnar
2015-05-13 17:33         ` Louis Langholtz
2015-05-13 17:50           ` Linus Torvalds

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=20150512084415.GA6535@gmail.com \
    --to=mingo@kernel.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lou_langholtz@me.com \
    --cc=luto@amacapital.net \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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.