On 10/30/20 12:47 PM, Vineet Gupta wrote:
> On 10/29/20 9:09 AM, Jens Axboe wrote:
>> Wire up TIF_NOTIFY_SIGNAL handling for arc.
>>
>> Cc:linux-arm-ker...@lists.infradead.org
>
>
> Just to be clear, ARC and ARM seem to differ in 1 letter, they are in no
> way related :-)
Oops, fat fingered that one. Will update :-)
>> 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
Ah thanks, I'll make that change. Hard for me to test a lot of these, so
I really appreciate someone knowledgable taking a look at it.
>>
>> ; 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 ?
Nope, that's all that's needed for each arch. The behavior you describe
is how it works. It makes it so that TIF_SIGPENDING will do the signal
delivery and syscall restart as it always has done, but
TIF_NOTIFY_SIGNAL will only do the syscall restart. The latter is the
intent, hence TIF_NOTIFY_SIGNAL provides a way to interrupt a process
and have it process task_work without going through signal delivery like
task_work with TWA_SIGNAL does today.
Updated version below:
commit 3c6239647d95d03d1436bc826a004791c3f04617
Author: Jens Axboe <ax...@kernel.dk>
Date: Mon Oct 12 07:15:37 2020 -0600
arc: add support for TIF_NOTIFY_SIGNAL
Wire up TIF_NOTIFY_SIGNAL handling for arc.
Cc: linux-snps-arc@lists.infradead.org
Signed-off-by: Jens Axboe <ax...@kernel.dk>
diff --git a/arch/arc/include/asm/thread_info.h
b/arch/arc/include/asm/thread_info.h
index f9eef0e8f0b7..c0942c24d401 100644
--- a/arch/arc/include/asm/thread_info.h
+++ b/arch/arc/include/asm/thread_info.h
@@ -79,6 +79,7 @@ static inline __attribute_const__ struct thread_info
*current_thread_info(void)
#define TIF_SIGPENDING 2 /* signal pending */
#define TIF_NEED_RESCHED 3 /* rescheduling necessary */
#define TIF_SYSCALL_AUDIT 4 /* syscall auditing active */
+#define TIF_NOTIFY_SIGNAL 5 /* signal notifications exist */
#define TIF_SYSCALL_TRACE 15 /* syscall trace active */
/* true if poll_idle() is polling TIF_NEED_RESCHED */
@@ -89,11 +90,12 @@ static inline __attribute_const__ struct thread_info
*current_thread_info(void)
#define _TIF_SIGPENDING (1<<TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1<<TIF_NEED_RESCHED)
#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT)
+#define _TIF_NOTIFY_SIGNAL (1<<TIF_NOTIFY_SIGNAL)
#define _TIF_MEMDIE (1<<TIF_MEMDIE)
/* work to do on interrupt/exception return */
#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
- _TIF_NOTIFY_RESUME)
+ _TIF_NOTIFY_RESUME | _TIF_NOTIFY_SIGNAL)
/*
* _TIF_ALLWORK_MASK includes SYSCALL_TRACE, but we don't need it.
diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
index ea00c8a17f07..1f5308abf36d 100644
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -307,7 +307,8 @@ resume_user_mode_begin:
mov r0, sp ; pt_regs for arg to do_signal()/do_notify_resume()
GET_CURR_THR_INFO_FLAGS r9
- bbit0 r9, TIF_SIGPENDING, .Lchk_notify_resume
+ 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 */
--
Jens Axboe
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
