[PATCH RFC 0/8] Signal: harmonize syscall restart logic

Jonas Bonn

author

2011-10-23 10:19 UTC #0 lore raw

cover Signal: harmonize syscall restart logic

I've been looking into trying to harmonize the architecture specific signal handling code. With a couple of helper functions to mask some of the architecture specific bits, the code for do_signal and handle_signal (implemented by essentially everybody) start to look identical across all arch's and could likely be made generic.

The big helper is the one in this series. This one moves the syscall restart logic out to a common helper. The arch specific bit here is the register manipulations required to actually effect the restart; this patch series moves those manipulations to the asm/syscall.h header which already attempts to define a generic syscall interface for several arch's today.

Where this series remains an RFC is due to a couple of open questions:

i) ARM (and a few arch's copied from it) does its restart logic in two steps so that a debugger can intervene; is there still value in this approach or would the single step approach in this patch series be sufficient for ARM (and others)?

ii) The syscall.h interface requires that the information needed to do
the register manipulations be in the pt_regs struct. This isn't the case for all arch's today. Are there any issues with putting that info into pt_regs for those arch's?

With the rest of the cleanups in this series it currently comes to 64 patches, so I'm just posting a couple of the syscall restart patches now for comment in order not to flood the lists. I'll post the remainder of the series later based on the feedback from this series.

/Jonas

Jonas Bonn (8):
signal: introduce generic syscall restart logic
blackfin: implement syscall restart generically
frv: implement syscall restart generically
mips: implement syscall restart generically
x86: implement syscall restart generically
m68k: implement syscall restart generically
ia64: implement syscall restart generically
tile: implement syscall restart generically

 arch/blackfin/include/asm/syscall.h |   26 +++++++++-
 arch/blackfin/kernel/signal.c       |   47 +-----------------
 arch/frv/include/asm/syscall.h      |   23 ++++++++-
 arch/frv/kernel/signal.c            |   48 +-----------------
 arch/ia64/include/asm/syscall.h     |   21 ++++++++
 arch/ia64/kernel/signal.c           |   54 +--------------------
 arch/m68k/include/asm/syscall.h     |   87 +++++++++++++++++++++++++++++++++
 arch/m68k/kernel/signal_mm.c        |   45 +----------------
 arch/m68k/kernel/signal_no.c        |   46 +----------------
 arch/mips/include/asm/syscall.h     |   91 +++++++++++++++++++++++++++++++++++
 arch/mips/kernel/signal.c           |   40 +--------------
 arch/tile/include/asm/syscall.h     |   28 ++++++++++-
 arch/tile/kernel/signal.c           |   52 ++------------------
 arch/x86/include/asm/syscall.h      |   21 ++++++++
 arch/x86/kernel/signal.c            |   43 +---------------
 include/asm-generic/syscall.h       |   46 +++++++++++++++++-
 include/linux/signal.h              |    3 +
 kernel/signal.c                     |   72 +++++++++++++++++++++++++++-
 18 files changed, 435 insertions(+), 358 deletions(-)
 create mode 100644 arch/m68k/include/asm/syscall.h
 create mode 100644 arch/mips/include/asm/syscall.h

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:19 UTC #1 lore raw

RFC PATCH 1/8 signal: introduce generic syscall restart logic

Manipulating the task state so that an interrupted syscall will be re-executed after signal handling has historically been purely architecture specific logic. Most architectures, however, are implementing essentially the same thing (some incorrectly) with the actual details of the registers to be manipulated being the only relevant difference.

This patch introduces generic code for handling the syscall restart logic by:

i) introducing methods for doing the actual register manipulations to the generic syscall interface in asm/syscall.h

ii) introducing the function handle_syscall_restart to determine the actual restart requirements and invoke the required register manipulations; this function is designed to be called immediately after invoking the function get_signal_to_deliver

This generic variant is applicable for any architecture that maintains the information necessary to effect a syscall restart in its pt_regs struct.

Signed-off-by Jonas Bonn <[email protected]>

---
 include/asm-generic/syscall.h |   46 +++++++++++++++++++++++++-
 include/linux/signal.h        |    3 ++
 kernel/signal.c               |   72 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 119 insertions(+), 2 deletions(-)

