* [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 = ¤t->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 = ¤t->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.