| Message ID | 20221118150751.469393-5-nathanl@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6c606e57eecc37d6b36d732b1ff7e55b7dc32dd4 |
| Headers | show |
| Series | RTAS maintenance | expand |
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=0x0000000b > 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: > [c000000007b8f000] [c000000001337110] dump_stack_lvl+0xb4/0x110 > (unreliable) > [c000000007b8f040] [c0000000002440e4] __might_resched+0x394/0x3c0 > [c000000007b8f0e0] [c00000000004f680] rtas_busy_delay+0x120/0x1b0 > [c000000007b8f100] [c000000000052d04] rtas_os_term+0xb8/0xf4 > [c000000007b8f180] [c0000000001150fc] pseries_panic+0x50/0x68 > [c000000007b8f1f0] [c000000000036354] > ppc_panic_platform_handler+0x34/0x50 > [c000000007b8f210] [c0000000002303c4] notifier_call_chain+0xd4/0x1c0 > [c000000007b8f2b0] [c0000000002306cc] > atomic_notifier_call_chain+0xac/0x1c0 > [c000000007b8f2f0] [c0000000001d62b8] panic+0x228/0x4d0 > [c000000007b8f390] [c0000000001e573c] do_exit+0x140c/0x1420 > [c000000007b8f480] [c0000000001e586c] 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 <nathanl@linux.ibm.com> Makes sense. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > --- > 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);
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=0x0000000b > 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: > [c000000007b8f000] [c000000001337110] dump_stack_lvl+0xb4/0x110 (unreliable) > [c000000007b8f040] [c0000000002440e4] __might_resched+0x394/0x3c0 > [c000000007b8f0e0] [c00000000004f680] rtas_busy_delay+0x120/0x1b0 > [c000000007b8f100] [c000000000052d04] rtas_os_term+0xb8/0xf4 > [c000000007b8f180] [c0000000001150fc] pseries_panic+0x50/0x68 > [c000000007b8f1f0] [c000000000036354] ppc_panic_platform_handler+0x34/0x50 > [c000000007b8f210] [c0000000002303c4] notifier_call_chain+0xd4/0x1c0 > [c000000007b8f2b0] [c0000000002306cc] atomic_notifier_call_chain+0xac/0x1c0 > [c000000007b8f2f0] [c0000000001d62b8] panic+0x228/0x4d0 > [c000000007b8f390] [c0000000001e573c] do_exit+0x140c/0x1420 > [c000000007b8f480] [c0000000001e586c] 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 <npiggin@gmail.com> > > Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com> > --- > 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
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);
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=0x0000000b 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: [c000000007b8f000] [c000000001337110] dump_stack_lvl+0xb4/0x110 (unreliable) [c000000007b8f040] [c0000000002440e4] __might_resched+0x394/0x3c0 [c000000007b8f0e0] [c00000000004f680] rtas_busy_delay+0x120/0x1b0 [c000000007b8f100] [c000000000052d04] rtas_os_term+0xb8/0xf4 [c000000007b8f180] [c0000000001150fc] pseries_panic+0x50/0x68 [c000000007b8f1f0] [c000000000036354] ppc_panic_platform_handler+0x34/0x50 [c000000007b8f210] [c0000000002303c4] notifier_call_chain+0xd4/0x1c0 [c000000007b8f2b0] [c0000000002306cc] atomic_notifier_call_chain+0xac/0x1c0 [c000000007b8f2f0] [c0000000001d62b8] panic+0x228/0x4d0 [c000000007b8f390] [c0000000001e573c] do_exit+0x140c/0x1420 [c000000007b8f480] [c0000000001e586c] 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 <nathanl@linux.ibm.com> --- arch/powerpc/kernel/rtas.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)