include/asm-generic/syscall.h +45 -1

index 5c122ae..b7b489f 100644
@@ -36,11 +36,55 @@ struct pt_regs;
  * system call number can be meaningful.  If the actual arch value
  * is 64 bits, this truncates to 32 bits so 0xffffffff means -1.
  *
- * It's only valid to call this when @task is known to be blocked.
+ * It's only valid to call this for the current task or when @task
+ * is known to be blocked.
  */
 int syscall_get_nr(struct task_struct *task, struct pt_regs *regs);
 
 /**
+ * syscall_clear - clear the syscall execution state
+ *
+ * Clear the syscall execution state of @task so that future invocations
+ * of syscall_get_nr for @task return -1.
+ *
+ * It's only valid to call this for the current task.  This function
+ * is primarily intended for use in the signal handling path; when
+ * signal handling takes over after a syscall then the syscall handling
+ * is effectively finished and the syscall state can be cleared.
+ */
+void syscall_clear(struct task_struct *task, struct pt_regs *regs);
+
+/**
+ * syscall_do_restart - roll back PC and argument regs for syscall restart
+ * @task:	must be current task
+ * @regs:	task_pt_regs() of @task
+ * 
+ * Rolls back the syscall number, arguments, and the instruction pointer
+ * so that the current syscall will be reexecuted upon return to userspace.
+ *
+ * This function is primarily intended for use in the signal handling
+ * code and shouldn't really be used elsewhere.
+ */
+void syscall_restart(struct task_struct *task, struct pt_regs *regs);
+
+/**
+ * syscall_do_restartblock - roll back PC and execute restart_syscall
+ * @task:	must be current task
+ * @regs:	task_pt_regs() of @task
+ * 
+ * Configure @task so that upon return to userspace the restart_syscall
+ * (or equivalent) function is executed to perform syscall restart via a
+ * restart_block.  Since restart_syscall normally doesn't take any arguments,
+ * restoring the syscall arguments can be skipped.  This normally boils
+ * down to rewinding the PC and setting the syscall register to 
+ * __NR_restart_syscall; however, some arch's may do this differently.
+ *
+ * This function is primarily intended for use in the signal handling
+ * code.
+ */
+void syscall_do_restartblock(struct task_struct * taks, struct pt_regs* regs);
+
+/**
  * syscall_rollback - roll back registers after an aborted system call
  * @task:	task of interest, must be in system call exit tracing
  * @regs:	task_pt_regs() of @task

include/linux/signal.h +3 -0

index a822300..7ede592 100644
@@ -256,6 +256,9 @@ extern int show_unhandled_signals;
 extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
 extern void exit_signals(struct task_struct *tsk);
 
+void handle_syscall_restart(struct pt_regs *regs, struct k_sigaction *ka,
+			    int has_handler);
+
 extern struct kmem_cache *sighand_cachep;
 
 int unhandled_signal(struct task_struct *tsk, int sig);

kernel/signal.c +71 -1

85 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:20 UTC #2 lore raw

RFC PATCH 2/8 blackfin: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc [email protected]

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/blackfin/include/asm/syscall.h |   26 ++++++++++++++++++-
 arch/blackfin/kernel/signal.c       |   47 ++--------------------------------
 2 files changed, 27 insertions(+), 46 deletions(-)

arch/blackfin/include/asm/syscall.h +24 -2

index 4921a48..87005c4 100644
@@ -21,17 +21,39 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 #include <asm/ptrace.h>
+#include <asm/unistd.h>
 
 static inline long
 syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
 {
-	return regs->p0;
+	return regs->orig_p0;
 }
 
 static inline void
 syscall_rollback(struct task_struct *task, struct pt_regs *regs)
 {
 	regs->p0 = regs->orig_p0;
+	regs->r0 = regs->orig_r0;
+}
+
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->orig_p0 = -1;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->pc -= 2;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->p0 = __NR_restart_syscall;
+	regs->pc -= 2;
 }
 
 static inline long

@@ -50,7 +72,7 @@ static inline void
 syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
                          int error, long val)
 {
-	regs->r0 = error ? -error : val;
+	regs->r0 = error ? error : val;
 }
 
 /**

arch/blackfin/kernel/signal.c +3 ~~-44~~

77 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Mike Frysinger

2011-10-26 00:02 UTC #3 lore raw

Re: [PATCH RFC 2/8] blackfin: implement syscall restart generically

Quoted 9 lines · Jonas Bonn

arch/blackfin/include/asm/syscall.h +0 -0
static inline long
syscall_get_nr(struct task_struct *task, struct pt_regs *regs) {
- return regs->p0;
+ return regs->orig_p0;
}

i'm not sure this is correct. we set the orig_p0 to -1 when forcing the syscall to go to restart. shouldn't syscall_get_nr() still return the right value ?

Quoted 6 lines

syscall_set_return_value(struct task_struct *task, struct pt_regs *regs, int error, long val)
{
- regs->r0 = error ? -error : val;
+ regs->r0 = error ? error : val;
}

this fix is unrelated (and unmentioned in the changelog). i also see a bunch of other arches doing this. so we should pull this change out into a dedicated patchset, and fix all the arches at the same time. -mike

Jonas Bonn

author

2011-10-26 06:16 UTC #4 lore raw

Re: [PATCH RFC 2/8] blackfin: implement syscall restart generically

Quoted 4 lines · Mike Frysinger

Quoted 3 lines · Jonas Bonn
arch/blackfin/include/asm/syscall.h +0 -0

Quoted 10 lines

arch/blackfin/include/asm/syscall.h line 30
Quoted 6 lines
static inline long
syscall_get_nr(struct task_struct *task, struct pt_regs *regs) {
arch/blackfin/include/asm/syscall.h +1 -1
-       return regs->p0;
+       return regs->orig_p0;
 }
i'm not sure this is correct. we set the orig_p0 to -1 when forcing the syscall to go to restart. shouldn't syscall_get_nr() still return the right value ?

No, syscall_get_nr is only valid while processing a syscall (should return -1 otherwise) and the syscall processing is effectively finished once you've handled the restart requirements so returning -1 at that point makes sense... patch 1/8 relies on that fact.

Quoted 11 lines

arch/blackfin/include/asm/syscall.h static inline void · line 76
Quoted 7 lines
syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
int error, long val)
{
arch/blackfin/include/asm/syscall.h +1 -1
-       regs->r0 = error ? -error : val;
+       regs->r0 = error ? error : val;
 }
this fix is unrelated (and unmentioned in the changelog). i also see a bunch of other arches doing this. so we should pull this change out into a dedicated patchset, and fix all the arches at the same time.

Agreed, this should go into a separate patch. It's true that some arches do the negation as above, but those arch's return positive error numbers and indicate syscall error by setting a separate flag. I'll prepare a separate patch for this.

/Jonas

Mike Frysinger

2011-10-26 07:35 UTC #5 lore raw

Re: [uclinux-dist-devel] [PATCH RFC 2/8] blackfin: implement syscall restart generically

thanks, i look forward to the followups :)
-mike

Jonas Bonn

author

2011-10-23 10:20 UTC #6 lore raw

RFC PATCH 3/8 frv: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc David Howells <[email protected]>

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/frv/include/asm/syscall.h |   23 ++++++++++++++++++-
 arch/frv/kernel/signal.c       |   48 ++-------------------------------------
 2 files changed, 25 insertions(+), 46 deletions(-)

arch/frv/include/asm/syscall.h +22 -1

index 70689eb..a0493da 100644
@@ -14,6 +14,7 @@
 
 #include <linux/err.h>
 #include <asm/ptrace.h>
+#include <asm/unistd.h>
 
 /*
  * Get the system call number or -1

@@ -34,6 +35,26 @@ static inline void syscall_rollback(struct task_struct *task,
 	regs->gr8 = regs->orig_gr8;
 }
 
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->syscallno = -1;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->pc -= 4;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->gr7 = __NR_restart_syscall;
+	regs->pc -= 4;
+}
+
 /*
  * See if the syscall return value is an error, returning it if it is and 0 if
  * not

@@ -61,7 +82,7 @@ static inline void syscall_set_return_value(struct task_struct *task,
 					    int error, long val)
 {
 	if (error)
-		regs->gr8 = -error;
+		regs->gr8 = error;
 	else
 		regs->gr8 = val;
 }

arch/frv/kernel/signal.c +3 ~~-45~~

72 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:19 UTC #7 lore raw

RFC PATCH 4/8 mips: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc [email protected]

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/mips/include/asm/syscall.h |   91 +++++++++++++++++++++++++++++++++++++++
 arch/mips/kernel/signal.c       |   40 +----------------
 2 files changed, 94 insertions(+), 37 deletions(-)
 create mode 100644 arch/mips/include/asm/syscall.h

arch/mips/include/asm/syscall.h new file +91 -0

new file mode 100644
index 0000000..7280d95
@@ -0,0 +1,91 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Copyright (C) 2011 Jonas Bonn <[email protected]>
+ */
+
+#ifndef _ASM_SYSCALL_H
+#define _ASM_SYSCALL_H
+
+#include <linux/err.h>
+#include <linux/sched.h>
+#include <asm/unistd.h>
+
+static inline int
+syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
+{
+	return regs->regs[0] ?: -1;
+}
+
+static inline void
+syscall_rollback(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->regs[2] = regs->regs[0];
+	regs->regs[7] = regs->regs[26];
+}
+
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->regs[0] = 0;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->cp0_epc -= 4;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->regs[2] = current->thread.abi->restart;
+	regs->regs[7] = regs->regs[26];
+	regs->cp0_epc -= 4;
+}
+
+static inline long
+syscall_get_error(struct task_struct *task, struct pt_regs *regs)
+{
+	/*
+	 * r7 == 1 if syscall returns error
+	 * r2 contains _positive_ result
+	 */
+	if (regs->regs[7])
+		return -regs->regs[2];
+	else
+		return 0;
+}
+
+static inline long
+syscall_get_return_value(struct task_struct *task, struct pt_regs *regs)
+{
+	return regs->gpr[2];
+}
+
+static inline void
+syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
+			 int error, long val)
+{
+	regs->gpr[2] = (long) error ? -error : val;
+}
+
+static inline void
+syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned int i, unsigned int n, unsigned long *args)
+{
+	/* TODO */
+}
+
+static inline void
+syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned int i, unsigned int n, const unsigned long *args)
+{
+	/* TODO */
+}
+
+#endif

arch/mips/kernel/signal.c +3 ~~-37~~

index dbbe0ce..13f2864 100644
@@ -547,27 +547,6 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 	struct mips_abi *abi = current->thread.abi;
 	void *vdso = current->mm->context.vdso;
 
-	if (regs->regs[0]) {
-		switch(regs->regs[2]) {
-		case ERESTART_RESTARTBLOCK:
-		case ERESTARTNOHAND:
-			regs->regs[2] = EINTR;
-			break;
-		case ERESTARTSYS:
-			if (!(ka->sa.sa_flags & SA_RESTART)) {
-				regs->regs[2] = EINTR;
-				break;
-			}
-		/* fallthrough */
-		case ERESTARTNOINTR:
-			regs->regs[7] = regs->regs[26];
-			regs->regs[2] = regs->regs[0];
-			regs->cp0_epc -= 4;
-		}
-
-		regs->regs[0] = 0;		/* Don't deal with this again.  */
-	}
-
 	if (sig_uses_siginfo(ka))
 		ret = abi->setup_rt_frame(vdso + abi->rt_signal_return_offset,
 					  ka, regs, sig, oldset, info);

@@ -609,6 +588,9 @@ static void do_signal(struct pt_regs *regs)
 		oldset = &current->blocked;
 
 	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
