1. Patchew
  2. linux
  3. View series

[PATCH] m68k: Initialize jump labels early during setup_arch()

Jean-Michel Hautbois posted 1 patch 1 month, 1 week ago 
arch/m68k/kernel/setup_mm.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH] m68k: Initialize jump labels early during setup_arch()
Posted by Jean-Michel Hautbois 1 month, 1 week ago 
The jump_label_init() should be called from setup_arch() very
early for proper functioning of jump label support.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 arch/m68k/kernel/setup_mm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/kernel/setup_mm.c b/arch/m68k/kernel/setup_mm.c
index 10310b04f77d8d79bec858c6989c2cf21d0af557..15c1a595a1de0bef7e6565b7a6e02d773c23bb8c 100644
--- a/arch/m68k/kernel/setup_mm.c
+++ b/arch/m68k/kernel/setup_mm.c
@@ -249,7 +249,11 @@ void __init setup_arch(char **cmdline_p)
 	process_uboot_commandline(&m68k_command_line[0], CL_SIZE);
 	*cmdline_p = m68k_command_line;
 	memcpy(boot_command_line, *cmdline_p, CL_SIZE);
-
+	/*
+	 * Initialise the static keys early as they may be enabled by the
+	 * cpufeature code and early parameters.
+	 */
+	jump_label_init();
 	parse_early_param();
 
 	switch (m68k_machtype) {

---
base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354
change-id: 20241016-fix-jump-label-c083e90cddc6

Best regards,
-- 
Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
Re: [PATCH] m68k: Initialize jump labels early during setup_arch()
Posted by Geert Uytterhoeven 3 weeks ago 
Hi Jean-Michel,

On Wed, Oct 16, 2024 at 6:18 PM Jean-Michel Hautbois
<jeanmichel.hautbois@yoseli.org> wrote:
> The jump_label_init() should be called from setup_arch() very
> early for proper functioning of jump label support.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Thanks for your patch!

> --- a/arch/m68k/kernel/setup_mm.c
> +++ b/arch/m68k/kernel/setup_mm.c
> @@ -249,7 +249,11 @@ void __init setup_arch(char **cmdline_p)
>         process_uboot_commandline(&m68k_command_line[0], CL_SIZE);
>         *cmdline_p = m68k_command_line;
>         memcpy(boot_command_line, *cmdline_p, CL_SIZE);
> -
> +       /*
> +        * Initialise the static keys early as they may be enabled by the
> +        * cpufeature code and early parameters.
> +        */
> +       jump_label_init();
>         parse_early_param();
>
>         switch (m68k_machtype) {

This is indeed what some (but not all) other architectures are doing, so
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

I assume you saw some "static key used before call to jump_label_init()"
warning[1]? Since I never saw such a message, can you please elaborate
and explain your use case, so I can add that to the patch description
when applying?

Thanks!

[1] https://elixir.bootlin.com/linux/v6.11.6/source/include/linux/jump_label.h#L81

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] m68k: Initialize jump labels early during setup_arch()
Posted by Jean-Michel Hautbois 3 weeks ago 
Hi Geert !

On 11/5/24 15:03, Geert Uytterhoeven wrote:
> Hi Jean-Michel,
> 
> On Wed, Oct 16, 2024 at 6:18 PM Jean-Michel Hautbois
> <jeanmichel.hautbois@yoseli.org> wrote:
>> The jump_label_init() should be called from setup_arch() very
>> early for proper functioning of jump label support.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> 
> Thanks for your patch!
> 
>> --- a/arch/m68k/kernel/setup_mm.c
>> +++ b/arch/m68k/kernel/setup_mm.c
>> @@ -249,7 +249,11 @@ void __init setup_arch(char **cmdline_p)
>>          process_uboot_commandline(&m68k_command_line[0], CL_SIZE);
>>          *cmdline_p = m68k_command_line;
>>          memcpy(boot_command_line, *cmdline_p, CL_SIZE);
>> -
>> +       /*
>> +        * Initialise the static keys early as they may be enabled by the
>> +        * cpufeature code and early parameters.
>> +        */
>> +       jump_label_init();
>>          parse_early_param();
>>
>>          switch (m68k_machtype) {
> 
> This is indeed what some (but not all) other architectures are doing, so
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> I assume you saw some "static key used before call to jump_label_init()"
> warning[1]? Since I never saw such a message, can you please elaborate
> and explain your use case, so I can add that to the patch description
> when applying?

Indeed ! I saw this when I was passing the "threadirqs" parameter to the 
kernel commandline and the "select IRQ_FORCED_THREADING" line to the 
Kconfig.
I suspect this might be true for other keys.

BTW, threaded IRQs work fine ;-).

Thanks,
JM

> 
> Thanks!
> 
> [1] https://elixir.bootlin.com/linux/v6.11.6/source/include/linux/jump_label.h#L81
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
Re: [PATCH] m68k: Initialize jump labels early during setup_arch()
Posted by Geert Uytterhoeven 2 weeks, 6 days ago 
Hi Jean-Michel,

On Tue, Nov 5, 2024 at 3:56 PM Jean-Michel Hautbois
<jeanmichel.hautbois@yoseli.org> wrote:
> On 11/5/24 15:03, Geert Uytterhoeven wrote:
> > On Wed, Oct 16, 2024 at 6:18 PM Jean-Michel Hautbois
> > <jeanmichel.hautbois@yoseli.org> wrote:
> >> The jump_label_init() should be called from setup_arch() very
> >> early for proper functioning of jump label support.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> >
> > Thanks for your patch!
> >
> >> --- a/arch/m68k/kernel/setup_mm.c
> >> +++ b/arch/m68k/kernel/setup_mm.c
> >> @@ -249,7 +249,11 @@ void __init setup_arch(char **cmdline_p)
> >>          process_uboot_commandline(&m68k_command_line[0], CL_SIZE);
> >>          *cmdline_p = m68k_command_line;
> >>          memcpy(boot_command_line, *cmdline_p, CL_SIZE);
> >> -
> >> +       /*
> >> +        * Initialise the static keys early as they may be enabled by the
> >> +        * cpufeature code and early parameters.
> >> +        */
> >> +       jump_label_init();
> >>          parse_early_param();
> >>
> >>          switch (m68k_machtype) {
> >
> > This is indeed what some (but not all) other architectures are doing, so
> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >
> > I assume you saw some "static key used before call to jump_label_init()"
> > warning[1]? Since I never saw such a message, can you please elaborate
> > and explain your use case, so I can add that to the patch description
> > when applying?
>
> Indeed ! I saw this when I was passing the "threadirqs" parameter to the
> kernel commandline and the "select IRQ_FORCED_THREADING" line to the
> Kconfig.

Thanks, I can reproduce it using that method.

> I suspect this might be true for other keys.

Indeed. But m68k and its configs don't enable much code that uses keys.
The only one I found was "thread_backlog_napi". Adding that to the
kernel command line gives:

    WARNING: CPU: 0 PID: 0 at include/linux/jump_label.h:322
setup_backlog_napi_threads+0x40/0xa0
    static_key_enable(): static key '0x5ceec0' used before call to
jump_label_init()

so I'll add that to the patch description.

> BTW, threaded IRQs work fine ;-).

On Atari (ARAnyM), they blow up spectacularly:

    Unable to handle kernel NULL pointer dereference at virtual address 00000000
    Oops: 00000000
    Modules linked in:
    PC: [<00040baa>] try_to_wake_up+0x90/0x1b8
    SR: 2701  SP: (ptrval)  a2: 0055b660
    d0: 00000003    d1: 00000003    d2: 00000000    d3: 00002700
    d4: 00000008    d5: 00000000    a0: 00558000    a1: 00558000
    Process swapper (pid: 0, task=(ptrval))
    Frame format=7 eff addr=00000000 ssw=0505 faddr=00000000
    wb 1 stat/addr/data: 0000 00000000 00000000
    wb 2 stat/addr/data: 0000 00000000 00000000
    wb 3 stat/addr/data: 0000 00000000 00000000
    push data: 00000000 00000000 00000000 00000000
    Stack from 00559ea8:
    00000000 0000000f 00007874 01006140 00559ef0 00050ec6 0000000f 00559ed8
    00040ce6 00000000 00000003 00000000 00559f14 0003b9b6 00000000 00000000
    00592710 010040c0 00000000 00559ef4 00559ef4 0003ba26 00051cd2 010040c0
    ffffffff 004fc9a1 00559f2c 00559ff8 00050ef8 00051cd2 010040c0 ffffffff
    004fc9a1 0000000f 004f9268 00000000 00592710 010040c0 00051566 010040c0
    0000000f 00000000 00000000 0000000f 00007874 00000000 010040c0 00592710
    Call Trace: [<00007874>] stdma_int+0x0/0x1c
     [<00050ec6>] setup_irq_thread+0x0/0x98
     [<00040ce6>] wake_up_process+0x14/0x18
     [<0003b9b6>] __kthread_create_on_node+0xca/0x11e
     [<0003ba26>] kthread_create_on_node+0x1c/0x26
     [<00051cd2>] irq_thread+0x0/0x184
     [<00050ef8>] setup_irq_thread+0x32/0x98
     [<00051cd2>] irq_thread+0x0/0x184
     [<00051566>] __setup_irq+0x172/0x582
     [<00007874>] stdma_int+0x0/0x1c
     [<00051a34>] request_threaded_irq+0xbe/0x14e
     [<0003ab86>] parse_args+0x0/0x202
     [<0000556e>] m68k_setup_irq_controller+0x0/0x48
     [<00255bda>] __is_suppressed_warning+0x0/0x42
     [<005da080>] stdma_init+0x2a/0x42
     [<00007874>] stdma_int+0x0/0x1c
     [<00200080>] ksys_shmctl+0x420/0x5f8
     [<00007874>] stdma_int+0x0/0x1c
     [<005d9f82>] atari_init_IRQ+0xc0/0x194
     [<00412cc8>] _printk+0x0/0x18
     [<00255bda>] __is_suppressed_warning+0x0/0x42
     [<005d62b0>] start_kernel+0x40e/0x618
     [<005d5344>] _sinittext+0x344/0x9e8

    Code: 0008 5281 2341 0008 b6fc fc3c 6700 011e <2213> c280 6700
0108 202b 0018 4bf9 0004 05a6 6700 0096 486e fffc 2f0b 4eb9 0004
    Disabling lock debugging due to kernel taint
    Kernel panic - not syncing: Attempted to kill the idle task!
    ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] m68k: Initialize jump labels early during setup_arch()
