Skip to contentSkip to conversation

parisc,metag: Do not hardcode maximum userspace stack size

PATCH In review Helge Deller 4 participants 12 messages Apr 30, 2014 – May 13, 2014

Subsystems (5)
lore
Share & tools
lore

Helge Deller

author maintainer
#0 lore raw

PATCH 1/1 parisc,metag: Do not hardcode maximum userspace stack size

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

arch/parisc/kernel/sys_parisc.c +3 -3
index 7d8cbd1..9118f01 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +3 -3
index 476f3eb..994108c 100644
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
init/Kconfig +11 -0
index 9d3585b..436e479 100644
@@ -1862,6 +1862,17 @@ config STOP_MACHINE
 	help
 	  Need stop_machine() primitive.
 
+config MAX_STACK_SIZE_MB
+	int "Default initial maximum stack size"
+	default 80
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the default initial stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and matag
+	  arch).  The stack will be located at the highest memory address minus the
+	  given value. A sane initial value is 80 MB.
+
 source "block/Kconfig"
 
 config PREEMPT_NOTIFIERS

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
In reply to Helge Deller

Hi Helge,

Quoted 42 lines · Helge Deller

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

arch/parisc/kernel/sys_parisc.c +3 -3
index 7d8cbd1..9118f01 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +0 -0
index 476f3eb..994108c 100644

fs/exec.c int setup_arg_pages(struct linux_binprm *bprm, · line 663

Quoted 10 lines
fs/exec.c +3 -3
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;

When I remove metag's _STK_LIM_MAX override (before your patch) it panics when I next start a process (since stack_top = 0x3ffff000 so the 1GB default is way too big).

That could actually always have been triggered even with the default _STK_LIM_MAX override, by just changing it from userland (as root), e.g.:

# ulimit -H -s unlimited
# cat
BUG: failure at fs/exec.c:589/shift_arg_pages()!
Kernel panic - not syncing: BUG!

I'm guessing this doesn't affect parisc due to stack_top being above 1GB, but since this patch effectively fixes a bug on metag (by changing the maximum stack size to a smaller/safe value) I'd like to take this patch and submit upstream for v3.15, and mark for stable. Would that be okay with you?

A few suggestions below though...

Quoted 8 lines
fs/exec.c +0 -0
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
init/Kconfig +0 -0
index 9d3585b..436e479 100644

init/Kconfig config STOP_MACHINE · line 1867

Quoted 6 lines
init/Kconfig +3 -0
@@ -1862,6 +1862,17 @@ config STOP_MACHINE
 	help
 	  Need stop_machine() primitive.
 
+config MAX_STACK_SIZE_MB
+	int "Default initial maximum stack size"
+	default 80

can we insert here:
range 8 256 if METAG

init/Kconfig config STOP_MACHINE · line 1874

Quoted 7 lines
init/Kconfig +7 -0
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the default initial stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and matag
+	  arch).  The stack will be located at the highest memory address minus the
+	  given value. A sane initial value is 80 MB.

This config option appears in the root menu. Can we move it into a submenu, e.g. mm/Kconfig would seem a good place for it, then it appears in the "Processor type and features" menu.

Also, technically it's the absolute maximum stack size, which happens to be the default unless the user reduces the RLIMIT_STACK hard limit.

How does the v2 below look?

From c34f0ec062ae1a2c9fca3eddbc705f6b0faf97ca Mon Sep 17 00:00:00 2001
From: Helge Deller <[email protected]>
Date: Wed, 30 Apr 2014 23:26:02 +0200
Subject: [PATCH v2] parisc,metag: Do not hardcode maximum userspace stack
size

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

This also fixes a BUG on metag if the RLIMIT_STACK hard limit is increased beyond a safe value by root. E.g. when starting a process after running "ulimit -H -s unlimited" it will then attempt to use a stack size of the maximum 1GB which is far too big for metag's limited user virtual address space (stack_top is usually 0x3ffff000):

