public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: "Nicholas Piggin" <npiggin@gmail•com>
To: "Nathan Lynch" <nathanl@linux•ibm.com>, <linuxppc-dev@lists•ozlabs.org>
Cc: tyreld@linux•ibm.com, nnac123@linux•ibm.com,
	ldufour@linux•ibm.com, ajd@linux•ibm.com
Subject: Re: [PATCH 04/13] powerpc/rtas: avoid scheduling in rtas_os_term()
Date: Mon, 28 Nov 2022 12:34:55 +1000	[thread overview]
Message-ID: <CONKTXD6H622.AK0NSD7PPNG@bobo> (raw)
In-Reply-To: <20221118150751.469393-5-nathanl@linux.ibm.com>

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


  parent reply	other threads:[~2022-11-28  2:35 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-18 15:07 [PATCH 00/13] RTAS maintenance Nathan Lynch
2022-11-18 15:07 ` [PATCH 01/13] powerpc/rtas: document rtas_call() Nathan Lynch
2022-11-22  2:46   ` Andrew Donnellan
2022-11-18 15:07 ` [PATCH 02/13] powerpc/rtasd: use correct OF API for event scan rate Nathan Lynch
2022-11-22  2:39   ` Andrew Donnellan
2022-11-18 15:07 ` [PATCH 03/13] powerpc/rtas: avoid device tree lookups in rtas_os_term() Nathan Lynch
2022-11-22  3:03   ` Andrew Donnellan
2022-11-28 18:08     ` Nathan Lynch
2022-11-28  2:29   ` Nicholas Piggin
2022-11-28 18:26     ` Nathan Lynch
2022-11-29  6:45       ` Nicholas Piggin
2022-11-29 15:37         ` Nathan Lynch
2022-11-18 15:07 ` [PATCH 04/13] powerpc/rtas: avoid scheduling " Nathan Lynch
2022-11-22  3:17   ` Andrew Donnellan
2022-11-28  2:34   ` Nicholas Piggin [this message]
2022-11-18 15:07 ` [PATCH 05/13] powerpc/pseries/eeh: use correct API for error log size Nathan Lynch
2022-11-22  3:21   ` Andrew Donnellan
2022-11-28 20:22     ` Nathan Lynch
2022-11-18 15:07 ` [PATCH 06/13] powerpc/rtas: clean up rtas_error_log_max initialization Nathan Lynch
2022-11-22  3:40   ` Andrew Donnellan
2022-11-18 15:07 ` [PATCH 07/13] powerpc/rtas: clean up includes Nathan Lynch
2022-11-22  4:45   ` Andrew Donnellan
2022-11-18 15:07 ` [PATCH 08/13] powerpc/rtas: define pr_fmt and convert printk call sites Nathan Lynch
2022-11-22  4:07   ` Andrew Donnellan
2022-11-18 15:07 ` [PATCH 09/13] powerpc/rtas: mandate RTAS syscall filtering Nathan Lynch
2022-11-22  4:20   ` Andrew Donnellan
2022-11-18 15:07 ` [PATCH 10/13] powerpc/rtas: improve function information lookups Nathan Lynch
2022-11-23  2:51   ` Andrew Donnellan
2022-11-23 19:32     ` Nick Child
2022-11-24  3:28       ` Andrew Donnellan
2022-11-28 21:19         ` Nathan Lynch
2022-11-29  0:08     ` Nathan Lynch
2022-11-29  7:23       ` Andrew Donnellan
2022-11-29 15:33         ` Nathan Lynch
2022-11-23 20:06   ` Nick Child
2022-11-28 21:57     ` Nathan Lynch
2022-11-18 15:07 ` [PATCH 11/13] powerpc/rtas: strengthen do_enter_rtas() type safety, drop inline Nathan Lynch
2022-11-23  3:23   ` Andrew Donnellan
2022-11-28  2:37   ` Nicholas Piggin
2022-11-18 15:07 ` [PATCH 12/13] powerpc/tracing: tracepoints for RTAS entry and exit Nathan Lynch
2022-11-28  2:54   ` Nicholas Piggin
2022-11-18 15:07 ` [PATCH 13/13] powerpc/rtas: place tracepoints in do_enter_rtas() Nathan Lynch
2022-11-28  3:07   ` Nicholas Piggin
2022-11-28 23:44     ` Nathan Lynch
2022-11-29  0:49       ` Michael Ellerman
2022-11-29 20:24         ` Nathan Lynch
2022-11-30  7:43           ` Michael Ellerman
2022-11-29  6:47       ` Nicholas Piggin
2022-12-08 12:40 ` [PATCH 00/13] RTAS maintenance Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CONKTXD6H622.AK0NSD7PPNG@bobo \
    --to=npiggin@gmail$(echo .)com \
    --cc=ajd@linux$(echo .)ibm.com \
    --cc=ldufour@linux$(echo .)ibm.com \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=nathanl@linux$(echo .)ibm.com \
    --cc=nnac123@linux$(echo .)ibm.com \
    --cc=tyreld@linux$(echo .)ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox