All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Vincent Chen <vincent.chen@sifive.com>,
	Guo Ren <guoren@linux.alibaba.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	linux-kselftest <linux-kselftest@vger.kernel.org>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH V2 2/3] riscv: Add support for restartable sequence
Date: Mon, 19 Jul 2021 10:43:10 -0400 (EDT)	[thread overview]
Message-ID: <1257037909.25426.1626705790861.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <1583733592-22873-3-git-send-email-vincent.chen@sifive.com>

----- On Mar 9, 2020, at 1:59 AM, Vincent Chen vincent.chen@sifive.com wrote:
[...]
> --- a/arch/riscv/kernel/signal.c
> +++ b/arch/riscv/kernel/signal.c
> @@ -234,6 +234,7 @@ static void handle_signal(struct ksignal *ksig, struct
> pt_regs *regs)
> 	sigset_t *oldset = sigmask_to_save();
> 	int ret;
> 
> +	rseq_signal_deliver(ksig, regs);
> 	/* Are we from a system call? */
> 	if (regs->cause == EXC_SYSCALL) {

[...]

As Al Viro pointed out on IRC, the rseq_signal_deliver() should go after syscall
restart handling, similarly to what is done on every other supported architecture.

Note that there is already an upstream commit derived on this non-upstream patch:

commit 9866d141a097 ("csky: Add support for restartable sequence")

which is broken in the same way.

I'm not sure why I was never CC'd on the csky patch. Considering that nobody
bothered to implement the rseq selftests for csky, I don't see how any of
this is tested. I would favor a revert of that commit until the testing glue
is contributed. Unfortunately, the csky commit has been upstream since v5.7.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Vincent Chen <vincent.chen@sifive.com>,
	Guo Ren <guoren@linux.alibaba.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	 Palmer Dabbelt <palmer@dabbelt.com>,
	 linux-kselftest <linux-kselftest@vger.kernel.org>,
	 linux-riscv <linux-riscv@lists.infradead.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	 Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH V2 2/3] riscv: Add support for restartable sequence
Date: Mon, 19 Jul 2021 10:43:10 -0400 (EDT)	[thread overview]
Message-ID: <1257037909.25426.1626705790861.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <1583733592-22873-3-git-send-email-vincent.chen@sifive.com>

----- On Mar 9, 2020, at 1:59 AM, Vincent Chen vincent.chen@sifive.com wrote:
[...]
> --- a/arch/riscv/kernel/signal.c
> +++ b/arch/riscv/kernel/signal.c
> @@ -234,6 +234,7 @@ static void handle_signal(struct ksignal *ksig, struct
> pt_regs *regs)
> 	sigset_t *oldset = sigmask_to_save();
> 	int ret;
> 
> +	rseq_signal_deliver(ksig, regs);
> 	/* Are we from a system call? */
> 	if (regs->cause == EXC_SYSCALL) {

[...]

As Al Viro pointed out on IRC, the rseq_signal_deliver() should go after syscall
restart handling, similarly to what is done on every other supported architecture.

Note that there is already an upstream commit derived on this non-upstream patch:

commit 9866d141a097 ("csky: Add support for restartable sequence")

which is broken in the same way.

I'm not sure why I was never CC'd on the csky patch. Considering that nobody
bothered to implement the rseq selftests for csky, I don't see how any of
this is tested. I would favor a revert of that commit until the testing glue
is contributed. Unfortunately, the csky commit has been upstream since v5.7.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2021-07-19 14:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09  5:59 [PATCH V2 0/3] riscv: add support for restartable sequence Vincent Chen
2020-03-09  5:59 ` Vincent Chen
2020-03-09  5:59 ` [PATCH V2 1/3] riscv: add required functions to enable HAVE_REGS_AND_STACK_ACCESS_API Vincent Chen
2020-03-09  5:59   ` Vincent Chen
2020-03-09  5:59 ` [PATCH V2 2/3] riscv: Add support for restartable sequence Vincent Chen
2020-03-09  5:59   ` Vincent Chen
2021-07-19 14:43   ` Mathieu Desnoyers [this message]
2021-07-19 14:43     ` Mathieu Desnoyers
2021-07-21  3:19     ` Vincent Chen
2021-07-21  3:19       ` Vincent Chen
2020-03-09  5:59 ` [PATCH V2 3/3] rseq/selftests: Add support for riscv Vincent Chen
2020-03-09  5:59   ` Vincent Chen
2020-03-26 15:49   ` Palmer Dabbelt
2020-03-26 15:49     ` Palmer Dabbelt
2020-03-26 16:17     ` Mathieu Desnoyers
2020-03-26 16:17       ` Mathieu Desnoyers
2020-03-27  8:33       ` Vincent Chen
2020-03-27  8:33         ` Vincent Chen

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=1257037909.25426.1626705790861.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=guoren@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.chen@sifive.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.