BUG: failure at fs/exec.c:589/shift_arg_pages()!

---
v2 (James Hogan):
- updated description to mention BUG on metag.
- added custom range limit for METAG.
- moved Kconfig symbol to mm/Kconfig and reworded.
- fixed "matag" typo.

---
 arch/parisc/kernel/sys_parisc.c |  6 +++---
 fs/exec.c                       |  6 +++---
 mm/Kconfig                      | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 6 deletions(-)
arch/parisc/kernel/sys_parisc.c +3 -3
index 31ffa9b55322..9f040261151e 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +3 -3
index 476f3ebf437e..994108cc60f3 100644
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
mm/Kconfig +15 -0
index ebe5880c29d6..e80075979530 100644
@@ -581,3 +581,18 @@ config PGTABLE_MAPPING
 
 config GENERIC_EARLY_IOREMAP
 	bool
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.
Signature · 1 line
1.9.2
maintainer
#3 lore raw
Quoted 6 lines
mm/Kconfig +5 -0
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG

mm/Kconfig config PGTABLE_MAPPING · line 598

Quoted 10 lines
mm/Kconfig +10 -0
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Also, even on 32 bits, I can't help feeling we could simply layout the binary better ... the problem is we have three upward growing regions:
stack, maps and heap. However, if you look at the current standard elf
layout for downward growing stacks, the maps grow up from the bottom until it hits the mapped binary, the heap grows up from the mapped binary and the stack grows down from the top. You run out of memory when the stack and heap cross or when the maps hits the binary. Obviously with three upwardly growing regions, it's problematic, but we could do something like make the maps grow down (can't, unfortunately, make the heap grow down since sbrk depends on the upward behaviour).

James

#4 lore raw
Quoted 13 lines · James Bottomley
Quoted 13 lines
(quoted diff) +12 -0
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Does the following fixup diff look reasonable? It forces MAX_STACK_SIZE_MB to 1024 and hides the Kconfig option for 64BIT, effectively leaving the behaviour unchanged in that case.

mm/Kconfig +2 -1
index e80075979530..b0307f737bd7 100644
@@ -583,7 +583,8 @@ config GENERIC_EARLY_IOREMAP
 	bool
 
 config MAX_STACK_SIZE_MB
-	int "Maximum user stack size (MB)"
+	int "Maximum user stack size (MB)" if !64BIT
+	default 1024 if 64BIT
 	default 80
 	range 8 256 if METAG
 	range 8 2048

Thanks
James

maintainer
#5 lore raw
Quoted 32 lines · James Hogan
Quoted 13 lines · James Bottomley
Quoted 13 lines
(quoted diff) +12 -0
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Does the following fixup diff look reasonable? It forces MAX_STACK_SIZE_MB to 1024 and hides the Kconfig option for 64BIT, effectively leaving the behaviour unchanged in that case.

mm/Kconfig +0 -0
index e80075979530..b0307f737bd7 100644

mm/Kconfig config GENERIC_EARLY_IOREMAP · line 590

Quoted 9 lines
mm/Kconfig +2 -1
@@ -583,7 +583,8 @@ config GENERIC_EARLY_IOREMAP
 	bool
 
 config MAX_STACK_SIZE_MB
-	int "Maximum user stack size (MB)"
+	int "Maximum user stack size (MB)" if !64BIT
+	default 1024 if 64BIT
 	default 80
 	range 8 256 if METAG
 	range 8 2048

Yes, I think that's probably correct ... parisc doesn't actually have anything other than a testbed 64 bit userspace, so this is a bit theoretical for us.

James

author maintainer
#6 lore raw
Quoted 35 lines · James Bottomley
Quoted 34 lines · James Hogan
Quoted 13 lines · James Bottomley
Quoted 13 lines
(quoted diff) +12 -0
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Does the following fixup diff look reasonable? It forces MAX_STACK_SIZE_MB to 1024 and hides the Kconfig option for 64BIT, effectively leaving the behaviour unchanged in that case.

