Re: [PATCH] arm: add support for TIF_NOTIFY_SIGNAL

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Thu, Oct 29, 2020 at 10:11:07AM -0600, Jens Axboe wrote:
> Wire up TIF_NOTIFY_SIGNAL handling for arm.
> 
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> 
> 5.11 has support queued up for TIF_NOTIFY_SIGNAL, see this posting
> for details:
> 
> https://lore.kernel.org/io-uring/20201026203230.386348-1-axboe@kernel.dk/
> 
> 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.
> 
> This needs a bit of asm help, immediate doesn't like anything outside
> of 1 byte, it seems. Any clues?

Correct - immediates take an 8 bit value shifted by an even number of
bits.

I'm tempted to suggest that we simplify things by making TIF bits 0..15
invoke do_work_pending() no matter what - which will require a comment
in thread_info.h that all those bits will have that effect. The
resulting assembly would be:

 	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
-	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
+	movs	r1, r1, lsl #16

which avoids the additional load caused by "ldr r2, =..."

It's not like we're desperate for bits here.

Further comments below if we don't decide on that approach...

> 
>  arch/arm/include/asm/thread_info.h | 5 ++++-
>  arch/arm/kernel/entry-common.S     | 9 ++++++---
>  arch/arm/kernel/signal.c           | 2 +-
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
> index 536b6b979f63..fec16d770180 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -135,6 +135,7 @@ extern int vfp_restore_user_hwstate(struct user_vfp *,
>  #define TIF_SYSCALL_AUDIT	5	/* syscall auditing active */
>  #define TIF_SYSCALL_TRACEPOINT	6	/* syscall tracepoint instrumentation */
>  #define TIF_SECCOMP		7	/* seccomp syscall filtering active */
> +#define TIF_NOTIFY_SIGNAL	8	/* signal notifications exist */
>  
>  #define TIF_USING_IWMMXT	17
>  #define TIF_MEMDIE		18	/* is terminating due to OOM killer */
> @@ -148,6 +149,7 @@ extern int vfp_restore_user_hwstate(struct user_vfp *,
>  #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
>  #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
>  #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
> +#define _TIF_NOTIFY_SIGNAL	(1 << TIF_NOTIFY_SIGNAL)
>  #define _TIF_USING_IWMMXT	(1 << TIF_USING_IWMMXT)
>  
>  /* Checks for any syscall work in entry-common.S */
> @@ -158,7 +160,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp *,
>   * Change these and you break ASM code in entry-common.S
>   */
>  #define _TIF_WORK_MASK		(_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
> -				 _TIF_NOTIFY_RESUME | _TIF_UPROBE)
> +				 _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
> +				 _TIF_NOTIFY_SIGNAL)
>  
>  #endif /* __KERNEL__ */
>  #endif /* __ASM_ARM_THREAD_INFO_H */
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 271cb8a1eba1..7485b58673b4 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -53,7 +53,8 @@ __ret_fast_syscall:
>  	cmp	r2, #TASK_SIZE
>  	blne	addr_limit_check_failed
>  	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
> -	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
> +	ldr	r2, =#_TIF_SYSCALL_WORK | _TIF_WORK_MASK

No # required.

> +	tst	r1, r2
>  	bne	fast_work_pending
>  
>  
> @@ -90,7 +91,8 @@ __ret_fast_syscall:
>  	cmp	r2, #TASK_SIZE
>  	blne	addr_limit_check_failed
>  	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
> -	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
> +	ldr	r2, =#_TIF_SYSCALL_WORK | _TIF_WORK_MASK

Ditto.

> +	tst	r1, r2
>  	beq	no_work_pending
>   UNWIND(.fnend		)
>  ENDPROC(ret_fast_syscall)
> @@ -131,7 +133,8 @@ ENTRY(ret_to_user_from_irq)
>  	cmp	r2, #TASK_SIZE
>  	blne	addr_limit_check_failed
>  	ldr	r1, [tsk, #TI_FLAGS]
> -	tst	r1, #_TIF_WORK_MASK
> +	ldr	r2, =#_TIF_WORK_MASK

Ditto.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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