+
+	handle_syscall_restart(regs, &ka, (signr > 0));
+
 	if (signr > 0) {
 		/* Whee!  Actually deliver the signal.  */
 		if (handle_signal(signr, &info, &ka, oldset, regs) == 0) {

@@ -625,22 +607,6 @@ static void do_signal(struct pt_regs *regs)
 		return;
 	}
 
-	if (regs->regs[0]) {
-		if (regs->regs[2] == ERESTARTNOHAND ||
-		    regs->regs[2] == ERESTARTSYS ||
-		    regs->regs[2] == ERESTARTNOINTR) {
-			regs->regs[2] = regs->regs[0];
-			regs->regs[7] = regs->regs[26];
-			regs->cp0_epc -= 4;
-		}
-		if (regs->regs[2] == ERESTART_RESTARTBLOCK) {
-			regs->regs[2] = current->thread.abi->restart;
-			regs->regs[7] = regs->regs[26];
-			regs->cp0_epc -= 4;
-		}
-		regs->regs[0] = 0;	/* Don't deal with this again.  */
-	}
-
 	/*
 	 * If there's no signal to deliver, we just put the saved sigmask
 	 * back

Signature · 1 line

1.7.5.4

Ralf Baechle

2011-11-04 09:32 UTC #8 lore raw

Re: [PATCH RFC 4/8] mips: implement syscall restart generically

Quoted 10 lines · Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.
The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

Quoted 5 lines

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:
i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Nice cleanup.

Any reason why you add empty version of syscall_get_arguments and syscall_set_version? A non-functional version that causes a silent failure is way worse than something that fails at compile time.

Ralf

Jonas Bonn

2011-11-04 09:50 UTC #9 lore raw

Re: [PATCH RFC 4/8] mips: implement syscall restart generically

Quoted 6 lines · Ralf Baechle

Nice cleanup.
Any reason why you add empty version of syscall_get_arguments and syscall_set_version? A non-functional version that causes a silent failure is way worse than something that fails at compile time.

Good point... I'll remove those empty functions for this patch series and look into doing a proper implementation for them later.

Thanks for the review.

/Jonas

Jonas Bonn

author

2011-10-23 10:20 UTC #10 lore raw

RFC PATCH 5/8 x86: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc [email protected]

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/x86/include/asm/syscall.h |   21 +++++++++++++++++++
 arch/x86/kernel/signal.c       |   43 ++-------------------------------------
 2 files changed, 24 insertions(+), 40 deletions(-)

arch/x86/include/asm/syscall.h +21 -0

index c4a348f..bd7e0f6 100644
@@ -15,6 +15,7 @@
 
 #include <linux/sched.h>
 #include <linux/err.h>
+#include <asm/unistd.h>
 
 extern const unsigned long sys_call_table[];

@@ -34,6 +35,26 @@ static inline void syscall_rollback(struct task_struct *task,
 	regs->ax = regs->orig_ax;
 }
 
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->orig_ax = -1;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->ip -= 2;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->ax = __NR_restart_syscall;
+	regs->ip -= 2;
+}
+
 static inline long syscall_get_error(struct task_struct *task,
 				     struct pt_regs *regs)
 {

arch/x86/kernel/signal.c +3 ~~-40~~

61 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:20 UTC #11 lore raw

RFC PATCH 6/8 m68k: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc [email protected]

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/m68k/include/asm/syscall.h |   87 +++++++++++++++++++++++++++++++++++++++
 arch/m68k/kernel/signal_mm.c    |   45 +-------------------
 arch/m68k/kernel/signal_no.c    |   46 +-------------------
 3 files changed, 93 insertions(+), 85 deletions(-)
 create mode 100644 arch/m68k/include/asm/syscall.h

arch/m68k/include/asm/syscall.h new file +87 -0

new file mode 100644
index 0000000..a482b7b
@@ -0,0 +1,87 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Copyright (C) 2011 Jonas Bonn <[email protected]>
+ */
+
+#ifndef _M68K_SYSCALL_H
+#define _M68K_SYSCALL_H
+
+#include <linux/err.h>
+#include <linux/sched.h>
+#include <asm/unistd.h>
+
+static inline int
+syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
+{
+	if (regs->orig_d0 >= 0)
+		return regs->orig_d0;
+	else
+		return -1;
+}
+
+static inline void
+syscall_rollback(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->d0 = regs->orig_d0;
+}
+
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->orig_d0 = -1;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->pc -= 2;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->d0 = __NR_restart_syscall;
+	regs->pc -= 2;
+}
+
+static inline long
+syscall_get_error(struct task_struct *task, struct pt_regs *regs)
+{
+	return IS_ERR_VALUE(regs->d0) ? regs->d0 : 0;
+}
+
+static inline long
+syscall_get_return_value(struct task_struct *task, struct pt_regs *regs)
+{
+	return regs->d0;
+}
+
+static inline void
+syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
+			 int error, long val)
+{
+	regs->d0 = (long) error ?: val;
+}
+
+#if 0
+static inline void
+syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned int i, unsigned int n, unsigned long *args)
+{
+	/* TODO */
+}
+
+static inline void
+syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned int i, unsigned int n, const unsigned long *args)
+{
+	/* TODO */
+}
+#endif
+
+#endif