mm/Kconfig +0 -0
index e80075979530..b0307f737bd7 100644
@@ -583,7 +583,8 @@ config GENERIC_EARLY_IOREMAP
 	bool
 
Quoted 9 lines

mm/Kconfig config GENERIC_EARLY_IOREMAP · line 590

Quoted 7 lines
mm/Kconfig +2 -1
 config MAX_STACK_SIZE_MB
-	int "Maximum user stack size (MB)"
+	int "Maximum user stack size (MB)" if !64BIT
+	default 1024 if 64BIT
 	default 80
 	range 8 256 if METAG
 	range 8 2048

Yes, I think that's probably correct ...

No, it's not correct.
It will then choose then a 1GB stack for compat tasks on 64bit kernel.

Helge

Quoted 3 lines

parisc doesn't actually have
anything other than a testbed 64 bit userspace, so this is a bit theoretical for us.

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

Hi Helge,

Quoted 46 lines · Helge Deller
Quoted 35 lines · James Bottomley
Quoted 34 lines · James Hogan
Quoted 13 lines · James Bottomley
Quoted 13 lines
(quoted diff) +12 -0
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Does the following fixup diff look reasonable? It forces MAX_STACK_SIZE_MB to 1024 and hides the Kconfig option for 64BIT, effectively leaving the behaviour unchanged in that case.

mm/Kconfig +0 -0
index e80075979530..b0307f737bd7 100644
@@ -583,7 +583,8 @@ config GENERIC_EARLY_IOREMAP
 	bool
 
Quoted 9 lines

mm/Kconfig config GENERIC_EARLY_IOREMAP · line 590

Quoted 7 lines
mm/Kconfig +2 -1
 config MAX_STACK_SIZE_MB
-	int "Maximum user stack size (MB)"
+	int "Maximum user stack size (MB)" if !64BIT
+	default 1024 if 64BIT
 	default 80
 	range 8 256 if METAG
 	range 8 2048

Yes, I think that's probably correct ...

Quoted 2 lines

No, it's not correct.
It will then choose then a 1GB stack for compat tasks on 64bit kernel.

Sorry for the delay (I had most of last week off sick and still catching up).

That's a good point. It makes me think the best way to handle it is in a new definition in asm/processor.h, maybe STACK_SIZE_MAX. Does something like this look better? This patch isn't getting any cleaner unfortunately.

From 6ecb0392a3b670c4bf1641a6ec56f22427ca8b57 Mon Sep 17 00:00:00 2001
From: Helge Deller <[email protected]>
Date: Wed, 30 Apr 2014 23:26:02 +0200
Subject: [PATCH 1/1] parisc,metag: Do not hardcode maximum userspace stack
size

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB for 32-bit processes, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

This also fixes a BUG on metag if the RLIMIT_STACK hard limit is increased beyond a safe value by root. E.g. when starting a process after running "ulimit -H -s unlimited" it will then attempt to use a stack size of the maximum 1GB which is far too big for metag's limited user virtual address space (stack_top is usually 0x3ffff000):

BUG: failure at fs/exec.c:589/shift_arg_pages()!
Cc John David Anglin <[email protected]>
Cc [email protected] # only needed for >= v3.9 (arch/metag)

---
v3 (James Hogan):
- fix so that 64-bit parisc processes still use the 1GB limit. CONFIG_STACK_GROWSUP arches should provide a STACK_SIZE_MAX in their asm/processor.h, and for parisc it depends on USER_WIDE_MODE (whether the current process is 64-bit).
v2 (James Hogan):
- updated description to mention BUG on metag.
- added custom range limit for METAG.
- moved Kconfig symbol to mm/Kconfig and reworded.
- fixed "matag" typo.

