Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1398024 > unrolled thread

[PATCH] mm: add config option to select the initial overcommit mode

Back to article view | Back to linux.kernel

Contents

  [PATCH] mm: add config option to select the initial overcommit mode Sebastian Frias <sf84@laposte.net> - 2016-05-10 14:00 +0200
    Fwd: [PATCH] mm: add config option to select the initial overcommit  mode Sebastian Frias <sf84@laposte.net> - 2016-05-10 14:10 +0200
      Re: Fwd: [PATCH] mm: add config option to select the initial  overcommit mode Andy Whitcroft <apw@canonical.com> - 2016-05-10 14:40 +0200
        Re: Fwd: [PATCH] mm: add config option to select the initial overcommit  mode Sebastian Frias <sf84@laposte.net> - 2016-05-10 15:10 +0200

#1398024 — [PATCH] mm: add config option to select the initial overcommit mode

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 <sf84@laposte.net>
---

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"
+	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.
+
+	config OVERCOMMIT_ALWAYS
+		bool "Always"
+	help
+	  Selecting this option forces the initial value of overcommit mode to
+	  "Always" overcommit.
+	  See Documentation/vm/overcommit-accounting for more information.
+
+	config OVERCOMMIT_NEVER
+		bool "Never"
+	help
+	  Selecting this option forces the initial value of overcommit mode to
+	  "Never" overcommit.
+	  See Documentation/vm/overcommit-accounting for more information.
+
+endchoice
+
 #
 # UP and nommu archs use km based percpu allocator
 #
diff --git a/mm/util.c b/mm/util.c
index 917e0e3..fd098bb 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -418,7 +418,13 @@ int __page_mapcount(struct page *page)
 }
 EXPORT_SYMBOL_GPL(__page_mapcount);
 
-int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
+#if defined(CONFIG_OVERCOMMIT_NEVER)
+int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_NEVER;
+#elif defined(CONFIG_OVERCOMMIT_ALWAYS)
+int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_ALWAYS;
+#else
+int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
+#endif
 int sysctl_overcommit_ratio __read_mostly = 50;
 unsigned long sysctl_overcommit_kbytes __read_mostly;
 int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
-- 
2.1.4

[toc] | [next] | [standalone]

#1398030 — Fwd: [PATCH] mm: add config option to select the initial overcommit mode

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1398048 — Re: Fwd: [PATCH] mm: add config option to select the initial overcommit mode

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

> Date: Tue, 10 May 2016 13:56:30 +0200
> From: Sebastian Frias <sf84@laposte.net>
> To: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, Michal
>  Hocko <mhocko@suse.com>, Linus Torvalds <torvalds@linux-foundation.org>
> CC: LKML <linux-kernel@vger.kernel.org>, mason <slash.tmp@free.fr>
> 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 <sf84@laposte.net>
> ---
> 
> 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

[toc] | [prev] | [next] | [standalone]

#1398074 — Re: Fwd: [PATCH] mm: add config option to select the initial overcommit mode

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 <sf84@laposte.net>
>> To: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, Michal
>>  Hocko <mhocko@suse.com>, Linus Torvalds <torvalds@linux-foundation.org>
>> CC: LKML <linux-kernel@vger.kernel.org>, mason <slash.tmp@free.fr>
>> 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 <sf84@laposte.net>
>> ---
>>
>> 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
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web