Posted by Jean-Michel Hautbois 2 weeks, 6 days ago 
Hi Geert,

On 11/5/24 17:04, Geert Uytterhoeven wrote:
> Hi Jean-Michel,
> 
> On Tue, Nov 5, 2024 at 3:56 PM Jean-Michel Hautbois
> <jeanmichel.hautbois@yoseli.org> wrote:
>> On 11/5/24 15:03, Geert Uytterhoeven wrote:
>>> On Wed, Oct 16, 2024 at 6:18 PM Jean-Michel Hautbois
>>> <jeanmichel.hautbois@yoseli.org> wrote:
>>>> The jump_label_init() should be called from setup_arch() very
>>>> early for proper functioning of jump label support.
>>>>
>>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>>>
>>> Thanks for your patch!
>>>
>>>> --- a/arch/m68k/kernel/setup_mm.c
>>>> +++ b/arch/m68k/kernel/setup_mm.c
>>>> @@ -249,7 +249,11 @@ void __init setup_arch(char **cmdline_p)
>>>>           process_uboot_commandline(&m68k_command_line[0], CL_SIZE);
>>>>           *cmdline_p = m68k_command_line;
>>>>           memcpy(boot_command_line, *cmdline_p, CL_SIZE);
>>>> -
>>>> +       /*
>>>> +        * Initialise the static keys early as they may be enabled by the
>>>> +        * cpufeature code and early parameters.
>>>> +        */
>>>> +       jump_label_init();
>>>>           parse_early_param();
>>>>
>>>>           switch (m68k_machtype) {
>>>
>>> This is indeed what some (but not all) other architectures are doing, so
>>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>>
>>> I assume you saw some "static key used before call to jump_label_init()"
>>> warning[1]? Since I never saw such a message, can you please elaborate
>>> and explain your use case, so I can add that to the patch description
>>> when applying?
>>
>> Indeed ! I saw this when I was passing the "threadirqs" parameter to the
>> kernel commandline and the "select IRQ_FORCED_THREADING" line to the
>> Kconfig.
> 
> Thanks, I can reproduce it using that method.
> 
>> I suspect this might be true for other keys.
> 
> Indeed. But m68k and its configs don't enable much code that uses keys.
> The only one I found was "thread_backlog_napi". Adding that to the
> kernel command line gives:
> 
>      WARNING: CPU: 0 PID: 0 at include/linux/jump_label.h:322
> setup_backlog_napi_threads+0x40/0xa0
>      static_key_enable(): static key '0x5ceec0' used before call to
> jump_label_init()
> 
> so I'll add that to the patch description.

Great news !

> 
>> BTW, threaded IRQs work fine ;-).
> 
> On Atari (ARAnyM), they blow up spectacularly:
> 
>      Unable to handle kernel NULL pointer dereference at virtual address 00000000
>      Oops: 00000000
>      Modules linked in:
>      PC: [<00040baa>] try_to_wake_up+0x90/0x1b8
>      SR: 2701  SP: (ptrval)  a2: 0055b660
>      d0: 00000003    d1: 00000003    d2: 00000000    d3: 00002700
>      d4: 00000008    d5: 00000000    a0: 00558000    a1: 00558000
>      Process swapper (pid: 0, task=(ptrval))
>      Frame format=7 eff addr=00000000 ssw=0505 faddr=00000000
>      wb 1 stat/addr/data: 0000 00000000 00000000
>      wb 2 stat/addr/data: 0000 00000000 00000000
>      wb 3 stat/addr/data: 0000 00000000 00000000
>      push data: 00000000 00000000 00000000 00000000
>      Stack from 00559ea8:
>      00000000 0000000f 00007874 01006140 00559ef0 00050ec6 0000000f 00559ed8
>      00040ce6 00000000 00000003 00000000 00559f14 0003b9b6 00000000 00000000
>      00592710 010040c0 00000000 00559ef4 00559ef4 0003ba26 00051cd2 010040c0
>      ffffffff 004fc9a1 00559f2c 00559ff8 00050ef8 00051cd2 010040c0 ffffffff
>      004fc9a1 0000000f 004f9268 00000000 00592710 010040c0 00051566 010040c0
>      0000000f 00000000 00000000 0000000f 00007874 00000000 010040c0 00592710
>      Call Trace: [<00007874>] stdma_int+0x0/0x1c
>       [<00050ec6>] setup_irq_thread+0x0/0x98
>       [<00040ce6>] wake_up_process+0x14/0x18
>       [<0003b9b6>] __kthread_create_on_node+0xca/0x11e
>       [<0003ba26>] kthread_create_on_node+0x1c/0x26
>       [<00051cd2>] irq_thread+0x0/0x184
>       [<00050ef8>] setup_irq_thread+0x32/0x98
>       [<00051cd2>] irq_thread+0x0/0x184
>       [<00051566>] __setup_irq+0x172/0x582
>       [<00007874>] stdma_int+0x0/0x1c
>       [<00051a34>] request_threaded_irq+0xbe/0x14e
>       [<0003ab86>] parse_args+0x0/0x202
>       [<0000556e>] m68k_setup_irq_controller+0x0/0x48
>       [<00255bda>] __is_suppressed_warning+0x0/0x42
>       [<005da080>] stdma_init+0x2a/0x42
>       [<00007874>] stdma_int+0x0/0x1c
>       [<00200080>] ksys_shmctl+0x420/0x5f8
>       [<00007874>] stdma_int+0x0/0x1c
>       [<005d9f82>] atari_init_IRQ+0xc0/0x194
>       [<00412cc8>] _printk+0x0/0x18
>       [<00255bda>] __is_suppressed_warning+0x0/0x42
>       [<005d62b0>] start_kernel+0x40e/0x618
>       [<005d5344>] _sinittext+0x344/0x9e8
> 
>      Code: 0008 5281 2341 0008 b6fc fc3c 6700 011e <2213> c280 6700
> 0108 202b 0018 4bf9 0004 05a6 6700 0096 486e fffc 2f0b 4eb9 0004
>      Disabling lock debugging due to kernel taint
>      Kernel panic - not syncing: Attempted to kill the idle task!
>      ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---

I needed to make my nfc driver (vf610_nfc) be hardirq though using 
IRQF_NO_THREAD but ethernet and all work fine on coldfire.

Thanks !
JM

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.