arch/m68k/kernel/signal_mm.c +3 ~~-42~~

69 lines not on this page · raw message

arch/m68k/kernel/signal_no.c +3 ~~-43~~

70 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:20 UTC #12 lore raw

RFC PATCH 6/8 m68k: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc [email protected]

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/m68k/include/asm/syscall.h |   87 +++++++++++++++++++++++++++++++++++++++
 arch/m68k/kernel/signal_mm.c    |   45 +-------------------
 arch/m68k/kernel/signal_no.c    |   46 +-------------------
 3 files changed, 93 insertions(+), 85 deletions(-)
 create mode 100644 arch/m68k/include/asm/syscall.h

arch/m68k/include/asm/syscall.h new file +87 -0

new file mode 100644
index 0000000..a482b7b
@@ -0,0 +1,87 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Copyright (C) 2011 Jonas Bonn <[email protected]>
+ */
+
+#ifndef _M68K_SYSCALL_H
+#define _M68K_SYSCALL_H
+
+#include <linux/err.h>
+#include <linux/sched.h>
+#include <asm/unistd.h>
+
+static inline int
+syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
+{
+	if (regs->orig_d0 >= 0)
+		return regs->orig_d0;
+	else
+		return -1;
+}
+
+static inline void
+syscall_rollback(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->d0 = regs->orig_d0;
+}
+
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->orig_d0 = -1;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->pc -= 2;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->d0 = __NR_restart_syscall;
+	regs->pc -= 2;
+}
+
+static inline long
+syscall_get_error(struct task_struct *task, struct pt_regs *regs)
+{
+	return IS_ERR_VALUE(regs->d0) ? regs->d0 : 0;
+}
+
+static inline long
+syscall_get_return_value(struct task_struct *task, struct pt_regs *regs)
+{
+	return regs->d0;
+}
+
+static inline void
+syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
+			 int error, long val)
+{
+	regs->d0 = (long) error ?: val;
+}
+
+#if 0
+static inline void
+syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned int i, unsigned int n, unsigned long *args)
+{
+	/* TODO */
+}
+
+static inline void
+syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
+		      unsigned int i, unsigned int n, const unsigned long *args)
+{
+	/* TODO */
+}
+#endif
+
+#endif

