All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6/6] avr32: deal with syscall restarts
@ 2010-09-28  2:58 Al Viro
  2010-11-01  9:37 ` Hans-Christian Egtvedt
  0 siblings, 1 reply; 2+ messages in thread
From: Al Viro @ 2010-09-28  2:58 UTC (permalink / raw)
  To: hans-christian.egtvedt; +Cc: torvalds, linux-kernel, linux-arch


Currently we both have signals arriving at sigreturn trigger syscall restart
when the original handler hadn't been in the syscall at all *and* have
multiple pending signals trigger the sitation when %pc is shifted back by
more than one insn.

a) take handling of reschedule, signals and keychains into a new helper -
work_pending().  All looping is done there now; asm glue calls that if
we have anything for it to do.

b) do_signal() gets explicit "may restart" flag as an argument; after
the first call during that loop it gets unconditional 0.

c) sigreturn() sets a thread flag - TIF_NORESTART.  It is included into
the "work_pending() has something to do" mask.  work_pending() clears
it, clearing its may_restart flag if it had done so.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/avr32/include/asm/thread_info.h |    3 ++
 arch/avr32/kernel/entry-avr32b.S     |   45 ++++------------------------
 arch/avr32/kernel/signal.c           |   55 ++++++++++++++++++++++++----------
 3 files changed, 48 insertions(+), 55 deletions(-)

diff --git a/arch/avr32/include/asm/thread_info.h b/arch/avr32/include/asm/thread_info.h
index 7a9c03d..1e8fa49 100644
--- a/arch/avr32/include/asm/thread_info.h
+++ b/arch/avr32/include/asm/thread_info.h
@@ -85,6 +85,7 @@ static inline struct thread_info *current_thread_info(void)
 #define TIF_RESTORE_SIGMASK	7	/* restore signal mask in do_signal */
 #define TIF_CPU_GOING_TO_SLEEP	8	/* CPU is entering sleep 0 mode */
 #define TIF_NOTIFY_RESUME	9	/* callback before returning to user */
+#define TIF_NORESTART		10
 #define TIF_FREEZE		29
 #define TIF_DEBUG		30	/* debugging enabled */
 #define TIF_USERSPACE		31      /* true if FS sets userspace */
@@ -98,6 +99,7 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_RESTORE_SIGMASK	(1 << TIF_RESTORE_SIGMASK)
 #define _TIF_CPU_GOING_TO_SLEEP (1 << TIF_CPU_GOING_TO_SLEEP)
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
+#define _TIF_NORESTART		(1 << TIF_NORESTART)
 #define _TIF_FREEZE		(1 << TIF_FREEZE)
 
 /* Note: The masks below must never span more than 16 bits! */
@@ -106,6 +108,7 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_WORK_MASK				\
 	((1 << TIF_SIGPENDING)			\
 	 | _TIF_NOTIFY_RESUME			\
+	 | _TIF_NORESTART			\
 	 | (1 << TIF_NEED_RESCHED)		\
 	 | (1 << TIF_POLLING_NRFLAG)		\
 	 | (1 << TIF_BREAKPOINT)		\
diff --git a/arch/avr32/kernel/entry-avr32b.S b/arch/avr32/kernel/entry-avr32b.S
index 5e6beb2..bb455b7 100644
--- a/arch/avr32/kernel/entry-avr32b.S
+++ b/arch/avr32/kernel/entry-avr32b.S
@@ -271,28 +271,11 @@ syscall_exit_work:
 	unmask_interrupts
 	call	syscall_trace
 	mask_interrupts
-	ld.w	r1, r0[TI_flags]
-
-1:	bld	r1, TIF_NEED_RESCHED
-	brcc	2f
-	unmask_interrupts
-	call	schedule
-	mask_interrupts
-	ld.w	r1, r0[TI_flags]
-	rjmp	1b
-
-2:	mov	r2, _TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NOTIFY_RESUME
-	tst	r1, r2
-	breq	3f
-	unmask_interrupts
 	mov	r12, sp
 	mov	r11, r0
-	call	do_notify_resume
-	mask_interrupts
-	ld.w	r1, r0[TI_flags]
-	rjmp	1b
-
-3:	bld	r1, TIF_BREAKPOINT
+	mov	r10, 1
+	call	work_pending
+	bld	r12, TIF_BREAKPOINT
 	brcc	syscall_exit_cont
 	rjmp	enter_monitor_mode
 
@@ -576,29 +559,13 @@ irq_exit_work:
 	mtsr	SYSREG_SR, r8
 	sub	pc, -2
 	get_thread_info r0
-	ld.w	r1, r0[TI_flags]
 
 fault_exit_work:
-	bld	r1, TIF_NEED_RESCHED
-	brcc	1f
-	unmask_interrupts
-	call	schedule
-	mask_interrupts
-	ld.w	r1, r0[TI_flags]
-	rjmp	fault_exit_work
-
-1:	mov	r2, _TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NOTIFY_RESUME
-	tst	r1, r2
-	breq	2f
-	unmask_interrupts
 	mov	r12, sp
 	mov	r11, r0
-	call	do_notify_resume
-	mask_interrupts
-	ld.w	r1, r0[TI_flags]
-	rjmp	fault_exit_work
-
-2:	bld	r1, TIF_BREAKPOINT
+	mov	r10, 0
+	call	work_pending
+	bld	r12, TIF_BREAKPOINT
 	brcc	fault_resume_user
 	rjmp	enter_monitor_mode
 
diff --git a/arch/avr32/kernel/signal.c b/arch/avr32/kernel/signal.c
index 8e8f6ba..c4c76f5 100644
--- a/arch/avr32/kernel/signal.c
+++ b/arch/avr32/kernel/signal.c
@@ -45,6 +45,8 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
 
+	set_thread_flag(TIF_NORESTART);
+
 #define COPY(x)		err |= __get_user(regs->x, &sc->x)
 	COPY(sr);
 	COPY(pc);
@@ -264,7 +266,7 @@ fail:
  * doesn't want to handle. Thus you cannot kill init even with a
  * SIGKILL even by mistake.
  */
-static void do_signal(struct pt_regs *regs, int syscall)
+static void do_signal(struct pt_regs *regs, int may_restart)
 {
 	siginfo_t info;
 	int signr;
@@ -285,7 +287,7 @@ static void do_signal(struct pt_regs *regs, int syscall)
 		oldset = &current->blocked;
 
 	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
-	if (syscall) {
+	if (may_restart) {
 		switch (regs->r12) {
 		case -ERESTART_RESTARTBLOCK:
 		case -ERESTARTNOHAND:
@@ -317,20 +319,41 @@ static void do_signal(struct pt_regs *regs, int syscall)
 	handle_signal(signr, &ka, &info, oldset, regs);
 }
 
-asmlinkage void do_notify_resume(struct pt_regs *regs, struct thread_info *ti)
+asmlinkage int work_pending(struct pt_regs *regs, struct thread_info *ti, int may_restart)
 {
-	int syscall = 0;
-
-	if ((sysreg_read(SR) & MODE_MASK) == MODE_SUPERVISOR)
-		syscall = 1;
-
-	if (ti->flags & (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK))
-		do_signal(regs, syscall);
-
-	if (ti->flags & _TIF_NOTIFY_RESUME) {
-		clear_thread_flag(TIF_NOTIFY_RESUME);
-		tracehook_notify_resume(regs);
-		if (current->replacement_session_keyring)
-			key_replace_session_keyring();
+	int flags = ti->flags;
+	if (unlikely(flags & _TIF_NORESTART)) {
+		ti->flags &= ~_TIF_NORESTART;
+		may_restart = 0;
+	}
+	while (1) {
+		if (flags & _TIF_NEED_RESCHED) {
+			local_irq_enable();
+			schedule();
+			local_irq_disable();
+			flags = ti->flags;
+			continue;
+		}
+		if (flags & _TIF_SIGPENDING) {
+			local_irq_enable();
+			if ((sysreg_read(SR) & MODE_MASK) != MODE_SUPERVISOR)
+				may_restart = 0;
+			do_signal(regs, may_restart);
+			may_restart = 0;
+			local_irq_disable();
+			flags = ti->flags;
+			continue;
+		}
+		if (flags & _TIF_NOTIFY_RESUME) {
+			ti->flags &= ~_TIF_NOTIFY_RESUME;
+			local_irq_enable();
+			tracehook_notify_resume(regs);
+			if (current->replacement_session_keyring)
+				key_replace_session_keyring();
+			local_irq_disable();
+			flags = ti->flags;
+			continue;
+		}
+		return flags;
 	}
 }
-- 
1.5.6.5


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

* Re: [PATCH 6/6] avr32: deal with syscall restarts
  2010-09-28  2:58 [PATCH 6/6] avr32: deal with syscall restarts Al Viro
@ 2010-11-01  9:37 ` Hans-Christian Egtvedt
  0 siblings, 0 replies; 2+ messages in thread
