From mboxrd@z Thu Jan 1 00:00:00 1970 From: Masahiro Yamada Subject: Re: [PATCH] kbuild: modpost: Warn about references from rodata to __init text Date: Sat, 22 Jul 2017 00:57:44 +0900 Message-ID: References: <20170630225822.25349-1-sboyd@codeaurora.org> <20170720175213.GB3291@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170720175213.GB3291@codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Stephen Boyd Cc: Linux Kbuild mailing list , linux-arm-msm , Linux Kernel Mailing List , Bjorn Andersson , Rob Clark , Michal Marek , "linux-arm-kernel@lists.infradead.org" List-Id: linux-arm-msm@vger.kernel.org Hi Stephen, 2017-07-21 2:52 GMT+09:00 Stephen Boyd : > On 07/20, Masahiro Yamada wrote: >> Hi Stephen, Rob, >> >> 2017-07-01 8:59 GMT+09:00 Rob Clark : >> > On Fri, Jun 30, 2017 at 6:58 PM, Stephen Boyd wrote: >> >> If we have a structure that's marked const it will be placed >> >> into the .rodata section but it could reference an init section >> >> function. Include the read only data section in the check we have >> >> for read/write data sections referencing init sections so we can >> >> find this class of problems. This exposes quite a few places >> >> where const marked structures are referencing __init functions and >> >> __init data that we were previously ignoring. >> >> >> >> Cc: Rob Clark >> >> Cc: Bjorn Andersson >> >> Signed-off-by: Stephen Boyd >> >> --- >> >> >> >> Making this change leads to quite a few other errors even on the >> >> multi_v7_defconfig for ARM[1]. I still need to do a build of the >> >> allmodconfig to see how many other errors there, but it seems to >> >> be quite a few. I suppose those will need to be fixed before we can >> >> merge this? >> > >> > thanks.. the explosions you get with these mistakes when building >> > drivers as modules in a distro kernel config are quite "fun" to >> > debug.. >> > >> > I'm not quite sure about the rules for whether merging this would >> > count as a regression, but I would argue those drivers are already >> > broken, just no one noticed yet. Similar to when a new gcc gets more >> > clever about detecting bugs. So I wouldn't be against merging this >> > first to force drivers to fix their crap ;-) >> >> I applied this, but this way seems unacceptable. >> I cannot send a pull-req for this >> unless most of the warnings are fixed. >> >> Is there any activity for driver fixes? > > Sorry, no. I've only had a little time to look at the errors > reported. Some of them seem to be noise, because there are > structures that have a handful of members where one of the > members points to an __init function or piece of data that is > only used in __init code. We would need to mark these structures > as __ref or write more complicated section mismatch checking code > to be silent in these cases. OK. I will drop this patch for now. -- Best Regards Masahiro Yamada From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754217AbdGUP7n (ORCPT ); Fri, 21 Jul 2017 11:59:43 -0400 Received: from conssluserg-02.nifty.com ([210.131.2.81]:57307 "EHLO conssluserg-02.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753750AbdGUP6H (ORCPT ); Fri, 21 Jul 2017 11:58:07 -0400 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-02.nifty.com v6LFvk1k031318 X-Nifty-SrcIP: [209.85.213.175] MIME-Version: 1.0 In-Reply-To: <20170720175213.GB3291@codeaurora.org> References: <20170630225822.25349-1-sboyd@codeaurora.org> <20170720175213.GB3291@codeaurora.org> From: Masahiro Yamada Date: Sat, 22 Jul 2017 00:57:44 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] kbuild: modpost: Warn about references from rodata to __init text To: Stephen Boyd Cc: Rob Clark , Michal Marek , Linux Kernel Mailing List , linux-arm-msm , "linux-arm-kernel@lists.infradead.org" , Linux Kbuild mailing list , Bjorn Andersson Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Stephen, 2017-07-21 2:52 GMT+09:00 Stephen Boyd : > On 07/20, Masahiro Yamada wrote: >> Hi Stephen, Rob, >> >> 2017-07-01 8:59 GMT+09:00 Rob Clark : >> > On Fri, Jun 30, 2017 at 6:58 PM, Stephen Boyd wrote: >> >> If we have a structure that's marked const it will be placed >> >> into the .rodata section but it could reference an init section >> >> function. Include the read only data section in the check we have >> >> for read/write data sections referencing init sections so we can >> >> find this class of problems. This exposes quite a few places >> >> where const marked structures are referencing __init functions and >> >> __init data that we were previously ignoring. >> >> >> >> Cc: Rob Clark >> >> Cc: Bjorn Andersson >> >> Signed-off-by: Stephen Boyd >> >> --- >> >> >> >> Making this change leads to quite a few other errors even on the >> >> multi_v7_defconfig for ARM[1]. I still need to do a build of the >> >> allmodconfig to see how many other errors there, but it seems to >> >> be quite a few. I suppose those will need to be fixed before we can >> >> merge this? >> > >> > thanks.. the explosions you get with these mistakes when building >> > drivers as modules in a distro kernel config are quite "fun" to >> > debug.. >> > >> > I'm not quite sure about the rules for whether merging this would >> > count as a regression, but I would argue those drivers are already >> > broken, just no one noticed yet. Similar to when a new gcc gets more >> > clever about detecting bugs. So I wouldn't be against merging this >> > first to force drivers to fix their crap ;-) >> >> I applied this, but this way seems unacceptable. >> I cannot send a pull-req for this >> unless most of the warnings are fixed. >> >> Is there any activity for driver fixes? > > Sorry, no. I've only had a little time to look at the errors > reported. Some of them seem to be noise, because there are > structures that have a handful of members where one of the > members points to an __init function or piece of data that is > only used in __init code. We would need to mark these structures > as __ref or write more complicated section mismatch checking code > to be silent in these cases. OK. I will drop this patch for now. -- Best Regards Masahiro Yamada From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from conssluserg-02.nifty.com ([210.131.2.81]:57307 "EHLO conssluserg-02.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753750AbdGUP6H (ORCPT ); Fri, 21 Jul 2017 11:58:07 -0400 MIME-Version: 1.0 In-Reply-To: <20170720175213.GB3291@codeaurora.org> References: <20170630225822.25349-1-sboyd@codeaurora.org> <20170720175213.GB3291@codeaurora.org> From: Masahiro Yamada Date: Sat, 22 Jul 2017 00:57:44 +0900 Message-ID: Subject: Re: [PATCH] kbuild: modpost: Warn about references from rodata to __init text Content-Type: text/plain; charset="UTF-8" Sender: linux-kbuild-owner@vger.kernel.org List-ID: To: Stephen Boyd Cc: Rob Clark , Michal Marek , Linux Kernel Mailing List , linux-arm-msm , "linux-arm-kernel@lists.infradead.org" , Linux Kbuild mailing list , Bjorn Andersson Hi Stephen, 2017-07-21 2:52 GMT+09:00 Stephen Boyd : > On 07/20, Masahiro Yamada wrote: >> Hi Stephen, Rob, >> >> 2017-07-01 8:59 GMT+09:00 Rob Clark : >> > On Fri, Jun 30, 2017 at 6:58 PM, Stephen Boyd wrote: >> >> If we have a structure that's marked const it will be placed >> >> into the .rodata section but it could reference an init section >> >> function. Include the read only data section in the check we have >> >> for read/write data sections referencing init sections so we can >> >> find this class of problems. This exposes quite a few places >> >> where const marked structures are referencing __init functions and >> >> __init data that we were previously ignoring. >> >> >> >> Cc: Rob Clark >> >> Cc: Bjorn Andersson >> >> Signed-off-by: Stephen Boyd >> >> --- >> >> >> >> Making this change leads to quite a few other errors even on the >> >> multi_v7_defconfig for ARM[1]. I still need to do a build of the >> >> allmodconfig to see how many other errors there, but it seems to >> >> be quite a few. I suppose those will need to be fixed before we can >> >> merge this? >> > >> > thanks.. the explosions you get with these mistakes when building >> > drivers as modules in a distro kernel config are quite "fun" to >> > debug.. >> > >> > I'm not quite sure about the rules for whether merging this would >> > count as a regression, but I would argue those drivers are already >> > broken, just no one noticed yet. Similar to when a new gcc gets more >> > clever about detecting bugs. So I wouldn't be against merging this >> > first to force drivers to fix their crap ;-) >> >> I applied this, but this way seems unacceptable. >> I cannot send a pull-req for this >> unless most of the warnings are fixed. >> >> Is there any activity for driver fixes? > > Sorry, no. I've only had a little time to look at the errors > reported. Some of them seem to be noise, because there are > structures that have a handful of members where one of the > members points to an __init function or piece of data that is > only used in __init code. We would need to mark these structures > as __ref or write more complicated section mismatch checking code > to be silent in these cases. OK. I will drop this patch for now. -- Best Regards Masahiro Yamada From mboxrd@z Thu Jan 1 00:00:00 1970 From: yamada.masahiro@socionext.com (Masahiro Yamada) Date: Sat, 22 Jul 2017 00:57:44 +0900 Subject: [PATCH] kbuild: modpost: Warn about references from rodata to __init text In-Reply-To: <20170720175213.GB3291@codeaurora.org> References: <20170630225822.25349-1-sboyd@codeaurora.org> <20170720175213.GB3291@codeaurora.org> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Stephen, 2017-07-21 2:52 GMT+09:00 Stephen Boyd : > On 07/20, Masahiro Yamada wrote: >> Hi Stephen, Rob, >> >> 2017-07-01 8:59 GMT+09:00 Rob Clark : >> > On Fri, Jun 30, 2017 at 6:58 PM, Stephen Boyd wrote: >> >> If we have a structure that's marked const it will be placed >> >> into the .rodata section but it could reference an init section >> >> function. Include the read only data section in the check we have >> >> for read/write data sections referencing init sections so we can >> >> find this class of problems. This exposes quite a few places >> >> where const marked structures are referencing __init functions and >> >> __init data that we were previously ignoring. >> >> >> >> Cc: Rob Clark >> >> Cc: Bjorn Andersson >> >> Signed-off-by: Stephen Boyd >> >> --- >> >> >> >> Making this change leads to quite a few other errors even on the >> >> multi_v7_defconfig for ARM[1]. I still need to do a build of the >> >> allmodconfig to see how many other errors there, but it seems to >> >> be quite a few. I suppose those will need to be fixed before we can >> >> merge this? >> > >> > thanks.. the explosions you get with these mistakes when building >> > drivers as modules in a distro kernel config are quite "fun" to >> > debug.. >> > >> > I'm not quite sure about the rules for whether merging this would >> > count as a regression, but I would argue those drivers are already >> > broken, just no one noticed yet. Similar to when a new gcc gets more >> > clever about detecting bugs. So I wouldn't be against merging this >> > first to force drivers to fix their crap ;-) >> >> I applied this, but this way seems unacceptable. >> I cannot send a pull-req for this >> unless most of the warnings are fixed. >> >> Is there any activity for driver fixes? > > Sorry, no. I've only had a little time to look at the errors > reported. Some of them seem to be noise, because there are > structures that have a handful of members where one of the > members points to an __init function or piece of data that is > only used in __init code. We would need to mark these structures > as __ref or write more complicated section mismatch checking code > to be silent in these cases. OK. I will drop this patch for now. -- Best Regards Masahiro Yamada