From: Michael Ellerman <mpe@ellerman•id.au>
To: Nathan Lynch via B4 Relay
<devnull+nathanl.linux.ibm.com@kernel•org>,
Nicholas Piggin <npiggin@gmail•com>,
Christophe Leroy <christophe.leroy@csgroup•eu>
Cc: Nathan Lynch <nathanl@linux•ibm.com>,
Tyrel Datwyler <tyreld@linux•ibm.com>,
Nick Child <nnac123@linux•ibm.com>,
Andrew Donnellan <ajd@linux•ibm.com>,
Scott Cheloha <cheloha@linux•ibm.com>,
Laurent Dufour <ldufour@linux•ibm.com>,
linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH 8/8] powerpc/rtas: consume retry statuses in sys_rtas()
Date: Thu, 23 Mar 2023 20:44:20 +1100 [thread overview]
Message-ID: <87pm8zu7ij.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20230220-rtas-queue-for-6-4-v1-8-010e4416f13f@linux.ibm.com>
Nathan Lynch via B4 Relay <devnull+nathanl.linux.ibm.com@kernel•org> writes:
> From: Nathan Lynch <nathanl@linux•ibm.com>
>
> The kernel can handle retrying RTAS function calls in response to
> -2/990x in the sys_rtas() handler instead of relaying the intermediate
> status to user space.
This looks good in general.
One query ...
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 47a2aa43d7d4..c330a22ccc70 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -1798,7 +1798,6 @@ static bool block_rtas_call(int token, int nargs,
> /* We assume to be passed big endian arguments */
> SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
> {
> - struct pin_cookie cookie;
> struct rtas_args args;
> unsigned long flags;
> char *buff_copy, *errbuf = NULL;
> @@ -1866,20 +1865,25 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
>
> buff_copy = get_errorlog_buffer();
>
> - raw_spin_lock_irqsave(&rtas_lock, flags);
> - cookie = lockdep_pin_lock(&rtas_lock);
> + do {
> + struct pin_cookie cookie;
>
> - rtas_args = args;
> - do_enter_rtas(&rtas_args);
> - args = rtas_args;
> + raw_spin_lock_irqsave(&rtas_lock, flags);
> + cookie = lockdep_pin_lock(&rtas_lock);
>
> - /* A -1 return code indicates that the last command couldn't
> - be completed due to a hardware error. */
> - if (be32_to_cpu(args.rets[0]) == -1)
> - errbuf = __fetch_rtas_last_error(buff_copy);
> + rtas_args = args;
> + do_enter_rtas(&rtas_args);
> + args = rtas_args;
>
> - lockdep_unpin_lock(&rtas_lock, cookie);
> - raw_spin_unlock_irqrestore(&rtas_lock, flags);
> + /*
> + * Handle error record retrieval before releasing the lock.
> + */
> + if (be32_to_cpu(args.rets[0]) == -1)
> + errbuf = __fetch_rtas_last_error(buff_copy);
> +
> + lockdep_unpin_lock(&rtas_lock, cookie);
> + raw_spin_unlock_irqrestore(&rtas_lock, flags);
> + } while (rtas_busy_delay(be32_to_cpu(args.rets[0])));
rtas_busy_delay_early() has the successive_ext_delays case that will
break out eventually. But if we keep getting plain RTAS_BUSY back from
RTAS I *think* this loop will never terminate?
To avoid that, and just as good manners, I think we should have a
fatal_signal_pending() check, and if that returns true we bail out of
the syscall with -EINTR ?
cheers
next prev parent reply other threads:[~2023-03-23 9:45 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-06 21:33 [PATCH 0/8] RTAS changes for 6.4 Nathan Lynch via B4 Relay
2023-03-06 21:33 ` [PATCH 1/8] powerpc/rtas: ensure 8-byte alignment for struct rtas_args Nathan Lynch via B4 Relay
2023-03-23 4:00 ` Andrew Donnellan
2023-03-06 21:33 ` [PATCH 2/8] powerpc/rtas: use memmove for potentially overlapping buffer copy Nathan Lynch via B4 Relay
2023-03-23 4:09 ` Andrew Donnellan
2023-03-06 21:33 ` [PATCH 3/8] powerpc/rtas: rtas_call_unlocked() kerneldoc Nathan Lynch via B4 Relay
2023-03-23 4:15 ` Andrew Donnellan
2023-03-06 21:33 ` [PATCH 4/8] powerpc/rtas: fix miswording in rtas_function kerneldoc Nathan Lynch via B4 Relay
2023-03-23 0:17 ` Andrew Donnellan
2023-03-06 21:33 ` [PATCH 5/8] powerpc/rtas: rename va_rtas_call_unlocked() to va_rtas_call() Nathan Lynch via B4 Relay
2023-03-23 4:17 ` Andrew Donnellan
2023-03-23 16:11 ` Nathan Lynch
2023-03-29 12:24 ` Michael Ellerman
2023-03-06 21:33 ` [PATCH 6/8] powerpc/rtas: lockdep annotations Nathan Lynch via B4 Relay
2023-03-23 6:01 ` Andrew Donnellan
2023-03-06 21:33 ` [PATCH 7/8] powerpc/rtas: warn on unsafe argument to rtas_call_unlocked() Nathan Lynch via B4 Relay
2023-03-23 4:25 ` Andrew Donnellan
2023-03-23 12:17 ` Nathan Lynch
2023-03-24 0:56 ` Nathan Lynch
2023-03-29 12:20 ` Michael Ellerman
2023-03-29 16:23 ` Nathan Lynch
2023-03-06 21:33 ` [PATCH 8/8] powerpc/rtas: consume retry statuses in sys_rtas() Nathan Lynch via B4 Relay
2023-03-23 6:26 ` Andrew Donnellan
2023-03-23 19:39 ` Nathan Lynch
2023-03-23 9:44 ` Michael Ellerman [this message]
2023-03-23 13:40 ` Nathan Lynch
2024-01-25 15:55 ` Christophe Leroy
2024-01-25 16:33 ` Nathan Lynch
2024-01-25 16:46 ` Christophe Leroy
2024-01-25 17:23 ` Nathan Lynch
2023-04-06 1:09 ` (subset) [PATCH 0/8] RTAS changes for 6.4 Michael Ellerman
2023-04-26 12:12 ` 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=87pm8zu7ij.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=ajd@linux$(echo .)ibm.com \
--cc=cheloha@linux$(echo .)ibm.com \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=devnull+nathanl.linux.ibm.com@kernel$(echo .)org \
--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=npiggin@gmail$(echo .)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