From: Russell King - ARM Linux admin <linux@armlinux.org.uk> To: Arnd Bergmann <arnd@arndb.de> Cc: Sudeep Holla <sudeep.holla@arm.com>, Mark Rutland <mark.rutland@arm.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Steven Price <steven.price@arm.com>, harb@amperecomputing.com, Will Deacon <will@kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v4 0/7] firmware: smccc: Add basic SMCCC v1.2 + ARCH_SOC_ID support Date: Thu, 21 May 2020 12:46:07 +0100 [thread overview] Message-ID: <20200521114607.GP1551@shell.armlinux.org.uk> (raw) In-Reply-To: <CAK8P3a3cPPiprEpF_k-GWAgWSZiP3Qp3v++jvS_8W17Ns4_HGw@mail.gmail.com> On Thu, May 21, 2020 at 12:31:32PM +0200, Arnd Bergmann wrote: > On Thu, May 21, 2020 at 12:14 PM Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > > > On Thu, May 21, 2020 at 11:06:23AM +0200, Arnd Bergmann wrote: > > > Note that the warning should come up for either W=1 or C=1, and I also > > > think that > > > new code should generally be written sparse-clean and have no warnings with > > > 'make C=1' as a rule. > > > > No, absolutely not, that's a stupid idea, there are corner cases > > where hiding a sparse warning is the wrong thing to do. Look at > > many of the cases in fs/ for example. > > > > See https://lkml.org/lkml/2004/9/12/249 which should make anyone > > who sees a use of __force in some random code stop and question > > why it is there, and whether it is actually correct, or just there > > to hide a sparse warning. > > > > Remember, sparse is there to warn that something isn't quite right, > > and the view taken is, if it isn't right, then we don't "cast the > > warning away" with __force, even if we intend not to fix the code > > immediately. > > > > So, going for "sparse-clean" is actually not correct. Going for > > "no unnecessary warnings" is. > > > > And don't think what I've said above doesn't happen; I've rejected > > patches from people who've gone around trying to fix every sparse > > warning that they see by throwing __force incorrectly at it. > > > > The thing is, if you hide all the warnings, even for incorrect code, > > then sparse becomes completely useless to identify where things in > > the code are not quite correct. > > Adding __force is almost always the wrong solution, and I explictly > was not talking about existing code here where changing it would > risk introducing bugs or require bad hacks. I'm using existing code to illustrate the problem with your idea of "sparse-clean" new code, trying to show you that it is not about being sparse clean, but about being correct. > However, when writing a new driver, sparse warnings usually > indicate that you are doing something wrong that is better addressed > by doing something different that does not involve adding __force. Right, but if you lay down a rule that says "new submissions must be sparse clean" you will get people using __force to shut sparse up. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux admin <linux@armlinux.org.uk> To: Arnd Bergmann <arnd@arndb.de> Cc: Mark Rutland <mark.rutland@arm.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Steven Price <steven.price@arm.com>, harb@amperecomputing.com, Sudeep Holla <sudeep.holla@arm.com>, Will Deacon <will@kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v4 0/7] firmware: smccc: Add basic SMCCC v1.2 + ARCH_SOC_ID support Date: Thu, 21 May 2020 12:46:07 +0100 [thread overview] Message-ID: <20200521114607.GP1551@shell.armlinux.org.uk> (raw) In-Reply-To: <CAK8P3a3cPPiprEpF_k-GWAgWSZiP3Qp3v++jvS_8W17Ns4_HGw@mail.gmail.com> On Thu, May 21, 2020 at 12:31:32PM +0200, Arnd Bergmann wrote: > On Thu, May 21, 2020 at 12:14 PM Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > > > On Thu, May 21, 2020 at 11:06:23AM +0200, Arnd Bergmann wrote: > > > Note that the warning should come up for either W=1 or C=1, and I also > > > think that > > > new code should generally be written sparse-clean and have no warnings with > > > 'make C=1' as a rule. > > > > No, absolutely not, that's a stupid idea, there are corner cases > > where hiding a sparse warning is the wrong thing to do. Look at > > many of the cases in fs/ for example. > > > > See https://lkml.org/lkml/2004/9/12/249 which should make anyone > > who sees a use of __force in some random code stop and question > > why it is there, and whether it is actually correct, or just there > > to hide a sparse warning. > > > > Remember, sparse is there to warn that something isn't quite right, > > and the view taken is, if it isn't right, then we don't "cast the > > warning away" with __force, even if we intend not to fix the code > > immediately. > > > > So, going for "sparse-clean" is actually not correct. Going for > > "no unnecessary warnings" is. > > > > And don't think what I've said above doesn't happen; I've rejected > > patches from people who've gone around trying to fix every sparse > > warning that they see by throwing __force incorrectly at it. > > > > The thing is, if you hide all the warnings, even for incorrect code, > > then sparse becomes completely useless to identify where things in > > the code are not quite correct. > > Adding __force is almost always the wrong solution, and I explictly > was not talking about existing code here where changing it would > risk introducing bugs or require bad hacks. I'm using existing code to illustrate the problem with your idea of "sparse-clean" new code, trying to show you that it is not about being sparse clean, but about being correct. > However, when writing a new driver, sparse warnings usually > indicate that you are doing something wrong that is better addressed > by doing something different that does not involve adding __force. Right, but if you lay down a rule that says "new submissions must be sparse clean" you will get people using __force to shut sparse up. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-21 11:46 UTC|newest] Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-18 9:12 [PATCH v4 0/7] firmware: smccc: Add basic SMCCC v1.2 + ARCH_SOC_ID support Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:12 ` [PATCH v4 1/7] firmware: smccc: Add HAVE_ARM_SMCCC_DISCOVERY to identify SMCCC v1.1 and above Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:12 ` [PATCH v4 2/7] firmware: smccc: Update link to latest SMCCC specification Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:12 ` [PATCH v4 3/7] firmware: smccc: Add the definition for SMCCCv1.2 version/error codes Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:12 ` [PATCH v4 4/7] firmware: smccc: Drop smccc_version enum and use ARM_SMCCC_VERSION_1_x instead Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:12 ` [PATCH v4 5/7] firmware: smccc: Refactor SMCCC specific bits into separate file Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-19 12:00 ` kbuild test robot 2020-05-20 10:29 ` kbuild test robot 2020-05-18 9:12 ` [PATCH v4 6/7] firmware: smccc: Add function to fetch SMCCC version Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:12 ` [PATCH v4 7/7] firmware: smccc: Add ARCH_SOC_ID support Sudeep Holla 2020-05-18 9:12 ` Sudeep Holla 2020-05-18 9:30 ` Arnd Bergmann 2020-05-18 9:30 ` Arnd Bergmann 2020-05-18 11:55 ` Sudeep Holla 2020-05-18 11:55 ` Sudeep Holla 2020-05-20 21:51 ` Arnd Bergmann 2020-05-20 21:51 ` Arnd Bergmann 2020-05-21 7:07 ` Sudeep Holla 2020-05-21 7:07 ` Sudeep Holla 2020-05-20 21:29 ` [PATCH v4 0/7] firmware: smccc: Add basic SMCCC v1.2 + " Will Deacon 2020-05-20 21:29 ` Will Deacon 2020-05-20 21:54 ` Arnd Bergmann 2020-05-20 21:54 ` Arnd Bergmann 2020-05-21 7:07 ` Sudeep Holla 2020-05-21 7:07 ` Sudeep Holla 2020-05-21 7:34 ` Arnd Bergmann 2020-05-21 7:34 ` Arnd Bergmann 2020-05-21 7:57 ` Will Deacon 2020-05-21 7:57 ` Will Deacon 2020-05-21 8:10 ` Sudeep Holla 2020-05-21 8:10 ` Sudeep Holla 2020-05-21 9:06 ` Arnd Bergmann 2020-05-21 9:06 ` Arnd Bergmann 2020-05-21 9:15 ` Sudeep Holla 2020-05-21 9:15 ` Sudeep Holla 2020-05-21 9:17 ` Will Deacon 2020-05-21 9:17 ` Will Deacon 2020-05-21 9:26 ` Sudeep Holla 2020-05-21 9:26 ` Sudeep Holla 2020-05-21 10:14 ` Will Deacon 2020-05-21 10:14 ` Will Deacon 2020-05-21 10:24 ` Sudeep Holla 2020-05-21 10:24 ` Sudeep Holla 2020-05-21 9:30 ` Arnd Bergmann 2020-05-21 9:30 ` Arnd Bergmann 2020-05-21 10:14 ` Russell King - ARM Linux admin 2020-05-21 10:14 ` Russell King - ARM Linux admin 2020-05-21 10:31 ` Arnd Bergmann 2020-05-21 10:31 ` Arnd Bergmann 2020-05-21 11:46 ` Russell King - ARM Linux admin [this message] 2020-05-21 11:46 ` Russell King - ARM Linux admin 2020-05-21 8:05 ` Sudeep Holla 2020-05-21 8:05 ` Sudeep Holla
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=20200521114607.GP1551@shell.armlinux.org.uk \ --to=linux@armlinux.org.uk \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=harb@amperecomputing.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mark.rutland@arm.com \ --cc=steven.price@arm.com \ --cc=sudeep.holla@arm.com \ --cc=will@kernel.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: linkBe 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.