All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] xtensa: fix broken TIF_NOTIFY_SIGNAL assembly
@ 2020-11-11 20:53 Max Filippov
  2020-11-11 21:05 ` Jens Axboe
  0 siblings, 1 reply; 4+ messages in thread
From: Max Filippov @ 2020-11-11 20:53 UTC (permalink / raw)
  To: linux-xtensa; +Cc: Chris Zankel, linux-kernel, Max Filippov, Jens Axboe

TIF_NOTIFY_SIGNAL handling in xtensa assembly is implemented
incorrectly: there should be a call to do_notify_resume when either
TIF_SIGPENDING, TIF_NOTIFY_RESUME or TIF_NOTIFY_SIGNAL bit is set in the
thread_info::flags. The straightforward way to do it would be

    _bbsi.l a4, TIF_NEED_RESCHED, 3f
    _bbsi.l a4, TIF_NOTIFY_RESUME, 2f
    _bbsi.l a4, TIF_NOTIFY_SIGNAL, 2f
    _bbci.l a4, TIF_SIGPENDING, 5f

Optimize it a little bit and use bit mask and bnone opcode to skip
do_notify_resume invocation. Shuffle _TIF_* flags a bit so that used bit
mask fits into the immediate field of movi opcode.

Fixes: 4c6a9dcd4d13 ("xtensa: add support for TIF_NOTIFY_SIGNAL")
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
Changes v1->v2:
- use more canonical order of bnone arguments: tested value first,
  bit mask second.

 arch/xtensa/include/asm/thread_info.h | 7 ++++---
 arch/xtensa/kernel/entry.S            | 5 ++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/xtensa/include/asm/thread_info.h b/arch/xtensa/include/asm/thread_info.h
index 6ea521b8e2ec..a312333a9add 100644
--- a/arch/xtensa/include/asm/thread_info.h
+++ b/arch/xtensa/include/asm/thread_info.h
@@ -111,22 +111,23 @@ static inline struct thread_info *current_thread_info(void)
 #define TIF_NEED_RESCHED	2	/* rescheduling necessary */
 #define TIF_SINGLESTEP		3	/* restore singlestep on return to user mode */
 #define TIF_SYSCALL_TRACEPOINT	4	/* syscall tracepoint instrumentation */
-#define TIF_MEMDIE		5	/* is terminating due to OOM killer */
+#define TIF_NOTIFY_SIGNAL	5	/* signal notifications exist */
 #define TIF_RESTORE_SIGMASK	6	/* restore signal mask in do_signal() */
 #define TIF_NOTIFY_RESUME	7	/* callback before returning to user */
 #define TIF_DB_DISABLED		8	/* debug trap disabled for syscall */
 #define TIF_SYSCALL_AUDIT	9	/* syscall auditing active */
 #define TIF_SECCOMP		10	/* secure computing */
-#define TIF_NOTIFY_SIGNAL	11	/* signal notifications exist */
+#define TIF_MEMDIE		11	/* is terminating due to OOM killer */
 
 #define _TIF_SYSCALL_TRACE	(1<<TIF_SYSCALL_TRACE)
 #define _TIF_SIGPENDING		(1<<TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1<<TIF_NEED_RESCHED)
 #define _TIF_SINGLESTEP		(1<<TIF_SINGLESTEP)
 #define _TIF_SYSCALL_TRACEPOINT	(1<<TIF_SYSCALL_TRACEPOINT)
+#define _TIF_NOTIFY_SIGNAL	(1<<TIF_NOTIFY_SIGNAL)
+#define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
 #define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1<<TIF_SECCOMP)
-#define _TIF_NOTIFY_SIGNAL	(1<<TIF_NOTIFY_SIGNAL)
 
 #define _TIF_WORK_MASK		(_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP | \
 				 _TIF_SYSCALL_TRACEPOINT | \
diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S
index 7f733f40fef0..647b162f959b 100644
--- a/arch/xtensa/kernel/entry.S
+++ b/arch/xtensa/kernel/entry.S
@@ -500,9 +500,8 @@ common_exception_return:
 	 */
 
 	_bbsi.l	a4, TIF_NEED_RESCHED, 3f
-	_bbsi.l	a4, TIF_NOTIFY_RESUME, 2f
-	_bbci.l	a4, TIF_SIGPENDING, 5f
-	_bbci.l	a4, TIF_NOTIFY_SIGNAL, 5f
+	movi	a2, _TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_NOTIFY_SIGNAL
+	bnone	a4, a2, 5f
 
 2:	l32i	a4, a1, PT_DEPC
 	bgeui	a4, VALID_DOUBLE_EXCEPTION_ADDRESS, 4f
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] xtensa: fix broken TIF_NOTIFY_SIGNAL assembly
  2020-11-11 20:53 [PATCH v2] xtensa: fix broken TIF_NOTIFY_SIGNAL assembly Max Filippov
@ 2020-11-11 21:05 ` Jens Axboe
  2020-11-12  9:12   ` Max Filippov
  0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2020-11-11 21:05 UTC (permalink / raw)
  To: Max Filippov, linux-xtensa; +Cc: Chris Zankel, linux-kernel

On 11/11/20 1:53 PM, Max Filippov wrote:
> TIF_NOTIFY_SIGNAL handling in xtensa assembly is implemented
> incorrectly: there should be a call to do_notify_resume when either
> TIF_SIGPENDING, TIF_NOTIFY_RESUME or TIF_NOTIFY_SIGNAL bit is set in the
> thread_info::flags. The straightforward way to do it would be
> 
>     _bbsi.l a4, TIF_NEED_RESCHED, 3f
>     _bbsi.l a4, TIF_NOTIFY_RESUME, 2f
>     _bbsi.l a4, TIF_NOTIFY_SIGNAL, 2f
>     _bbci.l a4, TIF_SIGPENDING, 5f
> 
> Optimize it a little bit and use bit mask and bnone opcode to skip
> do_notify_resume invocation. Shuffle _TIF_* flags a bit so that used bit
> mask fits into the immediate field of movi opcode.

Thanks - do you mind if I fold this in with a reference to your
changes? Seems like that'd be a better option than leaving it
broken for a bit.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] xtensa: fix broken TIF_NOTIFY_SIGNAL assembly
  2020-11-11 21:05 ` Jens Axboe
