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
next parent 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.