---
 arch/metag/include/asm/processor.h  |  2 ++
 arch/parisc/include/asm/processor.h |  5 +++++
 arch/parisc/kernel/sys_parisc.c     |  6 +++---
 fs/exec.c                           |  6 +++---
 mm/Kconfig                          | 15 +++++++++++++++
 5 files changed, 28 insertions(+), 6 deletions(-)
arch/metag/include/asm/processor.h +2 -0
index f16477d1f571..a8a37477c66e 100644
@@ -22,6 +22,8 @@
 /* Add an extra page of padding at the top of the stack for the guard page. */
 #define STACK_TOP	(TASK_SIZE - PAGE_SIZE)
 #define STACK_TOP_MAX	STACK_TOP
+/* Maximum virtual space for stack */
+#define STACK_SIZE_MAX	(CONFIG_MAX_STACK_SIZE_MB*1024*1024)
 
 /* This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
arch/parisc/include/asm/processor.h +5 -0
index 198a86feb574..d951c9681ab3 100644
@@ -55,6 +55,11 @@
 #define STACK_TOP	TASK_SIZE
 #define STACK_TOP_MAX	DEFAULT_TASK_SIZE
 
+/* Allow bigger stacks for 64-bit processes */
+#define STACK_SIZE_MAX	(USER_WIDE_MODE					\
+			 ? (1 << 30)	/* 1 GB */			\
+			 : (CONFIG_MAX_STACK_SIZE_MB*1024*1024))
+
 #endif
 
 #ifndef __ASSEMBLY__
