* [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
@ 2017-01-17 12:56 Stafford Horne
2017-01-17 13:07 ` Vlastimil Babka
0 siblings, 1 reply; 10+ messages in thread
From: Stafford Horne @ 2017-01-17 12:56 UTC (permalink / raw)
To: linux-kernel
Cc: Stafford Horne, Vlastimil Babka, Andrew Morton, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
A recent change in -next introduced static_keys in init zonelists,
details are:
Author: Vlastimil Babka <vbabka@suse.cz>
Date: Thu Jan 12 12:19:03 2017 +1100
commit f5adbdff6a1c40e19 ("mm, page_alloc: convert
page_group_by_mobility_disable to static key")
This causes the following warning in openrisc as reported by Guenter,
and repoduced by me.
WARNING: CPU: 0 PID: 0 at ./include/linux/jump_label.h:198 0xc02d758c
static_key_slow_inc used before call to jump_label_init
This fixes this by initialized jump_labels even earlier, I am suprized the
issue is not showing up in other platforms yet.
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
init/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/init/main.c b/init/main.c
index 8b1adb6e..d1ca7cb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -513,6 +513,7 @@ asmlinkage __visible void __init start_kernel(void)
boot_cpu_state_init();
smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
+ jump_label_init();
build_all_zonelists(NULL, NULL);
page_alloc_init();
@@ -526,8 +527,6 @@ asmlinkage __visible void __init start_kernel(void)
parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
NULL, set_init_arg);
- jump_label_init();
-
/*
* These use large bootmem allocations and must precede
* kmem_cache_init()
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 12:56 [PATCH -next] init/main: Init jump_labels before they are used to build zonelists Stafford Horne
@ 2017-01-17 13:07 ` Vlastimil Babka
2017-01-17 13:44 ` Peter Zijlstra
0 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2017-01-17 13:07 UTC (permalink / raw)
To: Stafford Horne, linux-kernel
Cc: Andrew Morton, Thomas Gleixner, Kees Cook, Jessica Yu,
Petr Mladek, Rasmus Villemoes, Yang Shi, Tejun Heo,
Prarit Bhargava, Yaowei Bai, Andrey Ryabinin, Peter Zijlstra
[+CC PeterZ]
On 01/17/2017 01:56 PM, Stafford Horne wrote:
> A recent change in -next introduced static_keys in init zonelists,
> details are:
>
> Author: Vlastimil Babka <vbabka@suse.cz>
> Date: Thu Jan 12 12:19:03 2017 +1100
> commit f5adbdff6a1c40e19 ("mm, page_alloc: convert
> page_group_by_mobility_disable to static key")
>
> This causes the following warning in openrisc as reported by Guenter,
> and repoduced by me.
>
> WARNING: CPU: 0 PID: 0 at ./include/linux/jump_label.h:198 0xc02d758c
> static_key_slow_inc used before call to jump_label_init
>
> This fixes this by initialized jump_labels even earlier, I am suprized the
> issue is not showing up in other platforms yet.
AFAICS it depends on how much memory is available at that point. But
maybe I just missed the warning somehow.
Anyway I'm not sure if this patch is safe. Hopefully Peter can judge
this better...
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
> init/main.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 8b1adb6e..d1ca7cb 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -513,6 +513,7 @@ asmlinkage __visible void __init start_kernel(void)
> boot_cpu_state_init();
> smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
>
> + jump_label_init();
> build_all_zonelists(NULL, NULL);
> page_alloc_init();
>
> @@ -526,8 +527,6 @@ asmlinkage __visible void __init start_kernel(void)
> parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
> NULL, set_init_arg);
>
> - jump_label_init();
> -
> /*
> * These use large bootmem allocations and must precede
> * kmem_cache_init()
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 13:07 ` Vlastimil Babka
@ 2017-01-17 13:44 ` Peter Zijlstra
2017-01-17 14:30 ` Stafford Horne
2017-01-23 5:54 ` Michael Ellerman
0 siblings, 2 replies; 10+ messages in thread
From: Peter Zijlstra @ 2017-01-17 13:44 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Stafford Horne, linux-kernel, Andrew Morton, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
On Tue, Jan 17, 2017 at 02:07:36PM +0100, Vlastimil Babka wrote:
> Anyway I'm not sure if this patch is safe. Hopefully Peter can judge
> this better...
>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> > init/main.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/init/main.c b/init/main.c
> > index 8b1adb6e..d1ca7cb 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -513,6 +513,7 @@ asmlinkage __visible void __init start_kernel(void)
> > boot_cpu_state_init();
> > smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
> >
> > + jump_label_init();
> > build_all_zonelists(NULL, NULL);
> > page_alloc_init();
> >
> > @@ -526,8 +527,6 @@ asmlinkage __visible void __init start_kernel(void)
> > parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
> > NULL, set_init_arg);
> >
> > - jump_label_init();
> > -
Urgh, that means auditing all archs that implement this. The thing
you're looking for is if the self-modifying code cruft can be done that
early.
x86 looks to be fine, because this is after setup_arch() which is
required for ideal_nops[] to be initialied and we use text_poke_early()
which doesn't really need anything else.
I've not gone through the other arches...
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 13:44 ` Peter Zijlstra
@ 2017-01-17 14:30 ` Stafford Horne
2017-01-17 16:11 ` Vlastimil Babka
2017-01-23 5:54 ` Michael Ellerman
1 sibling, 1 reply; 10+ messages in thread
From: Stafford Horne @ 2017-01-17 14:30 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Vlastimil Babka, linux-kernel, Andrew Morton, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
On Tue, Jan 17, 2017 at 02:44:54PM +0100, Peter Zijlstra wrote:
> On Tue, Jan 17, 2017 at 02:07:36PM +0100, Vlastimil Babka wrote:
>
> > Anyway I'm not sure if this patch is safe. Hopefully Peter can judge
> > this better...
> >
> > > Cc: Vlastimil Babka <vbabka@suse.cz>
> > > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > > ---
> > > init/main.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/init/main.c b/init/main.c
> > > index 8b1adb6e..d1ca7cb 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -513,6 +513,7 @@ asmlinkage __visible void __init start_kernel(void)
> > > boot_cpu_state_init();
> > > smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
> > >
> > > + jump_label_init();
> > > build_all_zonelists(NULL, NULL);
> > > page_alloc_init();
> > >
> > > @@ -526,8 +527,6 @@ asmlinkage __visible void __init start_kernel(void)
> > > parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
> > > NULL, set_init_arg);
> > >
> > > - jump_label_init();
> > > -
>
> Urgh, that means auditing all archs that implement this. The thing
> you're looking for is if the self-modifying code cruft can be done that
> early.
>
> x86 looks to be fine, because this is after setup_arch() which is
> required for ideal_nops[] to be initialied and we use text_poke_early()
> which doesn't really need anything else.
>
> I've not gone through the other arches...
Vlastimil,
Will you be able to look into that? Openrisc doesnt have jump_label
support, so its no issue at the moment.
Archs that do have it:
arch/arm64/Kconfig: select HAVE_ARCH_JUMP_LABEL
arch/mips/Kconfig: select HAVE_ARCH_JUMP_LABEL
arch/s390/Kconfig: select HAVE_ARCH_JUMP_LABEL
arch/sparc/Kconfig: select HAVE_ARCH_JUMP_LABEL if SPARC64
arch/tile/Kconfig: select HAVE_ARCH_JUMP_LABEL
arch/x86/Kconfig: select HAVE_ARCH_JUMP_LABEL
arch/arm/Kconfig: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
arch/powerpc/Kconfig: select HAVE_ARCH_JUMP_LABEL
I looked at a few (arm, tile) and I dont see their arch_jump_label_transform*
implementations depending on global state like ideal_nops from x86. They
should be ok.
If no time, Should you change your patch to not use static keys for
build_all_zonelists at least?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 14:30 ` Stafford Horne
@ 2017-01-17 16:11 ` Vlastimil Babka
2017-01-17 20:34 ` Andrew Morton
0 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2017-01-17 16:11 UTC (permalink / raw)
To: Stafford Horne, Peter Zijlstra
Cc: linux-kernel, Andrew Morton, Thomas Gleixner, Kees Cook,
Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi, Tejun Heo,
Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
On 01/17/2017 03:30 PM, Stafford Horne wrote:
> On Tue, Jan 17, 2017 at 02:44:54PM +0100, Peter Zijlstra wrote:
>> On Tue, Jan 17, 2017 at 02:07:36PM +0100, Vlastimil Babka wrote:
>>
>>> Anyway I'm not sure if this patch is safe. Hopefully Peter can judge
>>> this better...
>>>
>>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>>> Signed-off-by: Stafford Horne <shorne@gmail.com>
>>>> ---
>>>> init/main.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/init/main.c b/init/main.c
>>>> index 8b1adb6e..d1ca7cb 100644
>>>> --- a/init/main.c
>>>> +++ b/init/main.c
>>>> @@ -513,6 +513,7 @@ asmlinkage __visible void __init start_kernel(void)
>>>> boot_cpu_state_init();
>>>> smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
>>>>
>>>> + jump_label_init();
>>>> build_all_zonelists(NULL, NULL);
>>>> page_alloc_init();
>>>>
>>>> @@ -526,8 +527,6 @@ asmlinkage __visible void __init start_kernel(void)
>>>> parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
>>>> NULL, set_init_arg);
>>>>
>>>> - jump_label_init();
>>>> -
>>
>> Urgh, that means auditing all archs that implement this. The thing
>> you're looking for is if the self-modifying code cruft can be done that
>> early.
>>
>> x86 looks to be fine, because this is after setup_arch() which is
>> required for ideal_nops[] to be initialied and we use text_poke_early()
>> which doesn't really need anything else.
>>
>> I've not gone through the other arches...
>
> Vlastimil,
>
> Will you be able to look into that? Openrisc doesnt have jump_label
> support, so its no issue at the moment.
>
> Archs that do have it:
>
> arch/arm64/Kconfig: select HAVE_ARCH_JUMP_LABEL
> arch/mips/Kconfig: select HAVE_ARCH_JUMP_LABEL
> arch/s390/Kconfig: select HAVE_ARCH_JUMP_LABEL
> arch/sparc/Kconfig: select HAVE_ARCH_JUMP_LABEL if SPARC64
> arch/tile/Kconfig: select HAVE_ARCH_JUMP_LABEL
> arch/x86/Kconfig: select HAVE_ARCH_JUMP_LABEL
> arch/arm/Kconfig: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> arch/powerpc/Kconfig: select HAVE_ARCH_JUMP_LABEL
>
> I looked at a few (arm, tile) and I dont see their arch_jump_label_transform*
> implementations depending on global state like ideal_nops from x86. They
> should be ok.
Thanks, I'll try.
> If no time, Should you change your patch to not use static keys for
> build_all_zonelists at least?
Yes that would be uglier but possible if I find issues or I'm not
confident enough with the auditing...
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 16:11 ` Vlastimil Babka
@ 2017-01-17 20:34 ` Andrew Morton
2017-01-17 20:49 ` Vlastimil Babka
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2017-01-17 20:34 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Stafford Horne, Peter Zijlstra, linux-kernel, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
> >
> > Will you be able to look into that? Openrisc doesnt have jump_label
> > support, so its no issue at the moment.
> >
> > Archs that do have it:
> >
> > arch/arm64/Kconfig: select HAVE_ARCH_JUMP_LABEL
> > arch/mips/Kconfig: select HAVE_ARCH_JUMP_LABEL
> > arch/s390/Kconfig: select HAVE_ARCH_JUMP_LABEL
> > arch/sparc/Kconfig: select HAVE_ARCH_JUMP_LABEL if SPARC64
> > arch/tile/Kconfig: select HAVE_ARCH_JUMP_LABEL
> > arch/x86/Kconfig: select HAVE_ARCH_JUMP_LABEL
> > arch/arm/Kconfig: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> > arch/powerpc/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >
> > I looked at a few (arm, tile) and I dont see their arch_jump_label_transform*
> > implementations depending on global state like ideal_nops from x86. They
> > should be ok.
>
> Thanks, I'll try.
>
> > If no time, Should you change your patch to not use static keys for
> > build_all_zonelists at least?
>
> Yes that would be uglier but possible if I find issues or I'm not
> confident enough with the auditing...
We could just revert f5adbdff6a1c40e19 ("mm, page_alloc: convert
page_group_by_mobility_disable to static key")?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 20:34 ` Andrew Morton
@ 2017-01-17 20:49 ` Vlastimil Babka
2017-01-19 8:28 ` Vlastimil Babka
0 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2017-01-17 20:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Stafford Horne, Peter Zijlstra, linux-kernel, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
On 17.1.2017 21:34, Andrew Morton wrote:
>>>
>>> Will you be able to look into that? Openrisc doesnt have jump_label
>>> support, so its no issue at the moment.
>>>
>>> Archs that do have it:
>>>
>>> arch/arm64/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>> arch/mips/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>> arch/s390/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>> arch/sparc/Kconfig: select HAVE_ARCH_JUMP_LABEL if SPARC64
>>> arch/tile/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>> arch/x86/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>> arch/arm/Kconfig: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
>>> arch/powerpc/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>
>>> I looked at a few (arm, tile) and I dont see their arch_jump_label_transform*
>>> implementations depending on global state like ideal_nops from x86. They
>>> should be ok.
>>
>> Thanks, I'll try.
>>
>>> If no time, Should you change your patch to not use static keys for
>>> build_all_zonelists at least?
>>
>> Yes that would be uglier but possible if I find issues or I'm not
>> confident enough with the auditing...
>
> We could just revert f5adbdff6a1c40e19 ("mm, page_alloc: convert
> page_group_by_mobility_disable to static key")?
That's a -next commit id, as the patch is in mmotm. I'll ask for removal if I
don't have a fix soon, but if you or somebody else prefers to do that ASAP, it
can be re-added later with a fix.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 20:49 ` Vlastimil Babka
@ 2017-01-19 8:28 ` Vlastimil Babka
2017-01-19 9:56 ` Mel Gorman
0 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2017-01-19 8:28 UTC (permalink / raw)
To: Andrew Morton
Cc: Stafford Horne, Peter Zijlstra, linux-kernel, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin,
linux-mm, Mel Gorman
On 01/17/2017 09:49 PM, Vlastimil Babka wrote:
> On 17.1.2017 21:34, Andrew Morton wrote:
>>>>
>>>> Will you be able to look into that? Openrisc doesnt have jump_label
>>>> support, so its no issue at the moment.
>>>>
>>>> Archs that do have it:
>>>>
>>>> arch/arm64/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>> arch/mips/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>> arch/s390/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>> arch/sparc/Kconfig: select HAVE_ARCH_JUMP_LABEL if SPARC64
>>>> arch/tile/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>> arch/x86/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>> arch/arm/Kconfig: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
>>>> arch/powerpc/Kconfig: select HAVE_ARCH_JUMP_LABEL
>>>>
>>>> I looked at a few (arm, tile) and I dont see their arch_jump_label_transform*
>>>> implementations depending on global state like ideal_nops from x86. They
>>>> should be ok.
>>>
>>> Thanks, I'll try.
>>>
>>>> If no time, Should you change your patch to not use static keys for
>>>> build_all_zonelists at least?
>>>
>>> Yes that would be uglier but possible if I find issues or I'm not
>>> confident enough with the auditing...
>>
>> We could just revert f5adbdff6a1c40e19 ("mm, page_alloc: convert
>> page_group_by_mobility_disable to static key")?
>
> That's a -next commit id, as the patch is in mmotm. I'll ask for removal if I
> don't have a fix soon, but if you or somebody else prefers to do that ASAP, it
> can be re-added later with a fix.
OK I think that we just drop the patch [1] from mmotm. Mel told me the
benefit was marginal, and also the last move of jump_label_init() caused
problems for several releases.
Thanks,
Vlastimil
[1] mm-page_alloc-convert-page_group_by_mobility_disable-to-static-key.patch
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-19 8:28 ` Vlastimil Babka
@ 2017-01-19 9:56 ` Mel Gorman
0 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2017-01-19 9:56 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Stafford Horne, Peter Zijlstra, linux-kernel,
Thomas Gleixner, Kees Cook, Jessica Yu, Petr Mladek,
Rasmus Villemoes, Yang Shi, Tejun Heo, Prarit Bhargava,
Yaowei Bai, Andrey Ryabinin, linux-mm
On Thu, Jan 19, 2017 at 09:28:28AM +0100, Vlastimil Babka wrote:
> On 01/17/2017 09:49 PM, Vlastimil Babka wrote:
> > On 17.1.2017 21:34, Andrew Morton wrote:
> >>>>
> >>>> Will you be able to look into that? Openrisc doesnt have jump_label
> >>>> support, so its no issue at the moment.
> >>>>
> >>>> Archs that do have it:
> >>>>
> >>>> arch/arm64/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >>>> arch/mips/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >>>> arch/s390/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >>>> arch/sparc/Kconfig: select HAVE_ARCH_JUMP_LABEL if SPARC64
> >>>> arch/tile/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >>>> arch/x86/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >>>> arch/arm/Kconfig: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> >>>> arch/powerpc/Kconfig: select HAVE_ARCH_JUMP_LABEL
> >>>>
> >>>> I looked at a few (arm, tile) and I dont see their arch_jump_label_transform*
> >>>> implementations depending on global state like ideal_nops from x86. They
> >>>> should be ok.
> >>>
> >>> Thanks, I'll try.
> >>>
> >>>> If no time, Should you change your patch to not use static keys for
> >>>> build_all_zonelists at least?
> >>>
> >>> Yes that would be uglier but possible if I find issues or I'm not
> >>> confident enough with the auditing...
> >>
> >> We could just revert f5adbdff6a1c40e19 ("mm, page_alloc: convert
> >> page_group_by_mobility_disable to static key")?
> >
> > That's a -next commit id, as the patch is in mmotm. I'll ask for removal if I
> > don't have a fix soon, but if you or somebody else prefers to do that ASAP, it
> > can be re-added later with a fix.
>
> OK I think that we just drop the patch [1] from mmotm. Mel told me the
> benefit was marginal, and also the last move of jump_label_init() caused
> problems for several releases.
>
Note that it's not guaranteed to cause any problems this time. If
jump_label_init can go ahead without the page allocator being fully up
and running then it may be ok.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -next] init/main: Init jump_labels before they are used to build zonelists
2017-01-17 13:44 ` Peter Zijlstra
2017-01-17 14:30 ` Stafford Horne
@ 2017-01-23 5:54 ` Michael Ellerman
1 sibling, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2017-01-23 5:54 UTC (permalink / raw)
To: Peter Zijlstra, Vlastimil Babka
Cc: Stafford Horne, linux-kernel, Andrew Morton, Thomas Gleixner,
Kees Cook, Jessica Yu, Petr Mladek, Rasmus Villemoes, Yang Shi,
Tejun Heo, Prarit Bhargava, Yaowei Bai, Andrey Ryabinin
Peter Zijlstra <peterz@infradead.org> writes:
> On Tue, Jan 17, 2017 at 02:07:36PM +0100, Vlastimil Babka wrote:
>
>> Anyway I'm not sure if this patch is safe. Hopefully Peter can judge
>> this better...
>>
>> > Cc: Vlastimil Babka <vbabka@suse.cz>
>> > Signed-off-by: Stafford Horne <shorne@gmail.com>
>> > ---
>> > init/main.c | 3 +--
>> > 1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>> > diff --git a/init/main.c b/init/main.c
>> > index 8b1adb6e..d1ca7cb 100644
>> > --- a/init/main.c
>> > +++ b/init/main.c
>> > @@ -513,6 +513,7 @@ asmlinkage __visible void __init start_kernel(void)
>> > boot_cpu_state_init();
>> > smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
>> >
>> > + jump_label_init();
>> > build_all_zonelists(NULL, NULL);
>> > page_alloc_init();
>> >
>> > @@ -526,8 +527,6 @@ asmlinkage __visible void __init start_kernel(void)
>> > parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
>> > NULL, set_init_arg);
>> >
>> > - jump_label_init();
>> > -
>
> Urgh, that means auditing all archs that implement this. The thing
> you're looking for is if the self-modifying code cruft can be done that
> early.
You could do what we do on powerpc, which is to call jump_label_init()
early in arch code.
The second call from generic code will just return without doing
anything, see the start of jump_label_init():
void __init jump_label_init(void)
{
struct jump_entry *iter_start = __start___jump_table;
struct jump_entry *iter_stop = __stop___jump_table;
struct static_key *key = NULL;
struct jump_entry *iter;
/*
* Since we are initializing the static_key.enabled field with
* with the 'raw' int values (to avoid pulling in atomic.h) in
* jump_label.h, let's make sure that is safe. There are only two
* cases to check since we initialize to 0 or 1.
*/
BUILD_BUG_ON((int)ATOMIC_INIT(0) != 0);
BUILD_BUG_ON((int)ATOMIC_INIT(1) != 1);
if (static_key_initialized)
return;
cheers
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-01-23 5:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-17 12:56 [PATCH -next] init/main: Init jump_labels before they are used to build zonelists Stafford Horne
2017-01-17 13:07 ` Vlastimil Babka
2017-01-17 13:44 ` Peter Zijlstra
2017-01-17 14:30 ` Stafford Horne
2017-01-17 16:11 ` Vlastimil Babka
2017-01-17 20:34 ` Andrew Morton
2017-01-17 20:49 ` Vlastimil Babka
2017-01-19 8:28 ` Vlastimil Babka
2017-01-19 9:56 ` Mel Gorman
2017-01-23 5:54 ` Michael Ellerman
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).