From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751804AbcEJNCe (ORCPT ); Tue, 10 May 2016 09:02:34 -0400 Received: from smtpoutz298.laposte.net ([178.22.154.198]:40547 "EHLO smtp.laposte.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751544AbcEJNCc (ORCPT ); Tue, 10 May 2016 09:02:32 -0400 Message-ID: <5731DBE5.4090308@laposte.net> Date: Tue, 10 May 2016 15:02:29 +0200 From: Sebastian Frias User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Andy Whitcroft CC: Joe Perches , mason , LKML Subject: Re: Fwd: [PATCH] mm: add config option to select the initial overcommit mode References: <5731CC6E.3080807@laposte.net> <5731CD6F.9070101@laposte.net> <20160510123941.GB11368@brain> In-Reply-To: <20160510123941.GB11368@brain> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-VR-SrcIP: 83.142.147.193 X-VR-FullState: 0 X-VR-Score: 0 X-VR-Cause-1: gggruggvucftvghtrhhoucdtuddrfeekledrudeggdeitdcutefuodetggdotefrodftvfcurfhrohhf X-VR-Cause-2: ihhlvgemucfntefrqffuvffgnecuuegrihhlohhuthemucehtddtnecunecujfgurhepkfffhfgfggfv X-VR-Cause-3: ufhfjggtgfesthejrgdttdefheenucfhrhhomhepufgvsggrshhtihgrnhcuhfhrihgrshcuoehsfhek X-VR-Cause-4: geeslhgrphhoshhtvgdrnhgvtheqnecuffhomhgrihhnpehsvggtthhiohhnrddqqddqmhhmnecukfhp X-VR-Cause-5: peekfedrudegvddrudegjedrudelfeenucfrrghrrghmpehmohguvgepshhmthhpohhuthdphhgvlhho X-VR-Cause-6: pegludejvddrvdejrddtrddvudegngdpihhnvghtpeekfedrudegvddrudegjedrudelfedpmhgrihhl X-VR-Cause-7: fhhrohhmpehsfhekgeeslhgrphhoshhtvgdrnhgvthdprhgtphhtthhopegrphifsegtrghnohhnihgt X-VR-Cause-8: rghlrdgtohhm X-VR-AvState: No X-VR-State: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andy, On 05/10/2016 02:39 PM, Andy Whitcroft wrote: > On Tue, May 10, 2016 at 02:00:47PM +0200, Sebastian Frias wrote: >> Hi, >> >> Using checkpatch.pl on the forwarded patch results in: >> >> WARNING: please write a paragraph that describes the config symbol fully >> #57: FILE: mm/Kconfig:451: >> + config OVERCOMMIT_GUESS >> >> WARNING: please write a paragraph that describes the config symbol fully >> #64: FILE: mm/Kconfig:458: >> + config OVERCOMMIT_ALWAYS >> >> but there is a 'help' section for those 'config' sections. >> NOTE: I followed the same indentation than the code laying just above the place where I inserted mine. >> >> I think it is a false positive, what do you think? >> >> Best regards, >> >> Sebastian > > Well, I am expecting the issue to be that the per option help is not > indented within the option like I am expecting ... Changing the indentation does not solve the issue. Marc (in CC) just told me he had had the same issue and it was related to having less than 4 lines of 'help'. If I add a dummy line to the 'help' section the warning goes indeed away. Also notice that 'config OVERCOMMIT_NEVER' is not giving warnings even if its 'help' section has also 3 lines. > >> Date: Tue, 10 May 2016 13:56:30 +0200 >> From: Sebastian Frias >> To: linux-mm@kvack.org, Andrew Morton , Michal >> Hocko , Linus Torvalds >> CC: LKML , mason >> Subject: [PATCH] mm: add config option to select the initial overcommit mode >> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 >> Thunderbird/31.2.0 >> >> Currently the initial value of the overcommit mode is OVERCOMMIT_GUESS. >> However, on embedded systems it is usually better to disable overcommit >> to avoid waking up the OOM-killer and its well known undesirable >> side-effects. >> >> This config option allows to setup the initial overcommit mode to any of >> the 3 available values, OVERCOMMIT_GUESS (which remains as default), >> OVERCOMMIT_ALWAYS and OVERCOMMIT_NEVER. >> The overcommit mode can still be changed thru sysctl after the system >> boots up. >> >> This config option depends on CONFIG_EXPERT. >> This patch does not introduces functional changes. >> >> Signed-off-by: Sebastian Frias >> --- >> >> NOTE: I understand that the overcommit mode can be changed dynamically thru >> sysctl, but on embedded systems, where we know in advance that overcommit >> will be disabled, there's no reason to postpone such setting. >> >> I would also be interested in knowing if you guys think this option should >> disable sysctl access for overcommit mode, essentially hardcoding the >> overcommit mode when this option is used. >> >> NOTE2: I tried to track down the history of overcommit but back then there >> were no single patches apparently and the patch that appears to have >> introduced the first overcommit mode (OVERCOMMIT_ALWAYS) is commit >> 9334eab8a36f ("Import 2.1.27"). OVERCOMMIT_NEVER was introduced with commit >> 502bff0685b2 ("[PATCH] strict overcommit"). >> My understanding is that prior to commit 9334eab8a36f ("Import 2.1.27") >> there was no overcommit, is that correct? >> >> NOTE3: checkpatch.pl is warning about missing description for the config >> symbols ("please write a paragraph that describes the config symbol fully") >> but my understanding is that that is a false positive (or the warning message >> not clear enough for me to understand it) considering that I have added >> 'help' sections for each 'config' section. >> --- >> mm/Kconfig | 32 ++++++++++++++++++++++++++++++++ >> mm/util.c | 8 +++++++- >> 2 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/mm/Kconfig b/mm/Kconfig >> index abb7dcf..6dad57d 100644 >> --- a/mm/Kconfig >> +++ b/mm/Kconfig >> @@ -439,6 +439,38 @@ choice >> benefit. >> endchoice >> >> +choice >> + prompt "Overcommit Mode" >> + default OVERCOMMIT_GUESS >> + depends on EXPERT >> + help >> + Selects the initial value for Overcommit mode. >> + >> + NOTE: The overcommit mode can be changed dynamically through sysctl. >> + >> + config OVERCOMMIT_GUESS >> + bool "Guess" > > I am expecting the help below to be indented at the same level as the > bool above. As you have done with the help for the choice itself. I am > pretty sure checkpatch is assuming the "contents" of the config item are > all intented more than it is. > >> + help >> + Selecting this option forces the initial value of overcommit mode to >> + "Guess" overcommits. This is the default value. >> + See Documentation/vm/overcommit-accounting for more information. > [...] > > -apw >