| Message ID | cc1190008368a3305d424a98be756fedec6348c1.1653926503.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/Kconfig: Force THREAD_SHIFT to at least 14 with KASAN | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | fail | boot (pmac32_defconfig, mac99, qemu-system-ppc openbios-ppc, ppc-rootfs.cpio.gz, korg-5.5.0, fedo... failed at step Run qemu-mac99 with korg-5.5.0 build kernel. |
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Allthough 14 is the default THREAD_SHIFT when KASAN is selected, > taking an old config may keep 13 when CONFIG_EXPERT is selected. > > Force it to 14 as a minimum when KASAN is selected. > > Also default to 15 when KASAN on PPC64. > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/Kconfig | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) I was thinking of doing it in C, similar to the way arm64 handles it. Something like below. It means we always double the stack size when KASAN is enabled. I think it's preferable, as it will always work regardless of whether the user has an old .config (or is bisectting)? cheers diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h index 125328d1b980..c9735f93f8e6 100644 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -14,12 +14,17 @@ #ifdef __KERNEL__ -#if defined(CONFIG_VMAP_STACK) && CONFIG_THREAD_SHIFT < PAGE_SHIFT -#define THREAD_SHIFT PAGE_SHIFT +#ifdef CONFIG_KASAN +#define THREAD_SHIFT (CONFIG_THREAD_SHIFT + 1) #else #define THREAD_SHIFT CONFIG_THREAD_SHIFT #endif +#if defined(CONFIG_VMAP_STACK) && THREAD_SHIFT < PAGE_SHIFT +#undef THREAD_SHIFT +#define THREAD_SHIFT PAGE_SHIFT +#endif + #define THREAD_SIZE (1 << THREAD_SHIFT) /*
Le 31/05/2022 à 08:16, Michael Ellerman a écrit : > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> Allthough 14 is the default THREAD_SHIFT when KASAN is selected, >> taking an old config may keep 13 when CONFIG_EXPERT is selected. >> >> Force it to 14 as a minimum when KASAN is selected. >> >> Also default to 15 when KASAN on PPC64. >> >> Reported-by: Erhard Furtner <erhard_f@mailbox.org> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/powerpc/Kconfig | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) > > I was thinking of doing it in C, similar to the way arm64 handles it. > > Something like below. It means we always double the stack size when > KASAN is enabled. I think it's preferable, as it will always work > regardless of whether the user has an old .config (or is bisectting)? Yes good idea. I leave it to you then. You can therefore also remove the default value for KASAN in Kconfig. Christophe > > cheers > > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h > index 125328d1b980..c9735f93f8e6 100644 > --- a/arch/powerpc/include/asm/thread_info.h > +++ b/arch/powerpc/include/asm/thread_info.h > @@ -14,12 +14,17 @@ > > #ifdef __KERNEL__ > > -#if defined(CONFIG_VMAP_STACK) && CONFIG_THREAD_SHIFT < PAGE_SHIFT > -#define THREAD_SHIFT PAGE_SHIFT > +#ifdef CONFIG_KASAN > +#define THREAD_SHIFT (CONFIG_THREAD_SHIFT + 1) > #else > #define THREAD_SHIFT CONFIG_THREAD_SHIFT > #endif > > +#if defined(CONFIG_VMAP_STACK) && THREAD_SHIFT < PAGE_SHIFT > +#undef THREAD_SHIFT > +#define THREAD_SHIFT PAGE_SHIFT > +#endif > + > #define THREAD_SIZE (1 << THREAD_SHIFT) > > /* > >
On Tue, May 31, 2022 at 04:16:19PM +1000, Michael Ellerman wrote: > I was thinking of doing it in C, similar to the way arm64 handles it. > > Something like below. It means we always double the stack size when > KASAN is enabled. I think it's preferable, as it will always work > regardless of whether the user has an old .config (or is bisectting)? Is there any reason to even offer the Kconfig? It is super cryptic and just picking the right value directly in the header would seem much more sensible: #if defined(CONFIG_PPC_256K_PAGES) #define MIN_THREAD_SHIFT 15 #elif defined(CONFIG_PPC64) #define MIN_THREAD_SHIFT 14 #else #define MIN_THREAD_SHIFT 13 #endif #ifdef CONFIG_KASAN #define THREAD_SHIFT (MIN_THREAD_SHIFT + 1) #else #define THREAD_SHIFT MIN_THREAD_SHIFT #endif #if defined(CONFIG_VMAP_STACK) && THREAD_SHIFT < PAGE_SHIFT #undef THREAD_SHIFT #define THREAD_SHIFT PAGE_SHIFT #endif
Le 31/05/2022 à 09:41, Christoph Hellwig a écrit : > On Tue, May 31, 2022 at 04:16:19PM +1000, Michael Ellerman wrote: >> I was thinking of doing it in C, similar to the way arm64 handles it. >> >> Something like below. It means we always double the stack size when >> KASAN is enabled. I think it's preferable, as it will always work >> regardless of whether the user has an old .config (or is bisectting)? > > Is there any reason to even offer the Kconfig? It is super cryptic and > just picking the right value directly in the header would seem much > more sensible: Looks like it was added by commit 476134070c03 ("powerpc: Move THREAD_SHIFT config to Kconfig") in 2017. https://github.com/torvalds/linux/commit/476134070c037820bd909ff6e43e0d3eae33f376 Seems like powerpc is the only architecture with it configurable via Kconfig. I'd be inclined to reverting that commit, and then I'd do something similar to arm64: #ifdef CONFIG_KSAN #define KASAN_THREAD_SHIFT 1 #else #define KASAN_THREAD_SHIFT 0 #endif #if defined(CONFIG_PPC_256K_PAGES) #define MIN_THREAD_SHIFT (15 + KASAN_THREAD_SHIFT) #elif defined(CONFIG_PPC64) #define MIN_THREAD_SHIFT (14 + KASAN_THREAD_SHIFT) #else #define MIN_THREAD_SHIFT (13 + KASAN_THREAD_SHIFT) #endif #if defined(CONFIG_VMAP_STACK) && (MIN_THREAD_SHIFT < PAGE_SHIFT) #define THREAD_SHIFT PAGE_SHIFT #else #define THREAD_SHIFT MIN_THREAD_SHIFT #endif And maybe the CONFIG_PPC_256K_PAGES case is already big enough and doesn't require the + 1 for KASAN. > > #if defined(CONFIG_PPC_256K_PAGES) > #define MIN_THREAD_SHIFT 15 > #elif defined(CONFIG_PPC64) > #define MIN_THREAD_SHIFT 14 > #else > #define MIN_THREAD_SHIFT 13 > #endif > > #ifdef CONFIG_KASAN > #define THREAD_SHIFT (MIN_THREAD_SHIFT + 1) > #else > #define THREAD_SHIFT MIN_THREAD_SHIFT > #endif > > #if defined(CONFIG_VMAP_STACK) && THREAD_SHIFT < PAGE_SHIFT > #undef THREAD_SHIFT > #define THREAD_SHIFT PAGE_SHIFT > #endif
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 3eaddb8997a9..a4427977fdf8 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -788,8 +788,10 @@ config PPC_PAGE_SHIFT config THREAD_SHIFT int "Thread shift" if EXPERT - range 13 15 + range 13 15 if !KASAN + range 14 15 if KASAN default "15" if PPC_256K_PAGES + default "15" if PPC64 && KASAN default "14" if PPC64 default "14" if KASAN default "13"
Allthough 14 is the default THREAD_SHIFT when KASAN is selected, taking an old config may keep 13 when CONFIG_EXPERT is selected. Force it to 14 as a minimum when KASAN is selected. Also default to 15 when KASAN on PPC64. Reported-by: Erhard Furtner <erhard_f@mailbox.org> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)