arch/parisc/kernel/sys_parisc.c +3 -3
index 31ffa9b55322..e1ffea2f9a0b 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > STACK_SIZE_MAX)
+		stack_base = STACK_SIZE_MAX;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +3 -3
index 476f3ebf437e..238b7aa26f68 100644
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > STACK_SIZE_MAX)
+		stack_base = STACK_SIZE_MAX;
 
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
mm/Kconfig +15 -0
index ebe5880c29d6..1b5a95f0fa01 100644
@@ -581,3 +581,18 @@ config PGTABLE_MAPPING
 
 config GENERIC_EARLY_IOREMAP
 	bool
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size for 32-bit processes (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP && (!64BIT || COMPAT)
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of 32-bit
+	  user processes when the stack grows upwards (currently only on parisc
+	  and metag arch). The stack will be located at the highest memory
+	  address minus the given value, unless the RLIMIT_STACK hard limit is
+	  changed to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.
Signature · 6 lines
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author maintainer
#8 lore raw

Hi James,

Quoted 46 lines · James Hogan
Quoted 45 lines · Helge Deller
Quoted 43 lines · James Bottomley
Quoted 31 lines · James Hogan
Quoted 12 lines · James Bottomley
Quoted 12 lines
(quoted diff) +11 -0
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
Quoted 8 lines

mm/Kconfig config PGTABLE_MAPPING · line 598

Quoted 3 lines
mm/Kconfig +3 -0
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Does the following fixup diff look reasonable? It forces MAX_STACK_SIZE_MB to 1024 and hides the Kconfig option for 64BIT, effectively leaving the behaviour unchanged in that case.

mm/Kconfig +0 -0
index e80075979530..b0307f737bd7 100644

mm/Kconfig config GENERIC_EARLY_IOREMAP · line 590

Quoted 9 lines
mm/Kconfig +2 -1
@@ -583,7 +583,8 @@ config GENERIC_EARLY_IOREMAP
 	bool
 
 config MAX_STACK_SIZE_MB
-	int "Maximum user stack size (MB)"
+	int "Maximum user stack size (MB)" if !64BIT
+	default 1024 if 64BIT
 	default 80
 	range 8 256 if METAG
 	range 8 2048

Yes, I think that's probably correct ...

Quoted 5 lines
Quoted 2 lines

No, it's not correct.
It will then choose then a 1GB stack for compat tasks on 64bit kernel.

Sorry for the delay (I had most of last week off sick and still catching up).

No problem.

Quoted 4 lines

That's a good point. It makes me think the best way to handle it is in a new definition in asm/processor.h, maybe STACK_SIZE_MAX. Does something like this look better? This patch isn't getting any cleaner unfortunately.

Yes, it's correct now.
Just tested it. Thanks!

Another problem:
I think you wanted to get it backported into older kernels?
It seems the changes to mm/Kconfig will not apply cleanly to 3.14 or lower, and the changes to arch/parisc/kernel/sys_parisc.c will not apply to 3.13 or lower...

Maybe cleaning up the patch to apply cleanly to 3.14 would make sense and ignore older kernels?

Helge

Quoted 142 lines

From 6ecb0392a3b670c4bf1641a6ec56f22427ca8b57 Mon Sep 17 00:00:00 2001
From: Helge Deller <[email protected]>
Date: Wed, 30 Apr 2014 23:26:02 +0200
Subject: [PATCH 1/1] parisc,metag: Do not hardcode maximum userspace stack
size

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB for 32-bit processes, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

This also fixes a BUG on metag if the RLIMIT_STACK hard limit is increased beyond a safe value by root. E.g. when starting a process after running "ulimit -H -s unlimited" it will then attempt to use a stack size of the maximum 1GB which is far too big for metag's limited user virtual address space (stack_top is usually 0x3ffff000):

BUG: failure at fs/exec.c:589/shift_arg_pages()!
Cc John David Anglin <[email protected]>
Cc [email protected] # only needed for >= v3.9 (arch/metag)

---
v3 (James Hogan):
- fix so that 64-bit parisc processes still use the 1GB limit. CONFIG_STACK_GROWSUP arches should provide a STACK_SIZE_MAX in their asm/processor.h, and for parisc it depends on USER_WIDE_MODE (whether the current process is 64-bit).
v2 (James Hogan):
- updated description to mention BUG on metag.
- added custom range limit for METAG.
- moved Kconfig symbol to mm/Kconfig and reworded.
- fixed "matag" typo.

---
 arch/metag/include/asm/processor.h  |  2 ++
 arch/parisc/include/asm/processor.h |  5 +++++
 arch/parisc/kernel/sys_parisc.c     |  6 +++---
 fs/exec.c                           |  6 +++---
 mm/Kconfig                          | 15 +++++++++++++++
 5 files changed, 28 insertions(+), 6 deletions(-)
arch/metag/include/asm/processor.h +2 -0
index f16477d1f571..a8a37477c66e 100644
@@ -22,6 +22,8 @@
 /* Add an extra page of padding at the top of the stack for the guard page. */
 #define STACK_TOP	(TASK_SIZE - PAGE_SIZE)
 #define STACK_TOP_MAX	STACK_TOP
+/* Maximum virtual space for stack */
+#define STACK_SIZE_MAX	(CONFIG_MAX_STACK_SIZE_MB*1024*1024)
 
 /* This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
arch/parisc/include/asm/processor.h +5 -0
index 198a86feb574..d951c9681ab3 100644
@@ -55,6 +55,11 @@
 #define STACK_TOP	TASK_SIZE
 #define STACK_TOP_MAX	DEFAULT_TASK_SIZE
 
+/* Allow bigger stacks for 64-bit processes */
+#define STACK_SIZE_MAX	(USER_WIDE_MODE					\
+			 ? (1 << 30)	/* 1 GB */			\
+			 : (CONFIG_MAX_STACK_SIZE_MB*1024*1024))
+
 #endif
 
 #ifndef __ASSEMBLY__