From: Hans-Christian Egtvedt @ 2010-11-01  9:37 UTC (permalink / raw)
  To: Al Viro; +Cc: torvalds, linux-kernel, linux-arch

On Tue, 2010-09-28 at 03:58 +0100, Al Viro wrote: 
> Currently we both have signals arriving at sigreturn trigger syscall restart
> when the original handler hadn't been in the syscall at all *and* have
> multiple pending signals trigger the sitation when %pc is shifted back by
> more than one insn.
> 
> a) take handling of reschedule, signals and keychains into a new helper -
> work_pending().  All looping is done there now; asm glue calls that if
> we have anything for it to do.

Am I the only one getting a conflict with the work_pending() function in
include/linux/workqueue.h?

> b) do_signal() gets explicit "may restart" flag as an argument; after
> the first call during that loop it gets unconditional 0.
> 
> c) sigreturn() sets a thread flag - TIF_NORESTART.  It is included into
> the "work_pending() has something to do" mask.  work_pending() clears
> it, clearing its may_restart flag if it had done so.

Both sounds and look sane further below.

> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  arch/avr32/include/asm/thread_info.h |    3 ++
>  arch/avr32/kernel/entry-avr32b.S     |   45 ++++------------------------
>  arch/avr32/kernel/signal.c           |   55 ++++++++++++++++++++++++----------
>  3 files changed, 48 insertions(+), 55 deletions(-)
> 
> diff --git a/arch/avr32/include/asm/thread_info.h b/arch/avr32/include/asm/thread_info.h
> index 7a9c03d..1e8fa49 100644
> --- a/arch/avr32/include/asm/thread_info.h
> +++ b/arch/avr32/include/asm/thread_info.h
> @@ -85,6 +85,7 @@ static inline struct thread_info *current_thread_info(void)
>  #define TIF_RESTORE_SIGMASK	7	/* restore signal mask in do_signal */
>  #define TIF_CPU_GOING_TO_SLEEP	8	/* CPU is entering sleep 0 mode */
>  #define TIF_NOTIFY_RESUME	9	/* callback before returning to user */
> +#define TIF_NORESTART		10

