Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932478AbbELIo0 (ORCPT ); Tue, 12 May 2015 04:44:26 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:33292 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932190AbbELIoV (ORCPT ); Tue, 12 May 2015 04:44:21 -0400 Date: Tue, 12 May 2015 10:44:15 +0200 From: Ingo Molnar To: Linus Torvalds Cc: Peter Anvin , Brian Gerst , Thomas Gleixner , Borislav Petkov , Peter Zijlstra , Andy Lutomirski , Louis Langholtz , Denys Vlasenko , linux-kernel@vger.kernel.org Subject: Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare Message-ID: <20150512084415.GA6535@gmail.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13546 Lines: 181 * Linus Torvalds wrote: > On May 11, 2015 05:44, "tip-bot for Ingo Molnar" 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/