Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 >> #include >> +#include >> >> #define THREAD_SIZE_ORDER 1 >> -#define THREAD_SIZE8192 >> +#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 > - > /* 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 > #include > #include > +#include > #ifdef CONFIG_MTD_PHYSMAP > #include > #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, >
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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_SIZE8192 +#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,
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 > #include > +#include > > #define THREAD_SIZE_ORDER 1 > -#define THREAD_SIZE8192 > +#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 - /* 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 #include #include +#include #ifdef CONFIG_MTD_PHYSMAP #include #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,
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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_SIZE8192 +#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
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 > > Acked-by: Nicolas Pitre > > 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 >> #include >> #include >> +#include >> +#include >> >> #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 >> #include >> +#include >> >> #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 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 Acked-by: Nicolas Pitre > --- > 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 > #include > #include > +#include > +#include > > #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 > #include > +#include > > #define THREAD_SIZE_ORDER1 > -#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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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.ryabi...@samsung.com Acked-by: Nicolas Pitre n...@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_ORDER1 -#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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: get rid of hardcoded assumptions about kernel stack size
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.ryabi...@samsung.com Acked-by: Nicolas Pitre n...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
