Re: [PATCH 04/13] powerpc/rtas: avoid scheduling in rtas_os_term()
On Sat Nov 19, 2022 at 1:07 AM AEST, Nathan Lynch wrote:
> It's unsafe to use rtas_busy_delay() to handle a busy status from
> the ibm,os-term RTAS function in rtas_os_term():
>
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
> BUG: sleeping function called from invalid context at
> arch/powerpc/kernel/rtas.c:618
> in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
> preempt_count: 2, expected: 0
> CPU: 7 PID: 1 Comm: swapper/0 Tainted: G D
> 6.0.0-rc5-02182-gf8553a572277-dirty #9
> Call Trace:
> [c7b8f000] [c1337110] dump_stack_lvl+0xb4/0x110 (unreliable)
> [c7b8f040] [c02440e4] __might_resched+0x394/0x3c0
> [c7b8f0e0] [c004f680] rtas_busy_delay+0x120/0x1b0
> [c7b8f100] [c0052d04] rtas_os_term+0xb8/0xf4
> [c7b8f180] [c01150fc] pseries_panic+0x50/0x68
> [c7b8f1f0] [c0036354] ppc_panic_platform_handler+0x34/0x50
> [c7b8f210] [c02303c4] notifier_call_chain+0xd4/0x1c0
> [c7b8f2b0] [c02306cc] atomic_notifier_call_chain+0xac/0x1c0
> [c7b8f2f0] [c01d62b8] panic+0x228/0x4d0
> [c7b8f390] [c01e573c] do_exit+0x140c/0x1420
> [c7b8f480] [c01e586c] make_task_dead+0xdc/0x200
>
> Use rtas_busy_delay_time() instead, which signals without side effects
> whether to attempt the ibm,os-term RTAS call again.
rtas_busy_delay should probably be renamed to rtas_busy_sleep, to make
that self-documenting that it can schedule. You could then add a
rtas_busy_delay which doesn't sleep, which a few other places could
use...
But that's a bigger chance and there is precedent for using this call
this way, so looks okay to me. Maybe you could open-code an mdelay
though, although I guess firmware should be tolerant of calling it in
a loop.
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Nathan Lynch
> ---
> arch/powerpc/kernel/rtas.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 81e4996012b7..51f0508593a7 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -965,10 +965,15 @@ void rtas_os_term(char *str)
>
> snprintf(rtas_os_term_buf, 2048, "OS panic: %s", str);
>
> + /*
> + * Keep calling as long as RTAS returns a "try again" status,
> + * but don't use rtas_busy_delay(), which potentially
> + * schedules.
> + */
> do {
> status = rtas_call(ibm_os_term_token, 1, 1, NULL,
> __pa(rtas_os_term_buf));
> - } while (rtas_busy_delay(status));
> + } while (rtas_busy_delay_time(status));
>
> if (status != 0)
> printk(KERN_EMERG "ibm,os-term call failed %d\n", status);
> --
> 2.37.1
Re: [PATCH 04/13] powerpc/rtas: avoid scheduling in rtas_os_term()
On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote:
> It's unsafe to use rtas_busy_delay() to handle a busy status from
> the ibm,os-term RTAS function in rtas_os_term():
>
> Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x000b
> BUG: sleeping function called from invalid context at
> arch/powerpc/kernel/rtas.c:618
> in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1, name:
> swapper/0
> preempt_count: 2, expected: 0
> CPU: 7 PID: 1 Comm: swapper/0 Tainted: G D 6.0.0-rc5-
> 02182-gf8553a572277-dirty #9
> Call Trace:
> [c7b8f000] [c1337110] dump_stack_lvl+0xb4/0x110
> (unreliable)
> [c7b8f040] [c02440e4] __might_resched+0x394/0x3c0
> [c7b8f0e0] [c004f680] rtas_busy_delay+0x120/0x1b0
> [c7b8f100] [c0052d04] rtas_os_term+0xb8/0xf4
> [c7b8f180] [c01150fc] pseries_panic+0x50/0x68
> [c7b8f1f0] [c0036354]
> ppc_panic_platform_handler+0x34/0x50
> [c7b8f210] [c02303c4] notifier_call_chain+0xd4/0x1c0
> [c7b8f2b0] [c02306cc]
> atomic_notifier_call_chain+0xac/0x1c0
> [c7b8f2f0] [c01d62b8] panic+0x228/0x4d0
> [c7b8f390] [c01e573c] do_exit+0x140c/0x1420
> [c7b8f480] [c01e586c] make_task_dead+0xdc/0x200
>
> Use rtas_busy_delay_time() instead, which signals without side
> effects
> whether to attempt the ibm,os-term RTAS call again.
>
> Signed-off-by: Nathan Lynch
Makes sense.
Reviewed-by: Andrew Donnellan
> ---
> arch/powerpc/kernel/rtas.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 81e4996012b7..51f0508593a7 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -965,10 +965,15 @@ void rtas_os_term(char *str)
>
> snprintf(rtas_os_term_buf, 2048, "OS panic: %s", str);
>
> + /*
> + * Keep calling as long as RTAS returns a "try again" status,
> + * but don't use rtas_busy_delay(), which potentially
> + * schedules.
> + */
> do {
> status = rtas_call(ibm_os_term_token, 1, 1, NULL,
> __pa(rtas_os_term_buf));
> - } while (rtas_busy_delay(status));
> + } while (rtas_busy_delay_time(status));
>
> if (status != 0)
> printk(KERN_EMERG "ibm,os-term call failed %d\n",
> status);
--
Andrew DonnellanOzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
[PATCH 04/13] powerpc/rtas: avoid scheduling in rtas_os_term()
It's unsafe to use rtas_busy_delay() to handle a busy status from
the ibm,os-term RTAS function in rtas_os_term():
Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
BUG: sleeping function called from invalid context at
arch/powerpc/kernel/rtas.c:618
in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
preempt_count: 2, expected: 0
CPU: 7 PID: 1 Comm: swapper/0 Tainted: G D
6.0.0-rc5-02182-gf8553a572277-dirty #9
Call Trace:
[c7b8f000] [c1337110] dump_stack_lvl+0xb4/0x110 (unreliable)
[c7b8f040] [c02440e4] __might_resched+0x394/0x3c0
[c7b8f0e0] [c004f680] rtas_busy_delay+0x120/0x1b0
[c7b8f100] [c0052d04] rtas_os_term+0xb8/0xf4
[c7b8f180] [c01150fc] pseries_panic+0x50/0x68
[c7b8f1f0] [c0036354] ppc_panic_platform_handler+0x34/0x50
[c7b8f210] [c02303c4] notifier_call_chain+0xd4/0x1c0
[c7b8f2b0] [c02306cc] atomic_notifier_call_chain+0xac/0x1c0
[c7b8f2f0] [c01d62b8] panic+0x228/0x4d0
[c7b8f390] [c01e573c] do_exit+0x140c/0x1420
[c7b8f480] [c01e586c] make_task_dead+0xdc/0x200
Use rtas_busy_delay_time() instead, which signals without side effects
whether to attempt the ibm,os-term RTAS call again.
Signed-off-by: Nathan Lynch
---
arch/powerpc/kernel/rtas.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 81e4996012b7..51f0508593a7 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -965,10 +965,15 @@ void rtas_os_term(char *str)
snprintf(rtas_os_term_buf, 2048, "OS panic: %s", str);
+ /*
+* Keep calling as long as RTAS returns a "try again" status,
+* but don't use rtas_busy_delay(), which potentially
+* schedules.
+*/
do {
status = rtas_call(ibm_os_term_token, 1, 1, NULL,
__pa(rtas_os_term_buf));
- } while (rtas_busy_delay(status));
+ } while (rtas_busy_delay_time(status));
if (status != 0)
printk(KERN_EMERG "ibm,os-term call failed %d\n", status);
--
2.37.1