arch/parisc/kernel/sys_parisc.c +3 -3
index 31ffa9b55322..e1ffea2f9a0b 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > STACK_SIZE_MAX)
+		stack_base = STACK_SIZE_MAX;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +3 -3
index 476f3ebf437e..238b7aa26f68 100644
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > STACK_SIZE_MAX)
+		stack_base = STACK_SIZE_MAX;
 
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
mm/Kconfig +15 -0
index ebe5880c29d6..1b5a95f0fa01 100644
@@ -581,3 +581,18 @@ config PGTABLE_MAPPING
 
 config GENERIC_EARLY_IOREMAP
 	bool
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size for 32-bit processes (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP && (!64BIT || COMPAT)
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of 32-bit
+	  user processes when the stack grows upwards (currently only on parisc
+	  and metag arch). The stack will be located at the highest memory
+	  address minus the given value, unless the RLIMIT_STACK hard limit is
+	  changed to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.
#9 lore raw

Hi Helge,

Quoted 55 lines · Helge Deller
Quoted 46 lines · James Hogan
Quoted 45 lines · Helge Deller
Quoted 43 lines · James Bottomley
Quoted 31 lines · James Hogan
Quoted 12 lines · James Bottomley
Quoted 12 lines
(quoted diff) +11 -0
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
Quoted 8 lines

mm/Kconfig config PGTABLE_MAPPING · line 598

Quoted 3 lines
mm/Kconfig +3 -0
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.

There's one final issue with this: placement of the stack only really matters on 32 bits. We have three expanding memory areas: stack, heap and maps. On 64 bits these are placed well separated from each other on 64 bits, so an artificial limit like this doesn't matter.

Does the following fixup diff look reasonable? It forces MAX_STACK_SIZE_MB to 1024 and hides the Kconfig option for 64BIT, effectively leaving the behaviour unchanged in that case.

mm/Kconfig +0 -0
index e80075979530..b0307f737bd7 100644

mm/Kconfig config GENERIC_EARLY_IOREMAP · line 590

Quoted 9 lines
mm/Kconfig +2 -1
@@ -583,7 +583,8 @@ config GENERIC_EARLY_IOREMAP
 	bool
 
 config MAX_STACK_SIZE_MB
-	int "Maximum user stack size (MB)"
+	int "Maximum user stack size (MB)" if !64BIT
+	default 1024 if 64BIT
 	default 80
 	range 8 256 if METAG
 	range 8 2048

Yes, I think that's probably correct ...

Quoted 5 lines
Quoted 2 lines

No, it's not correct.
It will then choose then a 1GB stack for compat tasks on 64bit kernel.

Sorry for the delay (I had most of last week off sick and still catching up).

No problem.

Quoted 8 lines
Quoted 4 lines

That's a good point. It makes me think the best way to handle it is in a new definition in asm/processor.h, maybe STACK_SIZE_MAX. Does something like this look better? This patch isn't getting any cleaner unfortunately.

Yes, it's correct now.
Just tested it. Thanks!

Thanks.

Quoted 2 lines

Another problem:
I think you wanted to get it backported into older kernels?

Yeh, back to v3.9 ideally.

Quoted 1 line

It seems the changes to mm/Kconfig will not apply cleanly to 3.14 or lower,

Hmm, I guess the patch could be split easily though such that METAG always defined STACK_SIZE_MAX as 256MB and parisc always as 1GB (i.e. minimal fix for metag without actually changing any stack sizes), then have a second patch (not for stable) to make it match the v3 patch.

I'll try that (I probably should have done that in the first place).

Thanks
James

Quoted 151 lines

and the changes to arch/parisc/kernel/sys_parisc.c will not apply to 3.13 or lower...

Maybe cleaning up the patch to apply cleanly to 3.14 would make sense and ignore older kernels?

Helge

Quoted 142 lines

From 6ecb0392a3b670c4bf1641a6ec56f22427ca8b57 Mon Sep 17 00:00:00 2001
From: Helge Deller <[email protected]>
Date: Wed, 30 Apr 2014 23:26:02 +0200
Subject: [PATCH 1/1] parisc,metag: Do not hardcode maximum userspace stack
size

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB for 32-bit processes, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

This also fixes a BUG on metag if the RLIMIT_STACK hard limit is increased beyond a safe value by root. E.g. when starting a process after running "ulimit -H -s unlimited" it will then attempt to use a stack size of the maximum 1GB which is far too big for metag's limited user virtual address space (stack_top is usually 0x3ffff000):

BUG: failure at fs/exec.c:589/shift_arg_pages()!
Cc John David Anglin <[email protected]>
Cc [email protected] # only needed for >= v3.9 (arch/metag)

---
v3 (James Hogan):
- fix so that 64-bit parisc processes still use the 1GB limit. CONFIG_STACK_GROWSUP arches should provide a STACK_SIZE_MAX in their asm/processor.h, and for parisc it depends on USER_WIDE_MODE (whether the current process is 64-bit).
v2 (James Hogan):
- updated description to mention BUG on metag.
- added custom range limit for METAG.
- moved Kconfig symbol to mm/Kconfig and reworded.
- fixed "matag" typo.

---
 arch/metag/include/asm/processor.h  |  2 ++
 arch/parisc/include/asm/processor.h |  5 +++++
 arch/parisc/kernel/sys_parisc.c     |  6 +++---
 fs/exec.c                           |  6 +++---
 mm/Kconfig                          | 15 +++++++++++++++
 5 files changed, 28 insertions(+), 6 deletions(-)
arch/metag/include/asm/processor.h +2 -0
index f16477d1f571..a8a37477c66e 100644
@@ -22,6 +22,8 @@
 /* Add an extra page of padding at the top of the stack for the guard page. */
 #define STACK_TOP	(TASK_SIZE - PAGE_SIZE)
 #define STACK_TOP_MAX	STACK_TOP
+/* Maximum virtual space for stack */
+#define STACK_SIZE_MAX	(CONFIG_MAX_STACK_SIZE_MB*1024*1024)
 
 /* This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
arch/parisc/include/asm/processor.h +5 -0
index 198a86feb574..d951c9681ab3 100644
@@ -55,6 +55,11 @@
 #define STACK_TOP	TASK_SIZE
 #define STACK_TOP_MAX	DEFAULT_TASK_SIZE
 
+/* Allow bigger stacks for 64-bit processes */
+#define STACK_SIZE_MAX	(USER_WIDE_MODE					\
+			 ? (1 << 30)	/* 1 GB */			\
+			 : (CONFIG_MAX_STACK_SIZE_MB*1024*1024))
+
 #endif
 
 #ifndef __ASSEMBLY__