arch/m68k/kernel/signal_mm.c +3 ~~-42~~

69 lines not on this page · raw message

arch/m68k/kernel/signal_no.c +3 ~~-43~~

70 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:20 UTC #13 lore raw

RFC PATCH 7/8 ia64: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc [email protected]

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/ia64/include/asm/syscall.h |   21 +++++++++++++++
 arch/ia64/kernel/signal.c       |   54 +--------------------------------------
 2 files changed, 22 insertions(+), 53 deletions(-)

arch/ia64/include/asm/syscall.h +21 -0

index a7ff1c6..ee18042 100644
@@ -15,6 +15,7 @@
 
 #include <linux/sched.h>
 #include <linux/err.h>
+#include <asm/unistd.h>
 
 static inline long syscall_get_nr(struct task_struct *task,
 				  struct pt_regs *regs)

@@ -31,6 +32,26 @@ static inline void syscall_rollback(struct task_struct *task,
 	/* do nothing */
 }
 
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->cr_ifs |= (1UL << 63);
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	ia64_decrement_ip(regs);
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->r15 = __NR_restart_syscall;
+	ia64_decrement_ip(regs);
+}
+
 static inline long syscall_get_error(struct task_struct *task,
 				     struct pt_regs *regs)
 {

arch/ia64/kernel/signal.c +1 ~~-53~~

76 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Jonas Bonn

author

2011-10-23 10:20 UTC #14 lore raw

RFC PATCH 8/8 tile: implement syscall restart generically

In reply to Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.

The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:

i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart

Cc Chris Metcalf <[email protected]>

Signed-off-by Jonas Bonn <[email protected]>

---
 arch/tile/include/asm/syscall.h |   28 ++++++++++++++++++++-
 arch/tile/kernel/signal.c       |   52 +++------------------------------------
 2 files changed, 31 insertions(+), 49 deletions(-)

arch/tile/include/asm/syscall.h +27 -1

index d35e0dc..428e28c 100644
@@ -21,6 +21,7 @@
 #include <linux/sched.h>
 #include <linux/err.h>
 #include <arch/abi.h>
+#include <asm/unistd.h>
 
 /*
  * Only the low 32 bits of orig_r0 are meaningful, so we return int.

@@ -29,15 +30,40 @@
  */
 static inline int syscall_get_nr(struct task_struct *t, struct pt_regs *regs)
 {
-	return regs->regs[TREG_SYSCALL_NR];
+	if (regs->faultnum == INT_SWINT_1)
+		return regs->regs[TREG_SYSCALL_NR];
+	else
+		return -1;
 }
 
 static inline void syscall_rollback(struct task_struct *task,
 				    struct pt_regs *regs)
 {
+	regs->flags |= PT_FLAGS_CALLER_SAVES;
 	regs->regs[0] = regs->orig_r0;
 }
 
+static inline void
+syscall_clear(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->faultnum = INT_SWINT_1_SIGRETURN;
+}
+
+static inline void
+syscall_restart(struct task_struct *task, struct pt_regs *regs)
+{
+	syscall_rollback(task, regs);
+	regs->pc -= 8;
+}
+
+static inline void
+syscall_do_restartblock(struct task_struct *task, struct pt_regs *regs)
+{
+	regs->flags |= PT_FLAGS_CALLER_SAVES;
+	regs->gpr[TREG_SYSCALL_NR] = __NR_restart_syscall;
+	regs->pc -= 8;
+}
+
 static inline long syscall_get_error(struct task_struct *task,
 				     struct pt_regs *regs)
 {

arch/tile/kernel/signal.c +4 ~~-48~~

76 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Chris Metcalf

2011-10-31 15:48 UTC #15 lore raw

Re: [PATCH RFC 8/8] tile: implement syscall restart generically

Quoted 10 lines · Jonas Bonn

Manipulating task state to effect re-execution of an interrupted syscall used to be purely architecture specific code. However, as most arch's were essentially just making minor adjustments to almost identical logic, this code could be moved to a common implementation.
The generic variant introduces the function handle_syscall_restart() to be called after get_signal_to_deliver(). The architecture specific register manipulations required to effect the actual restart are now implemented in the generic syscall interface found in asm/syscall.h

Quoted 8 lines

This patch transitions this architecture's signal handling code over to using the generic syscall restart code by:
i) Implementing the register manipulations in asm/syscall.h ii) Replacing the restart logic with a call to handle_syscall_restart
Cc Chris Metcalf <[email protected]>
Signed-off-by Jonas Bonn <[email protected]>

I like the idea, though the patch isn't quite right:

- You need to move the definition of INT_SWINT_1_SIGRETURN from <asm/sigframe.h> to <asm/syscall.h>. You then have to add #includes of <asm/syscall.h> to signal.c and stack.c in arch/tile/kernel. (And probably use syscall_clear() in signal.c.)

- In syscall_do_restartblock() you need to use regs->regs[], not regs->gprs[].

Signature · 2 lines

Chris Metcalf, Tilera Corp.
http://www.tilera.com