* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
@ 2014-06-18 13:50 Andrey Ryabinin
2014-06-18 14:31 ` Will Deacon
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Andrey Ryabinin @ 2014-06-18 13:50 UTC (permalink / raw)
To: linux-arm-kernel
Changing kernel stack size on arm is not as simple as it should be:
1) THRED_SIZE macro doen't respect PAGE_SIZE and THREAD_SIZE_ORDER
2) stack size is hardcoded in get_thread_info macro
This patch fixes it by caculating THREAD_SIZE and thread_info address
taking into account PAGE_SIZE and THREAD_SIZE_ORDER.
Now changing stack size becomes simply changing THREAD_SIZE_ORDER.
Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
arch/arm/include/asm/assembler.h | 8 +++++---
arch/arm/include/asm/thread_info.h | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 57f0584..906703a 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -24,6 +24,8 @@
#include <asm/domain.h>
#include <asm/opcodes-virt.h>
#include <asm/asm-offsets.h>
+#include <asm/page.h>
+#include <asm/thread_info.h>
#define IOMEM(x) (x)
@@ -179,10 +181,10 @@
* Get current thread_info.
*/
.macro get_thread_info, rd
- ARM( mov \rd, sp, lsr #13 )
+ ARM( mov \rd, sp, lsr #THREAD_SIZE_ORDER + PAGE_SHIFT )
THUMB( mov \rd, sp )
- THUMB( lsr \rd, \rd, #13 )
- mov \rd, \rd, lsl #13
+ THUMB( lsr \rd, \rd, #THREAD_SIZE_ORDER + PAGE_SHIFT )
+ mov \rd, \rd, lsl #THREAD_SIZE_ORDER + PAGE_SHIFT
.endm
/*
diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index f989d7c..f85d2b0 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -14,9 +14,10 @@
#include <linux/compiler.h>
#include <asm/fpstate.h>
+#include <asm/page.h>
#define THREAD_SIZE_ORDER 1
-#define THREAD_SIZE 8192
+#define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER)
#define THREAD_START_SP (THREAD_SIZE - 8)
#ifndef __ASSEMBLY__
--
1.8.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-06-18 13:50 [PATCH] arm: get rid of hardcoded assumptions about kernel stack size Andrey Ryabinin @ 2014-06-18 14:31 ` Will Deacon 2014-06-18 14:49 ` Andrey Ryabinin 2014-06-18 14:40 ` Nicolas Pitre 2014-07-03 20:24 ` Arnd Bergmann 2 siblings, 1 reply; 9+ messages in thread From: Will Deacon @ 2014-06-18 14:31 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 18, 2014 at 02:50:22PM +0100, Andrey Ryabinin wrote: > Changing kernel stack size on arm is not as simple as it should be: > 1) THRED_SIZE macro doen't respect PAGE_SIZE and THREAD_SIZE_ORDER THREAD_SIZE > 2) stack size is hardcoded in get_thread_info macro > > This patch fixes it by caculating THREAD_SIZE and thread_info address > taking into account PAGE_SIZE and THREAD_SIZE_ORDER. > > Now changing stack size becomes simply changing THREAD_SIZE_ORDER. Curious: is this just a cleanup, or are you actually running out of kernel stack on an ARM platform? Will ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-06-18 14:31 ` Will Deacon @ 2014-06-18 14:49 ` Andrey Ryabinin 0 siblings, 0 replies; 9+ messages in thread From: Andrey Ryabinin @ 2014-06-18 14:49 UTC (permalink / raw) To: linux-arm-kernel On 06/18/14 18:31, Will Deacon wrote: > On Wed, Jun 18, 2014 at 02:50:22PM +0100, Andrey Ryabinin wrote: >> Changing kernel stack size on arm is not as simple as it should be: >> 1) THRED_SIZE macro doen't respect PAGE_SIZE and THREAD_SIZE_ORDER > > THREAD_SIZE > Yup, I just found some more typos in my commit message. I'll fix them in update. >> 2) stack size is hardcoded in get_thread_info macro >> >> This patch fixes it by caculating THREAD_SIZE and thread_info address >> taking into account PAGE_SIZE and THREAD_SIZE_ORDER. >> >> Now changing stack size becomes simply changing THREAD_SIZE_ORDER. > > Curious: is this just a cleanup, or are you actually running out of kernel > stack on an ARM platform? > It's actually both. I'm working on address sanitizer for kernel [1]. Recently we started experiments with stack instrumentation. Compiler inserts redzones around every variable on stack, so we could detect accesses to such redzones and catch out-of-bound read/write bugs on stack variables. Obviously stack is bloated in such kernel. For mainline kernel it just a cleanup. [1] https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel > Will > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-06-18 13:50 [PATCH] arm: get rid of hardcoded assumptions about kernel stack size Andrey Ryabinin 2014-06-18 14:31 ` Will Deacon @ 2014-06-18 14:40 ` Nicolas Pitre 2014-06-18 15:27 ` Andrey Ryabinin 2014-07-03 20:24 ` Arnd Bergmann 2 siblings, 1 reply; 9+ messages in thread From: Nicolas Pitre @ 2014-06-18 14:40 UTC (permalink / raw) To: linux-arm-kernel On Wed, 18 Jun 2014, Andrey Ryabinin wrote: > Changing kernel stack size on arm is not as simple as it should be: > 1) THRED_SIZE macro doen't respect PAGE_SIZE and THREAD_SIZE_ORDER > 2) stack size is hardcoded in get_thread_info macro > > This patch fixes it by caculating THREAD_SIZE and thread_info address > taking into account PAGE_SIZE and THREAD_SIZE_ORDER. > > Now changing stack size becomes simply changing THREAD_SIZE_ORDER. > > Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com> Acked-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/include/asm/assembler.h | 8 +++++--- > arch/arm/include/asm/thread_info.h | 3 ++- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h > index 57f0584..906703a 100644 > --- a/arch/arm/include/asm/assembler.h > +++ b/arch/arm/include/asm/assembler.h > @@ -24,6 +24,8 @@ > #include <asm/domain.h> > #include <asm/opcodes-virt.h> > #include <asm/asm-offsets.h> > +#include <asm/page.h> > +#include <asm/thread_info.h> > > #define IOMEM(x) (x) > > @@ -179,10 +181,10 @@ > * Get current thread_info. > */ > .macro get_thread_info, rd > - ARM( mov \rd, sp, lsr #13 ) > + ARM( mov \rd, sp, lsr #THREAD_SIZE_ORDER + PAGE_SHIFT ) > THUMB( mov \rd, sp ) > - THUMB( lsr \rd, \rd, #13 ) > - mov \rd, \rd, lsl #13 > + THUMB( lsr \rd, \rd, #THREAD_SIZE_ORDER + PAGE_SHIFT ) > + mov \rd, \rd, lsl #THREAD_SIZE_ORDER + PAGE_SHIFT > .endm > > /* > diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h > index f989d7c..f85d2b0 100644 > --- a/arch/arm/include/asm/thread_info.h > +++ b/arch/arm/include/asm/thread_info.h > @@ -14,9 +14,10 @@ > > #include <linux/compiler.h> > #include <asm/fpstate.h> > +#include <asm/page.h> > > #define THREAD_SIZE_ORDER 1 > -#define THREAD_SIZE 8192 > +#define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER) > #define THREAD_START_SP (THREAD_SIZE - 8) > > #ifndef __ASSEMBLY__ > -- > 1.8.5.5 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-06-18 14:40 ` Nicolas Pitre @ 2014-06-18 15:27 ` Andrey Ryabinin 0 siblings, 0 replies; 9+ messages in thread From: Andrey Ryabinin @ 2014-06-18 15:27 UTC (permalink / raw) To: linux-arm-kernel On 06/18/14 18:40, Nicolas Pitre wrote: > On Wed, 18 Jun 2014, Andrey Ryabinin wrote: > >> Changing kernel stack size on arm is not as simple as it should be: >> 1) THRED_SIZE macro doen't respect PAGE_SIZE and THREAD_SIZE_ORDER >> 2) stack size is hardcoded in get_thread_info macro >> >> This patch fixes it by caculating THREAD_SIZE and thread_info address >> taking into account PAGE_SIZE and THREAD_SIZE_ORDER. >> >> Now changing stack size becomes simply changing THREAD_SIZE_ORDER. >> >> Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com> > > Acked-by: Nicolas Pitre <nico@linaro.org> > > Thanks. Patch with fixes in commit message pushed to Russel's patch tracking system, #8078/1. http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8078/1 >> --- >> arch/arm/include/asm/assembler.h | 8 +++++--- >> arch/arm/include/asm/thread_info.h | 3 ++- >> 2 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h >> index 57f0584..906703a 100644 >> --- a/arch/arm/include/asm/assembler.h >> +++ b/arch/arm/include/asm/assembler.h >> @@ -24,6 +24,8 @@ >> #include <asm/domain.h> >> #include <asm/opcodes-virt.h> >> #include <asm/asm-offsets.h> >> +#include <asm/page.h> >> +#include <asm/thread_info.h> >> >> #define IOMEM(x) (x) >> >> @@ -179,10 +181,10 @@ >> * Get current thread_info. >> */ >> .macro get_thread_info, rd >> - ARM( mov \rd, sp, lsr #13 ) >> + ARM( mov \rd, sp, lsr #THREAD_SIZE_ORDER + PAGE_SHIFT ) >> THUMB( mov \rd, sp ) >> - THUMB( lsr \rd, \rd, #13 ) >> - mov \rd, \rd, lsl #13 >> + THUMB( lsr \rd, \rd, #THREAD_SIZE_ORDER + PAGE_SHIFT ) >> + mov \rd, \rd, lsl #THREAD_SIZE_ORDER + PAGE_SHIFT >> .endm >> >> /* >> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h >> index f989d7c..f85d2b0 100644 >> --- a/arch/arm/include/asm/thread_info.h >> +++ b/arch/arm/include/asm/thread_info.h >> @@ -14,9 +14,10 @@ >> >> #include <linux/compiler.h> >> #include <asm/fpstate.h> >> +#include <asm/page.h> >> >> #define THREAD_SIZE_ORDER 1 >> -#define THREAD_SIZE 8192 >> +#define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER) >> #define THREAD_START_SP (THREAD_SIZE - 8) >> >> #ifndef __ASSEMBLY__ >> -- >> 1.8.5.5 >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-06-18 13:50 [PATCH] arm: get rid of hardcoded assumptions about kernel stack size Andrey Ryabinin 2014-06-18 14:31 ` Will Deacon 2014-06-18 14:40 ` Nicolas Pitre @ 2014-07-03 20:24 ` Arnd Bergmann 2014-07-04 7:13 ` Andrey Ryabinin 2 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2014-07-03 20:24 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 18 June 2014, Andrey Ryabinin wrote: > diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h > index f989d7c..f85d2b0 100644 > --- a/arch/arm/include/asm/thread_info.h > +++ b/arch/arm/include/asm/thread_info.h > @@ -14,9 +14,10 @@ > > #include <linux/compiler.h> > #include <asm/fpstate.h> > +#include <asm/page.h> > > #define THREAD_SIZE_ORDER 1 > -#define THREAD_SIZE 8192 > +#define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER) > #define THREAD_START_SP (THREAD_SIZE - 8) > > #ifndef __ASSEMBLY__ The extra #include has caused recursive inclusion on the iop13xx platform, as you can see below. I've been able to work around it using this small patch: diff --git a/arch/arm/mach-iop13xx/include/mach/iop13xx.h b/arch/arm/mach-iop13xx/include/mach/iop13xx.h index 17b4027..e851f5c 100644 --- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h +++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h @@ -3,8 +3,6 @@ #ifndef __ASSEMBLY__ -#include <linux/reboot.h> - /* The ATU offsets can change based on the strapping */ extern u32 iop13xx_atux_pmmr_offset; extern u32 iop13xx_atue_pmmr_offset; @@ -14,6 +12,7 @@ void iop13xx_map_io(void); void iop13xx_platform_init(void); void iop13xx_add_tpmi_devices(void); void iop13xx_init_irq(void); +enum reboot_mode; void iop13xx_restart(enum reboot_mode, const char *); /* CPUID CP6 R0 Page 0 */ diff --git a/arch/arm/mach-iop13xx/setup.c b/arch/arm/mach-iop13xx/setup.c index bca96f43..53c316f 100644 --- a/arch/arm/mach-iop13xx/setup.c +++ b/arch/arm/mach-iop13xx/setup.c @@ -20,6 +20,7 @@ #include <linux/dma-mapping.h> #include <linux/serial_8250.h> #include <linux/io.h> +#include <linux/reboot.h> #ifdef CONFIG_MTD_PHYSMAP #include <linux/mtd/physmap.h> #endif but I wonder if there is a way to avoid the extra include here, as it might also cause a general slowdown because of asm/memory.h getting pulled into more .c files. Would it be reasonable to hardcode PAGE_SIZE here? Arnd ---- In file included from /git/arm-soc/include/linux/srcu.h:33:0, from /git/arm-soc/include/linux/notifier.h:15, from /git/arm-soc/include/linux/reboot.h:5, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/iop13xx.h:6, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/hardware.h:14, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/memory.h:4, from /git/arm-soc/arch/arm/include/asm/memory.h:24, from /git/arm-soc/arch/arm/include/asm/page.h:163, from /git/arm-soc/arch/arm/include/asm/thread_info.h:17, from /git/arm-soc/include/linux/thread_info.h:54, from /git/arm-soc/include/asm-generic/preempt.h:4, from arch/arm/include/generated/asm/preempt.h:1, from /git/arm-soc/include/linux/preempt.h:18, from /git/arm-soc/include/linux/spinlock.h:50, from /git/arm-soc/include/linux/seqlock.h:35, from /git/arm-soc/include/linux/time.h:5, from /git/arm-soc/include/uapi/linux/timex.h:56, from /git/arm-soc/include/linux/timex.h:56, from /git/arm-soc/include/linux/sched.h:19, from /git/arm-soc/arch/arm/kernel/asm-offsets.c:13: /git/arm-soc/include/linux/rcupdate.h: In function '__rcu_read_lock': /git/arm-soc/include/linux/rcupdate.h:219:2: error: implicit declaration of function 'preempt_disable' [-Werror=implicit-function-declaration] preempt_disable(); ^ /git/arm-soc/include/linux/rcupdate.h: In function '__rcu_read_unlock': /git/arm-soc/include/linux/rcupdate.h:224:2: error: implicit declaration of function 'preempt_enable' [-Werror=implicit-function-declaration] preempt_enable(); ^ In file included from /git/arm-soc/include/linux/srcu.h:33:0, from /git/arm-soc/include/linux/notifier.h:15, from /git/arm-soc/include/linux/reboot.h:5, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/iop13xx.h:6, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/hardware.h:14, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/memory.h:4, from /git/arm-soc/arch/arm/include/asm/memory.h:24, from /git/arm-soc/arch/arm/include/asm/page.h:163, from /git/arm-soc/arch/arm/include/asm/thread_info.h:17, from /git/arm-soc/include/linux/thread_info.h:54, from /git/arm-soc/include/asm-generic/preempt.h:4, from arch/arm/include/generated/asm/preempt.h:1, from /git/arm-soc/include/linux/preempt.h:18, from /git/arm-soc/include/linux/spinlock.h:50, from /git/arm-soc/include/linux/seqlock.h:35, from /git/arm-soc/include/linux/time.h:5, from /git/arm-soc/include/uapi/linux/timex.h:56, from /git/arm-soc/include/linux/timex.h:56, from /git/arm-soc/include/linux/sched.h:19, from /git/arm-soc/arch/arm/kernel/asm-offsets.c:13: /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_lock_bh': /git/arm-soc/include/linux/rcupdate.h:928:2: error: implicit declaration of function 'local_bh_disable' [-Werror=implicit-function-declaration] local_bh_disable(); ^ /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_unlock_bh': /git/arm-soc/include/linux/rcupdate.h:942:2: error: implicit declaration of function 'local_bh_enable' [-Werror=implicit-function-declaration] local_bh_enable(); ^ /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_lock_sched_notrace': /git/arm-soc/include/linux/rcupdate.h:968:2: error: implicit declaration of function 'preempt_disable_notrace' [-Werror=implicit-function-declaration] preempt_disable_notrace(); ^ /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_unlock_sched_notrace': /git/arm-soc/include/linux/rcupdate.h:988:2: error: implicit declaration of function 'preempt_enable_notrace' [-Werror=implicit-function-declaration] preempt_enable_notrace(); ^ In file included from /git/arm-soc/include/linux/ktime.h:25:0, from /git/arm-soc/include/linux/timer.h:5, from /git/arm-soc/include/linux/workqueue.h:8, from /git/arm-soc/include/linux/srcu.h:34, from /git/arm-soc/include/linux/notifier.h:15, from /git/arm-soc/include/linux/reboot.h:5, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/iop13xx.h:6, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/hardware.h:14, from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/memory.h:4, from /git/arm-soc/arch/arm/include/asm/memory.h:24, from /git/arm-soc/arch/arm/include/asm/page.h:163, from /git/arm-soc/arch/arm/include/asm/thread_info.h:17, from /git/arm-soc/include/linux/thread_info.h:54, from /git/arm-soc/include/asm-generic/preempt.h:4, from arch/arm/include/generated/asm/preempt.h:1, from /git/arm-soc/include/linux/preempt.h:18, from /git/arm-soc/include/linux/spinlock.h:50, from /git/arm-soc/include/linux/seqlock.h:35, from /git/arm-soc/include/linux/time.h:5, from /git/arm-soc/include/uapi/linux/timex.h:56, from /git/arm-soc/include/linux/timex.h:56, from /git/arm-soc/include/linux/sched.h:19, from /git/arm-soc/arch/arm/kernel/asm-offsets.c:13: /git/arm-soc/include/linux/jiffies.h: At top level: /git/arm-soc/include/linux/jiffies.h:256:10: warning: "NSEC_PER_SEC" is not defined [-Wundef] ... many more... ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-07-03 20:24 ` Arnd Bergmann @ 2014-07-04 7:13 ` Andrey Ryabinin 2014-07-04 10:27 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Andrey Ryabinin @ 2014-07-04 7:13 UTC (permalink / raw) To: linux-arm-kernel On 07/04/14 00:24, Arnd Bergmann wrote: > On Wednesday 18 June 2014, Andrey Ryabinin wrote: >> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h >> index f989d7c..f85d2b0 100644 >> --- a/arch/arm/include/asm/thread_info.h >> +++ b/arch/arm/include/asm/thread_info.h >> @@ -14,9 +14,10 @@ >> >> #include <linux/compiler.h> >> #include <asm/fpstate.h> >> +#include <asm/page.h> >> >> #define THREAD_SIZE_ORDER 1 >> -#define THREAD_SIZE 8192 >> +#define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER) >> #define THREAD_START_SP (THREAD_SIZE - 8) >> >> #ifndef __ASSEMBLY__ > > The extra #include has caused recursive inclusion on the iop13xx platform, as you > can see below. I've been able to work around it using this small patch: > Right, I've got report about this from kbuild test robot, and I was going to send the same fix as yours, but you come first. > diff --git a/arch/arm/mach-iop13xx/include/mach/iop13xx.h b/arch/arm/mach-iop13xx/include/mach/iop13xx.h > index 17b4027..e851f5c 100644 > --- a/arch/arm/mach-iop13xx/include/mach/iop13xx.h > +++ b/arch/arm/mach-iop13xx/include/mach/iop13xx.h > @@ -3,8 +3,6 @@ > > #ifndef __ASSEMBLY__ > > -#include <linux/reboot.h> > - > /* The ATU offsets can change based on the strapping */ > extern u32 iop13xx_atux_pmmr_offset; > extern u32 iop13xx_atue_pmmr_offset; > @@ -14,6 +12,7 @@ void iop13xx_map_io(void); > void iop13xx_platform_init(void); > void iop13xx_add_tpmi_devices(void); > void iop13xx_init_irq(void); > +enum reboot_mode; > void iop13xx_restart(enum reboot_mode, const char *); > > /* CPUID CP6 R0 Page 0 */ > diff --git a/arch/arm/mach-iop13xx/setup.c b/arch/arm/mach-iop13xx/setup.c > index bca96f43..53c316f 100644 > --- a/arch/arm/mach-iop13xx/setup.c > +++ b/arch/arm/mach-iop13xx/setup.c > @@ -20,6 +20,7 @@ > #include <linux/dma-mapping.h> > #include <linux/serial_8250.h> > #include <linux/io.h> > +#include <linux/reboot.h> > #ifdef CONFIG_MTD_PHYSMAP > #include <linux/mtd/physmap.h> > #endif > > > but I wonder if there is a way to avoid the extra include here, as it might also > cause a general slowdown because of asm/memory.h getting pulled into more .c > files. Would it be reasonable to hardcode PAGE_SIZE here? > IMO it's a bug of iop13xx platform, that it includes "higlevel" linux/reboot.h from a very "lowlevel" header mach/iop13xx.h. I think it should be fixed with a patch above. Slowing down of kernel build for a few more seconds is not good enough reason for me to hardcode PAGE_SIZE here. > Arnd > > ---- > In file included from /git/arm-soc/include/linux/srcu.h:33:0, > from /git/arm-soc/include/linux/notifier.h:15, > from /git/arm-soc/include/linux/reboot.h:5, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/iop13xx.h:6, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/hardware.h:14, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/memory.h:4, > from /git/arm-soc/arch/arm/include/asm/memory.h:24, > from /git/arm-soc/arch/arm/include/asm/page.h:163, > from /git/arm-soc/arch/arm/include/asm/thread_info.h:17, > from /git/arm-soc/include/linux/thread_info.h:54, > from /git/arm-soc/include/asm-generic/preempt.h:4, > from arch/arm/include/generated/asm/preempt.h:1, > from /git/arm-soc/include/linux/preempt.h:18, > from /git/arm-soc/include/linux/spinlock.h:50, > from /git/arm-soc/include/linux/seqlock.h:35, > from /git/arm-soc/include/linux/time.h:5, > from /git/arm-soc/include/uapi/linux/timex.h:56, > from /git/arm-soc/include/linux/timex.h:56, > from /git/arm-soc/include/linux/sched.h:19, > from /git/arm-soc/arch/arm/kernel/asm-offsets.c:13: > /git/arm-soc/include/linux/rcupdate.h: In function '__rcu_read_lock': > /git/arm-soc/include/linux/rcupdate.h:219:2: error: implicit declaration of function 'preempt_disable' [-Werror=implicit-function-declaration] > preempt_disable(); > ^ > /git/arm-soc/include/linux/rcupdate.h: In function '__rcu_read_unlock': > /git/arm-soc/include/linux/rcupdate.h:224:2: error: implicit declaration of function 'preempt_enable' [-Werror=implicit-function-declaration] > preempt_enable(); > ^ > In file included from /git/arm-soc/include/linux/srcu.h:33:0, > from /git/arm-soc/include/linux/notifier.h:15, > from /git/arm-soc/include/linux/reboot.h:5, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/iop13xx.h:6, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/hardware.h:14, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/memory.h:4, > from /git/arm-soc/arch/arm/include/asm/memory.h:24, > from /git/arm-soc/arch/arm/include/asm/page.h:163, > from /git/arm-soc/arch/arm/include/asm/thread_info.h:17, > from /git/arm-soc/include/linux/thread_info.h:54, > from /git/arm-soc/include/asm-generic/preempt.h:4, > from arch/arm/include/generated/asm/preempt.h:1, > from /git/arm-soc/include/linux/preempt.h:18, > from /git/arm-soc/include/linux/spinlock.h:50, > from /git/arm-soc/include/linux/seqlock.h:35, > from /git/arm-soc/include/linux/time.h:5, > from /git/arm-soc/include/uapi/linux/timex.h:56, > from /git/arm-soc/include/linux/timex.h:56, > from /git/arm-soc/include/linux/sched.h:19, > from /git/arm-soc/arch/arm/kernel/asm-offsets.c:13: > /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_lock_bh': > /git/arm-soc/include/linux/rcupdate.h:928:2: error: implicit declaration of function 'local_bh_disable' [-Werror=implicit-function-declaration] > local_bh_disable(); > ^ > /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_unlock_bh': > /git/arm-soc/include/linux/rcupdate.h:942:2: error: implicit declaration of function 'local_bh_enable' [-Werror=implicit-function-declaration] > local_bh_enable(); > ^ > /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_lock_sched_notrace': > /git/arm-soc/include/linux/rcupdate.h:968:2: error: implicit declaration of function 'preempt_disable_notrace' [-Werror=implicit-function-declaration] > preempt_disable_notrace(); > ^ > /git/arm-soc/include/linux/rcupdate.h: In function 'rcu_read_unlock_sched_notrace': > /git/arm-soc/include/linux/rcupdate.h:988:2: error: implicit declaration of function 'preempt_enable_notrace' [-Werror=implicit-function-declaration] > preempt_enable_notrace(); > ^ > In file included from /git/arm-soc/include/linux/ktime.h:25:0, > from /git/arm-soc/include/linux/timer.h:5, > from /git/arm-soc/include/linux/workqueue.h:8, > from /git/arm-soc/include/linux/srcu.h:34, > from /git/arm-soc/include/linux/notifier.h:15, > from /git/arm-soc/include/linux/reboot.h:5, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/iop13xx.h:6, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/hardware.h:14, > from /git/arm-soc/arch/arm/mach-iop13xx/include/mach/memory.h:4, > from /git/arm-soc/arch/arm/include/asm/memory.h:24, > from /git/arm-soc/arch/arm/include/asm/page.h:163, > from /git/arm-soc/arch/arm/include/asm/thread_info.h:17, > from /git/arm-soc/include/linux/thread_info.h:54, > from /git/arm-soc/include/asm-generic/preempt.h:4, > from arch/arm/include/generated/asm/preempt.h:1, > from /git/arm-soc/include/linux/preempt.h:18, > from /git/arm-soc/include/linux/spinlock.h:50, > from /git/arm-soc/include/linux/seqlock.h:35, > from /git/arm-soc/include/linux/time.h:5, > from /git/arm-soc/include/uapi/linux/timex.h:56, > from /git/arm-soc/include/linux/timex.h:56, > from /git/arm-soc/include/linux/sched.h:19, > from /git/arm-soc/arch/arm/kernel/asm-offsets.c:13: > /git/arm-soc/include/linux/jiffies.h: At top level: > /git/arm-soc/include/linux/jiffies.h:256:10: warning: "NSEC_PER_SEC" is not defined [-Wundef] > ... many more... > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-07-04 7:13 ` Andrey Ryabinin @ 2014-07-04 10:27 ` Arnd Bergmann 2014-07-04 13:38 ` Andrey Ryabinin 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2014-07-04 10:27 UTC (permalink / raw) To: linux-arm-kernel On Friday 04 July 2014 11:13:31 Andrey Ryabinin wrote: > > > > but I wonder if there is a way to avoid the extra include here, as it might also > > cause a general slowdown because of asm/memory.h getting pulled into more .c > > files. Would it be reasonable to hardcode PAGE_SIZE here? > > > > IMO it's a bug of iop13xx platform, that it includes "higlevel" linux/reboot.h > from a very "lowlevel" header mach/iop13xx.h. I think it should be fixed with a patch above. > Slowing down of kernel build for a few more seconds is not good enough reason for me to > hardcode PAGE_SIZE here. I don't think we can pinpoint a specific header that is "wrong" here, the fundamental problem is that our header files are a bit messy when it comes to recursive inclusion and we'd be better off if we generally were a little more careful about including headers from other headers. It's also very hard to retroactively clean this up on a large scale. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] arm: get rid of hardcoded assumptions about kernel stack size 2014-07-04 10:27 ` Arnd Bergmann @ 2014-07-04 13:38 ` Andrey Ryabinin 0 siblings, 0 replies; 9+ messages in thread From: Andrey Ryabinin @ 2014-07-04 13:38 UTC (permalink / raw) To: linux-arm-kernel On 07/04/14 14:27, Arnd Bergmann wrote: > On Friday 04 July 2014 11:13:31 Andrey Ryabinin wrote: >>> >>> but I wonder if there is a way to avoid the extra include here, as it might also >>> cause a general slowdown because of asm/memory.h getting pulled into more .c >>> files. Would it be reasonable to hardcode PAGE_SIZE here? >>> >> >> IMO it's a bug of iop13xx platform, that it includes "higlevel" linux/reboot.h >> from a very "lowlevel" header mach/iop13xx.h. I think it should be fixed with a patch above. >> Slowing down of kernel build for a few more seconds is not good enough reason for me to >> hardcode PAGE_SIZE here. > > I don't think we can pinpoint a specific header that is "wrong" here, the That's just my opinion. Anyway, if this header needed only for declaring single enum it worth to replace include with enum declaration. > fundamental problem is that our header files are a bit messy when it comes > to recursive inclusion and we'd be better off if we generally were a little > more careful about including headers from other headers. > That's why we need to remove reboot.h from iop13xx.h. Are you going to send a proper formatted patch for that? Speaking of asm/page.h, a lot of other architectures also have it included in thread_info.h, so I would leave it there. > It's also very hard to retroactively clean this up on a large scale. > > Arnd > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-07-04 13:38 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-18 13:50 [PATCH] arm: get rid of hardcoded assumptions about kernel stack size Andrey Ryabinin 2014-06-18 14:31 ` Will Deacon 2014-06-18 14:49 ` Andrey Ryabinin 2014-06-18 14:40 ` Nicolas Pitre 2014-06-18 15:27 ` Andrey Ryabinin 2014-07-03 20:24 ` Arnd Bergmann 2014-07-04 7:13 ` Andrey Ryabinin 2014-07-04 10:27 ` Arnd Bergmann 2014-07-04 13:38 ` Andrey Ryabinin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).