[PATCH] arc: add support for TIF_NOTIFY_SIGNAL

[Vineet Gupta]

On 10/29/20 9:09 AM, Jens Axboe wrote:
> Wire up TIF_NOTIFY_SIGNAL handling for arc.
>
> Cc:linux-arm-kernel at lists.infradead.org


Just to be clear, ARC and ARM seem to differ in 1 letter, they are in no 
way related :-)


> Signed-off-by: Jens Axboe<axboe at kernel.dk>
> ---
>
> 5.11 has support queued up for TIF_NOTIFY_SIGNAL, see this posting
> for details:
>
> https://urldefense.com/v3/__https://lore.kernel.org/io-uring/20201026203230.386348-1-axboe@kernel.dk/__;!!A4F2R9G_pg!Oo4kwr6Yy8y5ByC8jIONjU28gLsqpyfzT10u5kfdsj2E-U2VIXQs7SvTmBRIkupZ$  

TL'DR ignore this url rewrite.
Corporate mailer fudging the urls  and I can't seem to find this msg in 
my mailing group subscription in Thunderbird (although it shows up in 
web achieve) hence need to reply from office account rather than the 
newsgroup account.


> As part of that work, I'm adding TIF_NOTIFY_SIGNAL support to all archs,
> as that will enable a set of cleanups once all of them support it. I'm
> happy carrying this patch if need be, or it can be funelled through the
> arch tree. Let me know.


I'm fine either ways too, best to do whatever you are doing for other 
arches. Although this patch per se is broken, please see below.


>   
>   	GET_CURR_THR_INFO_FLAGS   r9
> -	bbit0  r9, TIF_SIGPENDING, .Lchk_notify_resume
> +	and.f  0,  r9, TIF_SIGPENDING|TIF_NOTIFY_SIGNAL, .Lchk_notify_resume

This is not correct, AND is a simple ALU instruction with format: AND 
dest, src2, src1
With dest 0, it won't update any register. With .f suffix it would set 
CC flag based on ALU operation which can subsequent used for a 
predicated instruction such as BZ.

So you need something like

and.f 0, r9, TIF_SIGPENDING|TIF_NOTIFY_SIGNAL
bz .Lchk_notify_resume


>   
>   	; Normal Trap/IRQ entry only saves Scratch (caller-saved) regs
>   	; in pt_reg since the "C" ABI (kernel code) will automatically
> diff --git a/arch/arc/kernel/signal.c b/arch/arc/kernel/signal.c
> index 2be55fb96d87..a78d8f745a67 100644
> --- a/arch/arc/kernel/signal.c
> +++ b/arch/arc/kernel/signal.c
> @@ -362,7 +362,7 @@ void do_signal(struct pt_regs *regs)
>   
>   	restart_scall = in_syscall(regs) && syscall_restartable(regs);
>   
> -	if (get_signal(&ksig)) {
> +	if (test_thread_flag(TIF_SIGPENDING) && get_signal(&ksig)) {
>   		if (restart_scall) {
>   			arc_restart_syscall(&ksig.ka, regs);
>   			syscall_wont_restart(regs);	/* No more restarts */


I've not seen your entire patchset, but it seems we are now hitting 
do_signal() for either of TIF_{SIGPENDING|NOTIFY_SIGNAL} but then only 
handling signal for TIF_SIGPENDING, so why even bother to come here. Do 
you plan to add additional arch handling later ?

Thx,
-Vineet