@ 2020-11-12  9:12   ` Max Filippov
  2020-11-12 15:45     ` Jens Axboe
  0 siblings, 1 reply; 4+ messages in thread
From: Max Filippov @ 2020-11-12  9:12 UTC (permalink / raw)
  To: Jens Axboe; +Cc: open list:TENSILICA XTENSA PORT (xtensa), Chris Zankel, LKML

On Wed, Nov 11, 2020 at 1:05 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 11/11/20 1:53 PM, Max Filippov wrote:
> > TIF_NOTIFY_SIGNAL handling in xtensa assembly is implemented
> > incorrectly: there should be a call to do_notify_resume when either
> > TIF_SIGPENDING, TIF_NOTIFY_RESUME or TIF_NOTIFY_SIGNAL bit is set in the
> > thread_info::flags. The straightforward way to do it would be
> >
> >     _bbsi.l a4, TIF_NEED_RESCHED, 3f
> >     _bbsi.l a4, TIF_NOTIFY_RESUME, 2f
> >     _bbsi.l a4, TIF_NOTIFY_SIGNAL, 2f
> >     _bbci.l a4, TIF_SIGPENDING, 5f
> >
> > Optimize it a little bit and use bit mask and bnone opcode to skip
> > do_notify_resume invocation. Shuffle _TIF_* flags a bit so that used bit
> > mask fits into the immediate field of movi opcode.
>
> Thanks - do you mind if I fold this in with a reference to your
> changes? Seems like that'd be a better option than leaving it
> broken for a bit.

Sure Jens, by all means!

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] xtensa: fix broken TIF_NOTIFY_SIGNAL assembly
  2020-11-12  9:12   ` Max Filippov
@ 2020-11-12 15:45     ` Jens Axboe
  0 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2020-11-12 15:45 UTC (permalink / raw)
  To: Max Filippov; +Cc: open list:TENSILICA XTENSA PORT (xtensa), Chris Zankel, LKML

On 11/12/20 2:12 AM, Max Filippov wrote:
> On Wed, Nov 11, 2020 at 1:05 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 11/11/20 1:53 PM, Max Filippov wrote:
>>> TIF_NOTIFY_SIGNAL handling in xtensa assembly is implemented
>>> incorrectly: there should be a call to do_notify_resume when either
>>> TIF_SIGPENDING, TIF_NOTIFY_RESUME or TIF_NOTIFY_SIGNAL bit is set in the
>>> thread_info::flags. The straightforward way to do it would be
>>>
>>>     _bbsi.l a4, TIF_NEED_RESCHED, 3f
>>>     _bbsi.l a4, TIF_NOTIFY_RESUME, 2f
>>>     _bbsi.l a4, TIF_NOTIFY_SIGNAL, 2f
>>>     _bbci.l a4, TIF_SIGPENDING, 5f
>>>
>>> Optimize it a little bit and use bit mask and bnone opcode to skip
>>> do_notify_resume invocation. Shuffle _TIF_* flags a bit so that used bit
>>> mask fits into the immediate field of movi opcode.
>>
>> Thanks - do you mind if I fold this in with a reference to your
>> changes? Seems like that'd be a better option than leaving it
>> broken for a bit.
> 
> Sure Jens, by all means!

Thanks! Done.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-11-12 15:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 20:53 [PATCH v2] xtensa: fix broken TIF_NOTIFY_SIGNAL assembly Max Filippov
2020-11-11 21:05 ` Jens Axboe
2020-11-12  9:12   ` Max Filippov
2020-11-12 15:45     ` Jens Axboe

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.