arch/parisc/kernel/sys_parisc.c +3 -3
index 31ffa9b55322..e1ffea2f9a0b 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > STACK_SIZE_MAX)
+		stack_base = STACK_SIZE_MAX;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +3 -3
index 476f3ebf437e..238b7aa26f68 100644
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > STACK_SIZE_MAX)
+		stack_base = STACK_SIZE_MAX;
 
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
mm/Kconfig +15 -0
index ebe5880c29d6..1b5a95f0fa01 100644
@@ -581,3 +581,18 @@ config PGTABLE_MAPPING
 
 config GENERIC_EARLY_IOREMAP
 	bool
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size for 32-bit processes (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP && (!64BIT || COMPAT)
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of 32-bit
+	  user processes when the stack grows upwards (currently only on parisc
+	  and metag arch). The stack will be located at the highest memory
+	  address minus the given value, unless the RLIMIT_STACK hard limit is
+	  changed to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.
author maintainer
#10 lore raw
In reply to James Hogan

Hi James,

Quoted 4 lines

I'd like to take this patch and submit upstream for v3.15, and mark for stable. Would that be okay with you?
[...]
How does the v2 below look?

Your patch looks good.
Thanks for cleaning it up and moving the config option to the better place. I just tested it on parisc and it works as expected.

I'm absolutely fine if you push your version of the patch through the metag git tree upstream.

Thanks!
Helge

Quoted 105 lines

From c34f0ec062ae1a2c9fca3eddbc705f6b0faf97ca Mon Sep 17 00:00:00 2001
From: Helge Deller <[email protected]>
Date: Wed, 30 Apr 2014 23:26:02 +0200
Subject: [PATCH v2] parisc,metag: Do not hardcode maximum userspace stack
size

This patch affects only architectures where the stack grows upwards (currently parisc and metag only). On those do not hardcode the maximum initial stack size to 1GB, but make it configurable via a config option.

The main problem with the hardcoded stack size is, that we have two memory regions which grow upwards: stack and heap. To keep most of the memory available for heap in a flexmap memoy layout, it makes no sense to hard allocate up to 1GB of the memory for stack which can't be used as heap then.

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

This also fixes a BUG on metag if the RLIMIT_STACK hard limit is increased beyond a safe value by root. E.g. when starting a process after running "ulimit -H -s unlimited" it will then attempt to use a stack size of the maximum 1GB which is far too big for metag's limited user virtual address space (stack_top is usually 0x3ffff000):

BUG: failure at fs/exec.c:589/shift_arg_pages()!

---
v2 (James Hogan):
- updated description to mention BUG on metag.
- added custom range limit for METAG.
- moved Kconfig symbol to mm/Kconfig and reworded.
- fixed "matag" typo.

---
 arch/parisc/kernel/sys_parisc.c |  6 +++---
 fs/exec.c                       |  6 +++---
 mm/Kconfig                      | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 6 deletions(-)
arch/parisc/kernel/sys_parisc.c +3 -3
index 31ffa9b55322..9f040261151e 100644
@@ -72,10 +72,10 @@ static unsigned long mmap_upper_limit(void)
 {
 	unsigned long stack_base;
 
-	/* Limit stack size to 1GB - see setup_arg_pages() in fs/exec.c */
+	/* Limit stack size - see setup_arg_pages() in fs/exec.c */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	return PAGE_ALIGN(STACK_TOP - stack_base);
 }
fs/exec.c +3 -3
index 476f3ebf437e..994108cc60f3 100644
@@ -657,10 +657,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	unsigned long rlim_stack;
 
 #ifdef CONFIG_STACK_GROWSUP
-	/* Limit stack size to 1GB */
+	/* Limit stack size */
 	stack_base = rlimit_max(RLIMIT_STACK);
-	if (stack_base > (1 << 30))
-		stack_base = 1 << 30;
+	if (stack_base > CONFIG_MAX_STACK_SIZE_MB*1024*1024)
+		stack_base = CONFIG_MAX_STACK_SIZE_MB*1024*1024;
 
 	/* Make sure we didn't let the argument array grow too large. */
 	if (vma->vm_end - vma->vm_start > stack_base)
mm/Kconfig +15 -0
index ebe5880c29d6..e80075979530 100644
@@ -581,3 +581,18 @@ config PGTABLE_MAPPING
 
 config GENERIC_EARLY_IOREMAP
 	bool
+
+config MAX_STACK_SIZE_MB
+	int "Maximum user stack size (MB)"
+	default 80
+	range 8 256 if METAG
+	range 8 2048
+	depends on STACK_GROWSUP
+	help
+	  This is the maximum stack size in Megabytes in the VM layout of user
+	  processes when the stack grows upwards (currently only on parisc and
+	  metag arch). The stack will be located at the highest memory address
+	  minus the given value, unless the RLIMIT_STACK hard limit is changed
+	  to a smaller value in which case that is used.
+
+	  A sane initial value is 80 MB.
Tested-by on v1 1/1
#11 lore raw
In reply to Helge Deller
Quoted 9 lines · Helge Deller

This patch makes the stack size configurable and uses 80MB as default value which has been in use during the last few years on parisc and which didn't showed any problems yet.

I tested this version of the patch last night on 3.14.2. I can confirm that an 80MB region is reserved for stack at the expected location in virtual memory with the default config setting. GCC and many other packages have built successfully with this setting.

---

Dave

Signature · 1 line
John David Anglin    [email protected]