Please add a comment after the define.

<snipp>

> diff --git a/arch/avr32/kernel/entry-avr32b.S b/arch/avr32/kernel/entry-avr32b.S
> index 5e6beb2..bb455b7 100644
> --- a/arch/avr32/kernel/entry-avr32b.S
> +++ b/arch/avr32/kernel/entry-avr32b.S
> @@ -271,28 +271,11 @@ syscall_exit_work:
>  	unmask_interrupts
>  	call	syscall_trace
>  	mask_interrupts
> -	ld.w	r1, r0[TI_flags]
> -
> -1:	bld	r1, TIF_NEED_RESCHED

Hmmm...

@ line 268
syscall_exit_work:
bld r1, TIF_SYSCALL_TRACE
brcc 1f

Which refers to the 1: label you remove here.

> -	brcc	2f
> -	unmask_interrupts
> -	call	schedule
> -	mask_interrupts
> -	ld.w	r1, r0[TI_flags]
> -	rjmp	1b
> -
> -2:	mov	r2, _TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NOTIFY_RESUME
> -	tst	r1, r2
> -	breq	3f
> -	unmask_interrupts
>  	mov	r12, sp

Make a 1: label at this line should do the trick AFAICT.

<snipp>

> diff --git a/arch/avr32/kernel/signal.c b/arch/avr32/kernel/signal.c
> index 8e8f6ba..c4c76f5 100644
> --- a/arch/avr32/kernel/signal.c
> +++ b/arch/avr32/kernel/signal.c
> @@ -45,6 +45,8 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
>  	/* Always make any pending restarted system calls return -EINTR */
>  	current_thread_info()->restart_block.fn = do_no_restart_syscall;
>  
> +	set_thread_flag(TIF_NORESTART);
> +
>  #define COPY(x)		err |= __get_user(regs->x, &sc->x)
>  	COPY(sr);
>  	COPY(pc);
> @@ -264,7 +266,7 @@ fail:
>   * doesn't want to handle. Thus you cannot kill init even with a
>   * SIGKILL even by mistake.
>   */
> -static void do_signal(struct pt_regs *regs, int syscall)
> +static void do_signal(struct pt_regs *regs, int may_restart)
>  {
>  	siginfo_t info;
>  	int signr;
> @@ -285,7 +287,7 @@ static void do_signal(struct pt_regs *regs, int syscall)
>  		oldset = &current->blocked;
>  
>  	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
> -	if (syscall) {
> +	if (may_restart) {
>  		switch (regs->r12) {
>  		case -ERESTART_RESTARTBLOCK:
>  		case -ERESTARTNOHAND:
> @@ -317,20 +319,41 @@ static void do_signal(struct pt_regs *regs, int syscall)
>  	handle_signal(signr, &ka, &info, oldset, regs);
>  }
>  
> -asmlinkage void do_notify_resume(struct pt_regs *regs, struct thread_info *ti)
> +asmlinkage int work_pending(struct pt_regs *regs, struct thread_info *ti, int may_restart)

I get a conflict with a similar function declared in
include/linux/workqueue.h. Also, please brake it over multiple lines as
it is wider than 80 characters.

<snipp>

-- 
Hans-Christian Egtvedt


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

end of thread, other threads:[~2010-11-01  9:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-28  2:58 [PATCH 6/6] avr32: deal with syscall restarts Al Viro
2010-11-01  9:37 ` Hans-Christian Egtvedt

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.