Re: [PATCH] arm: add support for TIF_NOTIFY_SIGNAL

[Jens Axboe <axboe@kernel.dk>]

On 10/29/20 11:01 AM, Russell King - ARM Linux admin wrote:
> On Thu, Oct 29, 2020 at 10:35:54AM -0600, Jens Axboe wrote:
>> On 10/29/20 10:29 AM, Russell King - ARM Linux admin wrote:
>>> 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.
>>
>> So renumber TIF bits that don't need to worry about do_work_pending() >
>> 15 then? I agree, there's plenty of bits available, so seems reasonable
>> to me. But probably more work in terms of other bits being tested with
>> tstcurrently - at least when I looked, I could not find any of them I
>> could successfully remap from < 8 to > 8.
>>
>> Any chance I can talk you into hacking that up?
> 
> I don't believe that there's any need to do any renumbering.

You are right!

> _TIF_WORK_MASK covers bits 0-3. _TIF_SYSCALL_WORK covers bits 4-7.
> Then we have 17, 18, and 20 which are not used to trigger the
> do_work_pending().
> 
> So, I think it's just a case of changing what you're doing in the
> assembly to my suggestion, and adding a comment to thread_info.h
> noting that bits 0-15 will trigger a call into do_work_pending()
> when returning to userspace.

How about this?


commit c03932936d8f99ff7c1c6c7d984e7a457284396c
Author: Jens Axboe <axboe@kernel.dk>
Date:   Fri Oct 9 16:00:49 2020 -0600

    arm: add support for TIF_NOTIFY_SIGNAL
    
    Wire up TIF_NOTIFY_SIGNAL handling for arm.
    
    Cc: linux-arm-kernel@lists.infradead.org
    Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index 536b6b979f63..eb7ce2747eb0 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -126,6 +126,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp *,
  * thread information flags:
  *  TIF_USEDFPU		- FPU was used by this task this quantum (SMP)
  *  TIF_POLLING_NRFLAG	- true if poll_idle() is polling TIF_NEED_RESCHED
+ *
+ * Any bit in the range of 0..15 will cause do_work_pending() to be invoked.
  */
 #define TIF_SIGPENDING		0	/* signal pending */
 #define TIF_NEED_RESCHED	1	/* rescheduling necessary */
@@ -135,6 +137,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 +151,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 +162,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..77d16390a524 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -53,7 +53,7 @@ __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
+	movs	r1, r1, lsl #16
 	bne	fast_work_pending
 
 
@@ -90,7 +90,7 @@ __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
+	movs	r1, r1, lsl #16
 	beq	no_work_pending
  UNWIND(.fnend		)
 ENDPROC(ret_fast_syscall)
@@ -131,7 +131,7 @@ 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
+	movs	r1, r1, lsl #16
 	bne	slow_work_pending
 no_work_pending:
 	asm_trace_hardirqs_on save = 0
diff --git a/arch/arm/kernel/entry-v7m.S b/arch/arm/kernel/entry-v7m.S
index de1f20624be1..d0e898608d30 100644
--- a/arch/arm/kernel/entry-v7m.S
+++ b/arch/arm/kernel/entry-v7m.S
@@ -59,7 +59,7 @@ __irq_entry:
 
 	get_thread_info tsk
 	ldr	r2, [tsk, #TI_FLAGS]
-	tst	r2, #_TIF_WORK_MASK
+	movs	r2, r2, lsl #16
 	beq	2f			@ no work pending
 	mov	r0, #V7M_SCB_ICSR_PENDSVSET
 	str	r0, [r1, V7M_SCB_ICSR]	@ raise PendSV
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 585edbfccf6d..9d2e916121be 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -655,7 +655,7 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
 			if (unlikely(!user_mode(regs)))
 				return 0;
 			local_irq_enable();
-			if (thread_flags & _TIF_SIGPENDING) {
+			if (thread_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) {
 				int restart = do_signal(regs, syscall);
 				if (unlikely(restart)) {
 					/*

-- 
Jens Axboe


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