All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/10] Clean up signal code [take #3]
@ 2007-02-05 14:24 Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 1/10] signals: reduce {setup,restore}_sigcontext sizes Franck Bui-Huu
                   ` (10 more replies)
  0 siblings, 11 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips

Hi Ralf,

Here is the third version of this patchset.
I ran (on a 32-bits kernel _only_) all LTP signal testcases and
they all passed. I haven't time to look at what these tests exactly
do though. 

Changes from take #2:
---------------------
- Do not use save_static_function() anymore
- do not inline handle_signal()

Changes from take #1:
---------------------
- Fix ICACHE_REFILLS_WORKAROUND_WAR usages
- Do not save/restore cp0_status register anymore
- Check impact on performances

Please, consider.

		Franck

---
 arch/mips/kernel/signal-common.h |  194 +++++-----------------
 arch/mips/kernel/signal.c        |  231 +++++++++++++++++++-------
 arch/mips/kernel/signal32.c      |  341 +++++++++++++++-----------------------
 arch/mips/kernel/signal_n32.c    |   34 ++--
 4 files changed, 361 insertions(+), 439 deletions(-)

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

* [PATCH 1/10] signals: reduce {setup,restore}_sigcontext sizes
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 2/10] signal: do not inline functions in signal-common.h Franck Bui-Huu
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

This trivial change reduces considerably code size of these
2 functions callers. For instance, here is the figures for
arch/kernel/signal.o objects:

   text    data     bss     dec     hex filename
  11972       0       0   11972    2ec4 arch/mips/kernel/signal.o~old
   5380       0       0    5380    1504 arch/mips/kernel/signal.o~new

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal-common.h |   66 ++++++++++++--------------------------
 1 files changed, 21 insertions(+), 45 deletions(-)

diff --git a/arch/mips/kernel/signal-common.h b/arch/mips/kernel/signal-common.h
index b1f09d5..bb3c631 100644
--- a/arch/mips/kernel/signal-common.h
+++ b/arch/mips/kernel/signal-common.h
@@ -13,22 +13,13 @@ static inline int
 setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 {
 	int err = 0;
+	int i;
 
 	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
 
-#define save_gp_reg(i) do {						\
-	err |= __put_user(regs->regs[i], &sc->sc_regs[i]);		\
-} while(0)
-	__put_user(0, &sc->sc_regs[0]); save_gp_reg(1); save_gp_reg(2);
-	save_gp_reg(3); save_gp_reg(4); save_gp_reg(5); save_gp_reg(6);
-	save_gp_reg(7); save_gp_reg(8); save_gp_reg(9); save_gp_reg(10);
-	save_gp_reg(11); save_gp_reg(12); save_gp_reg(13); save_gp_reg(14);
-	save_gp_reg(15); save_gp_reg(16); save_gp_reg(17); save_gp_reg(18);
-	save_gp_reg(19); save_gp_reg(20); save_gp_reg(21); save_gp_reg(22);
-	save_gp_reg(23); save_gp_reg(24); save_gp_reg(25); save_gp_reg(26);
-	save_gp_reg(27); save_gp_reg(28); save_gp_reg(29); save_gp_reg(30);
-	save_gp_reg(31);
-#undef save_gp_reg
+	err |= __put_user(0, &sc->sc_regs[0]);
+	for (i = 1; i < 32; i++)
+		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
 
 	err |= __put_user(regs->hi, &sc->sc_mdhi);
 	err |= __put_user(regs->lo, &sc->sc_mdlo);
@@ -44,24 +35,21 @@ setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 
 	err |= __put_user(!!used_math(), &sc->sc_used_math);
 
-	if (!used_math())
-		goto out;
-
-	/*
-	 * Save FPU state to signal context.  Signal handler will "inherit"
-	 * current FPU state.
-	 */
-	preempt_disable();
-
-	if (!is_fpu_owner()) {
-		own_fpu();
-		restore_fp(current);
+	if (used_math()) {
+		/*
+		 * Save FPU state to signal context.  Signal handler
+		 * will "inherit" current FPU state.
+		 */
+		preempt_disable();
+
+		if (!is_fpu_owner()) {
+			own_fpu();
+			restore_fp(current);
+		}
+		err |= save_fp_context(sc);
+
+		preempt_enable();
 	}
-	err |= save_fp_context(sc);
-
-	preempt_enable();
-
-out:
 	return err;
 }
 
@@ -71,6 +59,7 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 	unsigned int used_math;
 	unsigned long treg;
 	int err = 0;
+	int i;
 
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
@@ -88,21 +77,8 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
 		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
 	}
 
-#define restore_gp_reg(i) do {						\
-	err |= __get_user(regs->regs[i], &sc->sc_regs[i]);		\
-} while(0)
-	restore_gp_reg( 1); restore_gp_reg( 2); restore_gp_reg( 3);
-	restore_gp_reg( 4); restore_gp_reg( 5); restore_gp_reg( 6);
-	restore_gp_reg( 7); restore_gp_reg( 8); restore_gp_reg( 9);
-	restore_gp_reg(10); restore_gp_reg(11); restore_gp_reg(12);
-	restore_gp_reg(13); restore_gp_reg(14); restore_gp_reg(15);
-	restore_gp_reg(16); restore_gp_reg(17); restore_gp_reg(18);
-	restore_gp_reg(19); restore_gp_reg(20); restore_gp_reg(21);
-	restore_gp_reg(22); restore_gp_reg(23); restore_gp_reg(24);
-	restore_gp_reg(25); restore_gp_reg(26); restore_gp_reg(27);
-	restore_gp_reg(28); restore_gp_reg(29); restore_gp_reg(30);
-	restore_gp_reg(31);
-#undef restore_gp_reg
+	for (i = 1; i < 32; i++)
+		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
 
 	err |= __get_user(used_math, &sc->sc_used_math);
 	conditional_used_math(used_math);
-- 
1.4.4.3.ge6d4

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

* [PATCH 2/10] signal: do not inline functions in signal-common.h
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 1/10] signals: reduce {setup,restore}_sigcontext sizes Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 3/10] signal: clean up sigframe structure Franck Bui-Huu
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

These functions are quite big and there are no points to make
them inlined. So this patch moves the functions implementation
in signal.c and make them available for others source files
which need them.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal-common.h |  150 ++++----------------------------------
 arch/mips/kernel/signal.c        |  139 +++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 136 deletions(-)

diff --git a/arch/mips/kernel/signal-common.h b/arch/mips/kernel/signal-common.h
index bb3c631..03d2b60 100644
--- a/arch/mips/kernel/signal-common.h
+++ b/arch/mips/kernel/signal-common.h
@@ -8,145 +8,23 @@
  * Copyright (C) 1999, 2000 Silicon Graphics, Inc.
  */
 
+#ifndef __SIGNAL_COMMON_H
+#define __SIGNAL_COMMON_H
 
-static inline int
-setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
-{
-	int err = 0;
-	int i;
-
-	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
-
-	err |= __put_user(0, &sc->sc_regs[0]);
-	for (i = 1; i < 32; i++)
-		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
-
-	err |= __put_user(regs->hi, &sc->sc_mdhi);
-	err |= __put_user(regs->lo, &sc->sc_mdlo);
-	if (cpu_has_dsp) {
-		err |= __put_user(mfhi1(), &sc->sc_hi1);
-		err |= __put_user(mflo1(), &sc->sc_lo1);
-		err |= __put_user(mfhi2(), &sc->sc_hi2);
-		err |= __put_user(mflo2(), &sc->sc_lo2);
-		err |= __put_user(mfhi3(), &sc->sc_hi3);
-		err |= __put_user(mflo3(), &sc->sc_lo3);
-		err |= __put_user(rddsp(DSP_MASK), &sc->sc_dsp);
-	}
-
-	err |= __put_user(!!used_math(), &sc->sc_used_math);
-
-	if (used_math()) {
-		/*
-		 * Save FPU state to signal context.  Signal handler
-		 * will "inherit" current FPU state.
-		 */
-		preempt_disable();
-
-		if (!is_fpu_owner()) {
-			own_fpu();
-			restore_fp(current);
-		}
-		err |= save_fp_context(sc);
-
-		preempt_enable();
-	}
-	return err;
-}
-
-static inline int
-restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
-{
-	unsigned int used_math;
-	unsigned long treg;
-	int err = 0;
-	int i;
-
-	/* Always make any pending restarted system calls return -EINTR */
-	current_thread_info()->restart_block.fn = do_no_restart_syscall;
-
-	err |= __get_user(regs->cp0_epc, &sc->sc_pc);
-	err |= __get_user(regs->hi, &sc->sc_mdhi);
-	err |= __get_user(regs->lo, &sc->sc_mdlo);
-	if (cpu_has_dsp) {
-		err |= __get_user(treg, &sc->sc_hi1); mthi1(treg);
-		err |= __get_user(treg, &sc->sc_lo1); mtlo1(treg);
-		err |= __get_user(treg, &sc->sc_hi2); mthi2(treg);
-		err |= __get_user(treg, &sc->sc_lo2); mtlo2(treg);
-		err |= __get_user(treg, &sc->sc_hi3); mthi3(treg);
-		err |= __get_user(treg, &sc->sc_lo3); mtlo3(treg);
-		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
-	}
-
-	for (i = 1; i < 32; i++)
-		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
-
-	err |= __get_user(used_math, &sc->sc_used_math);
-	conditional_used_math(used_math);
-
-	preempt_disable();
-
-	if (used_math()) {
-		/* restore fpu context if we have used it before */
-		own_fpu();
-		err |= restore_fp_context(sc);
-	} else {
-		/* signal handler may have used FPU.  Give it up. */
-		lose_fpu();
-	}
-
-	preempt_enable();
-
-	return err;
-}
+/*
+ * handle hardware context
+ */
+extern int setup_sigcontext(struct pt_regs *, struct sigcontext __user *);
+extern int restore_sigcontext(struct pt_regs *, struct sigcontext __user *);
 
 /*
  * Determine which stack to use..
  */
-static inline void __user *
-get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size)
-{
-	unsigned long sp;
-
-	/* Default to using normal stack */
-	sp = regs->regs[29];
-
-	/*
-	 * FPU emulator may have it's own trampoline active just
-	 * above the user stack, 16-bytes before the next lowest
-	 * 16 byte boundary.  Try to avoid trashing it.
-	 */
-	sp -= 32;
-
-	/* This is the X/Open sanctioned signal stack switching.  */
-	if ((ka->sa.sa_flags & SA_ONSTACK) && (sas_ss_flags (sp) == 0))
-		sp = current->sas_ss_sp + current->sas_ss_size;
-
-	return (void __user *)((sp - frame_size) & (ICACHE_REFILLS_WORKAROUND_WAR ? ~(cpu_icache_line_size()-1) : ALMASK));
-}
-
-static inline int install_sigtramp(unsigned int __user *tramp,
-	unsigned int syscall)
-{
-	int err;
-
-	/*
-	 * Set up the return code ...
-	 *
-	 *         li      v0, __NR__foo_sigreturn
-	 *         syscall
-	 */
-
-	err = __put_user(0x24020000 + syscall, tramp + 0);
-	err |= __put_user(0x0000000c          , tramp + 1);
-	if (ICACHE_REFILLS_WORKAROUND_WAR) {
-		err |= __put_user(0, tramp + 2);
-		err |= __put_user(0, tramp + 3);
-		err |= __put_user(0, tramp + 4);
-		err |= __put_user(0, tramp + 5);
-		err |= __put_user(0, tramp + 6);
-		err |= __put_user(0, tramp + 7);
-	}
-	flush_cache_sigtramp((unsigned long) tramp);
+extern void __user *get_sigframe(struct k_sigaction *ka, struct pt_regs *regs,
+				 size_t frame_size);
+/*
+ * install trampoline code to get back from the sig handler
+ */
+extern int install_sigtramp(unsigned int __user *tramp, unsigned int syscall);
 
-	return err;
-}
+#endif	/* __SIGNAL_COMMON_H */
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index b9d358e..7ec73f2 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -39,6 +39,145 @@
 #define _BLOCKABLE (~(sigmask(SIGKILL) | sigmask(SIGSTOP)))
 
 /*
+ * Helper routines
+ */
+int setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
+{
+	int err = 0;
+	int i;
+
+	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
+
+	err |= __put_user(0, &sc->sc_regs[0]);
+	for (i = 1; i < 32; i++)
+		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
+
+	err |= __put_user(regs->hi, &sc->sc_mdhi);
+	err |= __put_user(regs->lo, &sc->sc_mdlo);
+	if (cpu_has_dsp) {
+		err |= __put_user(mfhi1(), &sc->sc_hi1);
+		err |= __put_user(mflo1(), &sc->sc_lo1);
+		err |= __put_user(mfhi2(), &sc->sc_hi2);
+		err |= __put_user(mflo2(), &sc->sc_lo2);
+		err |= __put_user(mfhi3(), &sc->sc_hi3);
+		err |= __put_user(mflo3(), &sc->sc_lo3);
+		err |= __put_user(rddsp(DSP_MASK), &sc->sc_dsp);
+	}
+
+	err |= __put_user(!!used_math(), &sc->sc_used_math);
+
+	if (used_math()) {
+		/*
+		 * Save FPU state to signal context. Signal handler
+		 * will "inherit" current FPU state.
+		 */
+		preempt_disable();
+
+		if (!is_fpu_owner()) {
+			own_fpu();
+			restore_fp(current);
+		}
+		err |= save_fp_context(sc);
+
+		preempt_enable();
+	}
+	return err;
+}
+
+int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc)
+{
+	unsigned int used_math;
+	unsigned long treg;
+	int err = 0;
+	int i;
+
+	/* Always make any pending restarted system calls return -EINTR */
+	current_thread_info()->restart_block.fn = do_no_restart_syscall;
+
+	err |= __get_user(regs->cp0_epc, &sc->sc_pc);
+	err |= __get_user(regs->hi, &sc->sc_mdhi);
+	err |= __get_user(regs->lo, &sc->sc_mdlo);
+	if (cpu_has_dsp) {
+		err |= __get_user(treg, &sc->sc_hi1); mthi1(treg);
+		err |= __get_user(treg, &sc->sc_lo1); mtlo1(treg);
+		err |= __get_user(treg, &sc->sc_hi2); mthi2(treg);
+		err |= __get_user(treg, &sc->sc_lo2); mtlo2(treg);
+		err |= __get_user(treg, &sc->sc_hi3); mthi3(treg);
+		err |= __get_user(treg, &sc->sc_lo3); mtlo3(treg);
+		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
+	}
+
+	for (i = 1; i < 32; i++)
+		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
+
+	err |= __get_user(used_math, &sc->sc_used_math);
+	conditional_used_math(used_math);
+
+	preempt_disable();
+
+	if (used_math()) {
+		/* restore fpu context if we have used it before */
+		own_fpu();
+		err |= restore_fp_context(sc);
+	} else {
+		/* signal handler may have used FPU.  Give it up. */
+		lose_fpu();
+	}
+
+	preempt_enable();
+
+	return err;
+}
+
+void __user *get_sigframe(struct k_sigaction *ka, struct pt_regs *regs,
+			  size_t frame_size)
+{
+	unsigned long sp;
+
+	/* Default to using normal stack */
+	sp = regs->regs[29];
+
+	/*
+	 * FPU emulator may have it's own trampoline active just
+	 * above the user stack, 16-bytes before the next lowest
+	 * 16 byte boundary.  Try to avoid trashing it.
+	 */
+	sp -= 32;
+
+	/* This is the X/Open sanctioned signal stack switching.  */
+	if ((ka->sa.sa_flags & SA_ONSTACK) && (sas_ss_flags (sp) == 0))
+		sp = current->sas_ss_sp + current->sas_ss_size;
+
+	return (void __user *)((sp - frame_size) & (ICACHE_REFILLS_WORKAROUND_WAR ? ~(cpu_icache_line_size()-1) : ALMASK));
+}
+
+int install_sigtramp(unsigned int __user *tramp, unsigned int syscall)
+{
+	int err;
+
+	/*
+	 * Set up the return code ...
+	 *
+	 *         li      v0, __NR__foo_sigreturn
+	 *         syscall
+	 */
+
+	err = __put_user(0x24020000 + syscall, tramp + 0);
+	err |= __put_user(0x0000000c          , tramp + 1);
+	if (ICACHE_REFILLS_WORKAROUND_WAR) {
+		err |= __put_user(0, tramp + 2);
+		err |= __put_user(0, tramp + 3);
+		err |= __put_user(0, tramp + 4);
+		err |= __put_user(0, tramp + 5);
+		err |= __put_user(0, tramp + 6);
+		err |= __put_user(0, tramp + 7);
+	}
+	flush_cache_sigtramp((unsigned long) tramp);
+
+	return err;
+}
+
+/*
  * Atomically swap in the new signal mask, and wait for a signal.
  */
 
-- 
1.4.4.3.ge6d4

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

* [PATCH 3/10] signal: clean up sigframe structure
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 1/10] signals: reduce {setup,restore}_sigcontext sizes Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 2/10] signal: do not inline functions in signal-common.h Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 4/10] signal32: remove code duplication Franck Bui-Huu
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

This patch makes 'struct sigframe' declaration avalaible for all signals
code. It allows signal32 to not have its own declaration.

This patch also removes all ICACHE_REFILLS_WORKAROUND_WAR tests in
structure declaration and hopefully make them more readable.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal-common.h |   26 +++++++++++++++++
 arch/mips/kernel/signal.c        |   56 ++++++++++++++-----------------------
 arch/mips/kernel/signal32.c      |   49 ++++++++++++++-------------------
 arch/mips/kernel/signal_n32.c    |   19 +++++++++----
 4 files changed, 81 insertions(+), 69 deletions(-)

diff --git a/arch/mips/kernel/signal-common.h b/arch/mips/kernel/signal-common.h
index 03d2b60..6700bde 100644
--- a/arch/mips/kernel/signal-common.h
+++ b/arch/mips/kernel/signal-common.h
@@ -12,6 +12,32 @@
 #define __SIGNAL_COMMON_H
 
 /*
+ * Horribly complicated - with the bloody RM9000 workarounds enabled
+ * the signal trampolines is moving to the end of the structure so we can
+ * increase the alignment without breaking software compatibility.
+ */
+#if ICACHE_REFILLS_WORKAROUND_WAR == 0
+
+struct sigframe {
+	u32 sf_ass[4];		/* argument save space for o32 */
+	u32 sf_code[2];		/* signal trampoline */
+	struct sigcontext sf_sc;
+	sigset_t sf_mask;
+};
+
+#else  /* ICACHE_REFILLS_WORKAROUND_WAR */
+
+struct sigframe {
+	u32 sf_ass[4];			/* argument save space for o32 */
+	u32 sf_pad[2];
+	struct sigcontext sf_sc;	/* hw context */
+	sigset_t sf_mask;
+	u32 sf_code[8] ____cacheline_aligned;	/* signal trampoline */
+};
+
+#endif	/* !ICACHE_REFILLS_WORKAROUND_WAR */
+
+/*
  * handle hardware context
  */
 extern int setup_sigcontext(struct pt_regs *, struct sigcontext __user *);
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 7ec73f2..41033be 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -38,6 +38,27 @@
 
 #define _BLOCKABLE (~(sigmask(SIGKILL) | sigmask(SIGSTOP)))
 
+#if ICACHE_REFILLS_WORKAROUND_WAR == 0
+
+struct rt_sigframe {
+	u32 rs_ass[4];		/* argument save space for o32 */
+	u32 rs_code[2];		/* signal trampoline */
+	struct siginfo rs_info;
+	struct ucontext rs_uc;
+};
+
+#else
+
+struct rt_sigframe {
+	u32 rs_ass[4];			/* argument save space for o32 */
+	u32 rs_pad[2];
+	struct siginfo rs_info;
+	struct ucontext rs_uc;
+	u32 rs_code[8] ____cacheline_aligned;	/* signal trampoline */
+};
+
+#endif
+
 /*
  * Helper routines
  */
@@ -287,41 +308,6 @@ asmlinkage int sys_sigaltstack(nabi_no_regargs struct pt_regs regs)
 	return do_sigaltstack(uss, uoss, usp);
 }
 
-/*
- * Horribly complicated - with the bloody RM9000 workarounds enabled
- * the signal trampolines is moving to the end of the structure so we can
- * increase the alignment without breaking software compatibility.
- */
-#ifdef CONFIG_TRAD_SIGNALS
-struct sigframe {
-	u32 sf_ass[4];			/* argument save space for o32 */
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 sf_pad[2];
-#else
-	u32 sf_code[2];			/* signal trampoline */
-#endif
-	struct sigcontext sf_sc;
-	sigset_t sf_mask;
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 sf_code[8] ____cacheline_aligned;	/* signal trampoline */
-#endif
-};
-#endif
-
-struct rt_sigframe {
-	u32 rs_ass[4];			/* argument save space for o32 */
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 rs_pad[2];
-#else
-	u32 rs_code[2];			/* signal trampoline */
-#endif
-	struct siginfo rs_info;
-	struct ucontext rs_uc;
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 rs_code[8] ____cacheline_aligned;	/* signal trampoline */
-#endif
-};
-
 #ifdef CONFIG_TRAD_SIGNALS
 save_static_function(sys_sigreturn);
 __attribute_used__ noinline static void
diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index c86a5dd..e0a8553 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -139,6 +139,27 @@ struct ucontext32 {
 	sigset_t32          uc_sigmask;   /* mask last for extensibility */
 };
 
+#if ICACHE_REFILLS_WORKAROUND_WAR == 0
+
+struct rt_sigframe32 {
+	u32 rs_ass[4];			/* argument save space for o32 */
+	u32 rs_code[2];			/* signal trampoline */
+	compat_siginfo_t rs_info;
+	struct ucontext32 rs_uc;
+};
+
+#else  /* ICACHE_REFILLS_WORKAROUND_WAR */
+
+struct rt_sigframe32 {
+	u32 rs_ass[4];			/* argument save space for o32 */
+	u32 rs_pad[2];
+	compat_siginfo_t rs_info;
+	struct ucontext32 rs_uc;
+	u32 rs_code[8] __attribute__((aligned(32)));	/* signal trampoline */
+};
+
+#endif	/* !ICACHE_REFILLS_WORKAROUND_WAR */
+
 extern void __put_sigset_unknown_nsig(void);
 extern void __get_sigset_unknown_nsig(void);
 
@@ -383,34 +404,6 @@ static int restore_sigcontext32(struct pt_regs *regs, struct sigcontext32 __user
 	return err;
 }
 
-struct sigframe {
-	u32 sf_ass[4];			/* argument save space for o32 */
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 sf_pad[2];
-#else
-	u32 sf_code[2];			/* signal trampoline */
-#endif
-	struct sigcontext32 sf_sc;
-	sigset_t sf_mask;
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 sf_code[8] ____cacheline_aligned;	/* signal trampoline */
-#endif
-};
-
-struct rt_sigframe32 {
-	u32 rs_ass[4];			/* argument save space for o32 */
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 rs_pad[2];
-#else
-	u32 rs_code[2];			/* signal trampoline */
-#endif
-	compat_siginfo_t rs_info;
-	struct ucontext32 rs_uc;
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 rs_code[8] __attribute__((aligned(32)));	/* signal trampoline */
-#endif
-};
-
 int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
 {
 	int err;
diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c
index a67c185..f8e1539 100644
--- a/arch/mips/kernel/signal_n32.c
+++ b/arch/mips/kernel/signal_n32.c
@@ -66,20 +66,27 @@ struct ucontextn32 {
 	sigset_t            uc_sigmask;   /* mask last for extensibility */
 };
 
+#if ICACHE_REFILLS_WORKAROUND_WAR == 0
+
 struct rt_sigframe_n32 {
 	u32 rs_ass[4];			/* argument save space for o32 */
-#if ICACHE_REFILLS_WORKAROUND_WAR
-	u32 rs_pad[2];
-#else
 	u32 rs_code[2];			/* signal trampoline */
-#endif
 	struct siginfo rs_info;
 	struct ucontextn32 rs_uc;
-#if ICACHE_REFILLS_WORKAROUND_WAR
+};
+
+#else  /* ICACHE_REFILLS_WORKAROUND_WAR */
+
+struct rt_sigframe_n32 {
+	u32 rs_ass[4];			/* argument save space for o32 */
+	u32 rs_pad[2];
+	struct siginfo rs_info;
+	struct ucontextn32 rs_uc;
 	u32 rs_code[8] ____cacheline_aligned;		/* signal trampoline */
-#endif
 };
 
+#endif	/* !ICACHE_REFILLS_WORKAROUND_WAR */
+
 extern void sigset_from_compat (sigset_t *set, compat_sigset_t *compat);
 
 save_static_function(sysn32_rt_sigsuspend);
-- 
1.4.4.3.ge6d4

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

* [PATCH 4/10] signal32: remove code duplication
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (2 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 3/10] signal: clean up sigframe structure Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 5/10] signal: test return value of install_sigtramp() Franck Bui-Huu
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

There's no point for signal32.c to redefine get_sigframe().
It should use the one define in signal.c instead.

The same stands for install_sigtramp().

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal32.c |   50 +++---------------------------------------
 1 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index e0a8553..5934f33 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -33,6 +33,8 @@
 #include <asm/fpu.h>
 #include <asm/war.h>
 
+#include "signal-common.h"
+
 #define SI_PAD_SIZE32   ((SI_MAX_SIZE/sizeof(int)) - 3)
 
 typedef struct compat_siginfo {
@@ -604,32 +606,6 @@ out:
 	return err;
 }
 
-/*
- * Determine which stack to use..
- */
-static inline void __user *get_sigframe(struct k_sigaction *ka,
-					struct pt_regs *regs,
-					size_t frame_size)
-{
-	unsigned long sp;
-
-	/* Default to using normal stack */
-	sp = regs->regs[29];
-
-	/*
-	 * FPU emulator may have it's own trampoline active just
-	 * above the user stack, 16-bytes before the next lowest
-	 * 16 byte boundary.  Try to avoid trashing it.
-	 */
-	sp -= 32;
-
-	/* This is the X/Open sanctioned signal stack switching.  */
-	if ((ka->sa.sa_flags & SA_ONSTACK) && (sas_ss_flags (sp) == 0))
-		sp = current->sas_ss_sp + current->sas_ss_size;
-
-	return (void __user *)((sp - frame_size) & ALMASK);
-}
-
 int setup_frame_32(struct k_sigaction * ka, struct pt_regs *regs,
 	int signr, sigset_t *set)
 {
@@ -640,15 +616,7 @@ int setup_frame_32(struct k_sigaction * ka, struct pt_regs *regs,
 	if (!access_ok(VERIFY_WRITE, frame, sizeof (*frame)))
 		goto give_sigsegv;
 
-	/*
-	 * Set up the return code ...
-	 *
-	 *         li      v0, __NR_O32_sigreturn
-	 *         syscall
-	 */
-	err |= __put_user(0x24020000 + __NR_O32_sigreturn, frame->sf_code + 0);
-	err |= __put_user(0x0000000c                     , frame->sf_code + 1);
-	flush_cache_sigtramp((unsigned long) frame->sf_code);
+	err |= install_sigtramp(frame->sf_code, __NR_O32_sigreturn);
 
 	err |= setup_sigcontext32(regs, &frame->sf_sc);
 	err |= __copy_to_user(&frame->sf_mask, set, sizeof(*set));
@@ -695,17 +663,7 @@ int setup_rt_frame_32(struct k_sigaction * ka, struct pt_regs *regs,
 	if (!access_ok(VERIFY_WRITE, frame, sizeof (*frame)))
 		goto give_sigsegv;
 
-	/* Set up to return from userspace.  If provided, use a stub already
-	   in userspace.  */
-	/*
-	 * Set up the return code ...
-	 *
-	 *         li      v0, __NR_O32_rt_sigreturn
-	 *         syscall
-	 */
-	err |= __put_user(0x24020000 + __NR_O32_rt_sigreturn, frame->rs_code + 0);
-	err |= __put_user(0x0000000c                      , frame->rs_code + 1);
-	flush_cache_sigtramp((unsigned long) frame->rs_code);
+	err |= install_sigtramp(frame->rs_code, __NR_O32_rt_sigreturn);
 
 	/* Convert (siginfo_t -> compat_siginfo_t) and copy to user. */
 	err |= copy_siginfo_to_user32(&frame->rs_info, info);
-- 
1.4.4.3.ge6d4

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

* [PATCH 5/10] signal: test return value of install_sigtramp()
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (3 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 4/10] signal32: remove code duplication Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 6/10] signal: factorize debug code Franck Bui-Huu
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 41033be..5245135 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -184,7 +184,7 @@ int install_sigtramp(unsigned int __user *tramp, unsigned int syscall)
 	 */
 
 	err = __put_user(0x24020000 + syscall, tramp + 0);
-	err |= __put_user(0x0000000c          , tramp + 1);
+	err |= __put_user(0x0000000c         , tramp + 1);
 	if (ICACHE_REFILLS_WORKAROUND_WAR) {
 		err |= __put_user(0, tramp + 2);
 		err |= __put_user(0, tramp + 3);
@@ -400,7 +400,7 @@ int setup_frame(struct k_sigaction * ka, struct pt_regs *regs,
 	if (!access_ok(VERIFY_WRITE, frame, sizeof (*frame)))
 		goto give_sigsegv;
 
-	install_sigtramp(frame->sf_code, __NR_sigreturn);
+	err |= install_sigtramp(frame->sf_code, __NR_sigreturn);
 
 	err |= setup_sigcontext(regs, &frame->sf_sc);
 	err |= __copy_to_user(&frame->sf_mask, set, sizeof(*set));
@@ -447,7 +447,7 @@ int setup_rt_frame(struct k_sigaction * ka, struct pt_regs *regs,
 	if (!access_ok(VERIFY_WRITE, frame, sizeof (*frame)))
 		goto give_sigsegv;
 
-	install_sigtramp(frame->rs_code, __NR_rt_sigreturn);
+	err |= install_sigtramp(frame->rs_code, __NR_rt_sigreturn);
 
 	/* Create siginfo.  */
 	err |= copy_siginfo_to_user(&frame->rs_info, info);
-- 
1.4.4.3.ge6d4

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

* [PATCH 6/10] signal: factorize debug code
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (4 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 5/10] signal: test return value of install_sigtramp() Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 7/10] signal32: reduce {setup,restore}_sigcontext32 sizes Franck Bui-Huu
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal-common.h |    8 ++++++++
 arch/mips/kernel/signal.c        |   14 +++++---------
 arch/mips/kernel/signal32.c      |   16 ++++++----------
 arch/mips/kernel/signal_n32.c    |    7 ++-----
 4 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/arch/mips/kernel/signal-common.h b/arch/mips/kernel/signal-common.h
index 6700bde..9a8abd6 100644
--- a/arch/mips/kernel/signal-common.h
+++ b/arch/mips/kernel/signal-common.h
@@ -11,6 +11,14 @@
 #ifndef __SIGNAL_COMMON_H
 #define __SIGNAL_COMMON_H
 
+/* #define DEBUG_SIG */
+
+#ifdef DEBUG_SIG
+#  define DEBUGP(fmt, args...) printk("%s: " fmt, __FUNCTION__ , ##args)
+#else
+#  define DEBUGP(fmt, args...)
+#endif
+
 /*
  * Horribly complicated - with the bloody RM9000 workarounds enabled
  * the signal trampolines is moving to the end of the structure so we can
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 5245135..d3f6b0a 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -34,8 +34,6 @@
 
 #include "signal-common.h"
 
-#define DEBUG_SIG 0
-
 #define _BLOCKABLE (~(sigmask(SIGKILL) | sigmask(SIGSTOP)))
 
 #if ICACHE_REFILLS_WORKAROUND_WAR == 0
@@ -424,11 +422,10 @@ int setup_frame(struct k_sigaction * ka, struct pt_regs *regs,
 	regs->regs[31] = (unsigned long) frame->sf_code;
 	regs->cp0_epc = regs->regs[25] = (unsigned long) ka->sa.sa_handler;
 
-#if DEBUG_SIG
-	printk("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%p\n",
+	DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
 	       current->comm, current->pid,
-	       frame, regs->cp0_epc, frame->regs[31]);
-#endif
+	       frame, regs->cp0_epc, regs->regs[31]);
+
         return 0;
 
 give_sigsegv:
@@ -484,11 +481,10 @@ int setup_rt_frame(struct k_sigaction * ka, struct pt_regs *regs,
 	regs->regs[31] = (unsigned long) frame->rs_code;
 	regs->cp0_epc = regs->regs[25] = (unsigned long) ka->sa.sa_handler;
 
-#if DEBUG_SIG
-	printk("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%p\n",
+	DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
 	       current->comm, current->pid,
 	       frame, regs->cp0_epc, regs->regs[31]);
-#endif
+
 	return 0;
 
 give_sigsegv:
diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index 5934f33..1a99a57 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -104,8 +104,6 @@ typedef struct compat_siginfo {
 #define __NR_O32_rt_sigreturn		4193
 #define __NR_O32_restart_syscall	4253
 
-#define DEBUG_SIG 0
-
 #define _BLOCKABLE (~(sigmask(SIGKILL) | sigmask(SIGSTOP)))
 
 /* 32-bit compatibility types */
@@ -640,11 +638,10 @@ int setup_frame_32(struct k_sigaction * ka, struct pt_regs *regs,
 	regs->regs[31] = (unsigned long) frame->sf_code;
 	regs->cp0_epc = regs->regs[25] = (unsigned long) ka->sa.sa_handler;
 
-#if DEBUG_SIG
-	printk("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%p\n",
+	DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
 	       current->comm, current->pid,
-	       frame, regs->cp0_epc, frame->sf_code);
-#endif
+	       frame, regs->cp0_epc, regs->regs[31]);
+
 	return 0;
 
 give_sigsegv:
@@ -701,11 +698,10 @@ int setup_rt_frame_32(struct k_sigaction * ka, struct pt_regs *regs,
 	regs->regs[31] = (unsigned long) frame->rs_code;
 	regs->cp0_epc = regs->regs[25] = (unsigned long) ka->sa.sa_handler;
 
-#if DEBUG_SIG
-	printk("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%p\n",
+	DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
 	       current->comm, current->pid,
-	       frame, regs->cp0_epc, frame->rs_code);
-#endif
+	       frame, regs->cp0_epc, regs->regs[31]);
+
 	return 0;
 
 give_sigsegv:
diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c
index f8e1539..a6c1e67 100644
--- a/arch/mips/kernel/signal_n32.c
+++ b/arch/mips/kernel/signal_n32.c
@@ -47,8 +47,6 @@
 #define __NR_N32_rt_sigreturn		6211
 #define __NR_N32_restart_syscall	6214
 
-#define DEBUG_SIG 0
-
 #define _BLOCKABLE (~(sigmask(SIGKILL) | sigmask(SIGSTOP)))
 
 /* IRIX compatible stack_t  */
@@ -221,11 +219,10 @@ int setup_rt_frame_n32(struct k_sigaction * ka,
 	regs->regs[31] = (unsigned long) frame->rs_code;
 	regs->cp0_epc = regs->regs[25] = (unsigned long) ka->sa.sa_handler;
 
-#if DEBUG_SIG
-	printk("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%p\n",
+	DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
 	       current->comm, current->pid,
 	       frame, regs->cp0_epc, regs->regs[31]);
-#endif
+
 	return 0;
 
 give_sigsegv:
-- 
1.4.4.3.ge6d4

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

* [PATCH 7/10] signal32: reduce {setup,restore}_sigcontext32 sizes
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (5 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 6/10] signal: factorize debug code Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 8/10] signal32: no need to save c0_status register in setup_sigcontext32() Franck Bui-Huu
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

This trivial changes should decrease a lot the size of these
2 functions.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal32.c |  211 ++++++++++++++++++++-----------------------
 1 files changed, 97 insertions(+), 114 deletions(-)

diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index 1a99a57..5d102ef 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -160,6 +160,103 @@ struct rt_sigframe32 {
 
 #endif	/* !ICACHE_REFILLS_WORKAROUND_WAR */
 
+/*
+ * sigcontext handlers
+ */
+static int setup_sigcontext32(struct pt_regs *regs,
+			      struct sigcontext32 __user *sc)
+{
+	int err = 0;
+	int i;
+
+	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
+	err |= __put_user(regs->cp0_status, &sc->sc_status);
+
+	err |= __put_user(0, &sc->sc_regs[0]);
+	for (i = 1; i < 32; i++)
+		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
+
+	err |= __put_user(regs->hi, &sc->sc_mdhi);
+	err |= __put_user(regs->lo, &sc->sc_mdlo);
+	if (cpu_has_dsp) {
+		err |= __put_user(rddsp(DSP_MASK), &sc->sc_dsp);
+		err |= __put_user(mfhi1(), &sc->sc_hi1);
+		err |= __put_user(mflo1(), &sc->sc_lo1);
+		err |= __put_user(mfhi2(), &sc->sc_hi2);
+		err |= __put_user(mflo2(), &sc->sc_lo2);
+		err |= __put_user(mfhi3(), &sc->sc_hi3);
+		err |= __put_user(mflo3(), &sc->sc_lo3);
+	}
+
+	err |= __put_user(!!used_math(), &sc->sc_used_math);
+
+	if (used_math()) {
+		/*
+		 * Save FPU state to signal context.  Signal handler
+		 * will "inherit" current FPU state.
+		 */
+		preempt_disable();
+
+		if (!is_fpu_owner()) {
+			own_fpu();
+			restore_fp(current);
+		}
+		err |= save_fp_context32(sc);
+
+		preempt_enable();
+	}
+	return err;
+}
+
+static int restore_sigcontext32(struct pt_regs *regs,
+				struct sigcontext32 __user *sc)
+{
+	u32 used_math;
+	int err = 0;
+	s32 treg;
+	int i;
+
+	/* Always make any pending restarted system calls return -EINTR */
+	current_thread_info()->restart_block.fn = do_no_restart_syscall;
+
+	err |= __get_user(regs->cp0_epc, &sc->sc_pc);
+	err |= __get_user(regs->hi, &sc->sc_mdhi);
+	err |= __get_user(regs->lo, &sc->sc_mdlo);
+	if (cpu_has_dsp) {
+		err |= __get_user(treg, &sc->sc_hi1); mthi1(treg);
+		err |= __get_user(treg, &sc->sc_lo1); mtlo1(treg);
+		err |= __get_user(treg, &sc->sc_hi2); mthi2(treg);
+		err |= __get_user(treg, &sc->sc_lo2); mtlo2(treg);
+		err |= __get_user(treg, &sc->sc_hi3); mthi3(treg);
+		err |= __get_user(treg, &sc->sc_lo3); mtlo3(treg);
+		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
+	}
+
+	for (i = 1; i < 32; i++)
+		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
+
+	err |= __get_user(used_math, &sc->sc_used_math);
+	conditional_used_math(used_math);
+
+	preempt_disable();
+
+	if (used_math()) {
+		/* restore fpu context if we have used it before */
+		own_fpu();
+		err |= restore_fp_context32(sc);
+	} else {
+		/* signal handler may have used FPU.  Give it up. */
+		lose_fpu();
+	}
+
+	preempt_enable();
+
+	return err;
+}
+
+/*
+ *
+ */
 extern void __put_sigset_unknown_nsig(void);
 extern void __get_sigset_unknown_nsig(void);
 
@@ -347,63 +444,6 @@ asmlinkage int sys32_sigaltstack(nabi_no_regargs struct pt_regs regs)
 	return ret;
 }
 
-static int restore_sigcontext32(struct pt_regs *regs, struct sigcontext32 __user *sc)
-{
-	u32 used_math;
-	int err = 0;
-	s32 treg;
-
-	/* Always make any pending restarted system calls return -EINTR */
-	current_thread_info()->restart_block.fn = do_no_restart_syscall;
-
-	err |= __get_user(regs->cp0_epc, &sc->sc_pc);
-	err |= __get_user(regs->hi, &sc->sc_mdhi);
-	err |= __get_user(regs->lo, &sc->sc_mdlo);
-	if (cpu_has_dsp) {
-		err |= __get_user(treg, &sc->sc_hi1); mthi1(treg);
-		err |= __get_user(treg, &sc->sc_lo1); mtlo1(treg);
-		err |= __get_user(treg, &sc->sc_hi2); mthi2(treg);
-		err |= __get_user(treg, &sc->sc_lo2); mtlo2(treg);
-		err |= __get_user(treg, &sc->sc_hi3); mthi3(treg);
-		err |= __get_user(treg, &sc->sc_lo3); mtlo3(treg);
-		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
-	}
-
-#define restore_gp_reg(i) do {						\
-	err |= __get_user(regs->regs[i], &sc->sc_regs[i]);		\
-} while(0)
-	restore_gp_reg( 1); restore_gp_reg( 2); restore_gp_reg( 3);
-	restore_gp_reg( 4); restore_gp_reg( 5); restore_gp_reg( 6);
-	restore_gp_reg( 7); restore_gp_reg( 8); restore_gp_reg( 9);
-	restore_gp_reg(10); restore_gp_reg(11); restore_gp_reg(12);
-	restore_gp_reg(13); restore_gp_reg(14); restore_gp_reg(15);
-	restore_gp_reg(16); restore_gp_reg(17); restore_gp_reg(18);
-	restore_gp_reg(19); restore_gp_reg(20); restore_gp_reg(21);
-	restore_gp_reg(22); restore_gp_reg(23); restore_gp_reg(24);
-	restore_gp_reg(25); restore_gp_reg(26); restore_gp_reg(27);
-	restore_gp_reg(28); restore_gp_reg(29); restore_gp_reg(30);
-	restore_gp_reg(31);
-#undef restore_gp_reg
-
-	err |= __get_user(used_math, &sc->sc_used_math);
-	conditional_used_math(used_math);
-
-	preempt_disable();
-
-	if (used_math()) {
-		/* restore fpu context if we have used it before */
-		own_fpu();
-		err |= restore_fp_context32(sc);
-	} else {
-		/* signal handler may have used FPU.  Give it up. */
-		lose_fpu();
-	}
-
-	preempt_enable();
-
-	return err;
-}
-
 int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
 {
 	int err;
@@ -547,63 +587,6 @@ badframe:
 	force_sig(SIGSEGV, current);
 }
 
-static inline int setup_sigcontext32(struct pt_regs *regs,
-				     struct sigcontext32 __user *sc)
-{
-	int err = 0;
-
-	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
-	err |= __put_user(regs->cp0_status, &sc->sc_status);
-
-#define save_gp_reg(i) {						\
-	err |= __put_user(regs->regs[i], &sc->sc_regs[i]);		\
-} while(0)
-	__put_user(0, &sc->sc_regs[0]); save_gp_reg(1); save_gp_reg(2);
-	save_gp_reg(3); save_gp_reg(4); save_gp_reg(5); save_gp_reg(6);
-	save_gp_reg(7); save_gp_reg(8); save_gp_reg(9); save_gp_reg(10);
-	save_gp_reg(11); save_gp_reg(12); save_gp_reg(13); save_gp_reg(14);
-	save_gp_reg(15); save_gp_reg(16); save_gp_reg(17); save_gp_reg(18);
-	save_gp_reg(19); save_gp_reg(20); save_gp_reg(21); save_gp_reg(22);
-	save_gp_reg(23); save_gp_reg(24); save_gp_reg(25); save_gp_reg(26);
-	save_gp_reg(27); save_gp_reg(28); save_gp_reg(29); save_gp_reg(30);
-	save_gp_reg(31);
-#undef save_gp_reg
-
-	err |= __put_user(regs->hi, &sc->sc_mdhi);
-	err |= __put_user(regs->lo, &sc->sc_mdlo);
-	if (cpu_has_dsp) {
-		err |= __put_user(rddsp(DSP_MASK), &sc->sc_dsp);
-		err |= __put_user(mfhi1(), &sc->sc_hi1);
-		err |= __put_user(mflo1(), &sc->sc_lo1);
-		err |= __put_user(mfhi2(), &sc->sc_hi2);
-		err |= __put_user(mflo2(), &sc->sc_lo2);
-		err |= __put_user(mfhi3(), &sc->sc_hi3);
-		err |= __put_user(mflo3(), &sc->sc_lo3);
-	}
-
-	err |= __put_user(!!used_math(), &sc->sc_used_math);
-
-	if (!used_math())
-		goto out;
-
-	/*
-	 * Save FPU state to signal context.  Signal handler will "inherit"
-	 * current FPU state.
-	 */
-	preempt_disable();
-
-	if (!is_fpu_owner()) {
-		own_fpu();
-		restore_fp(current);
-	}
-	err |= save_fp_context32(sc);
-
-	preempt_enable();
-
-out:
-	return err;
-}
-
 int setup_frame_32(struct k_sigaction * ka, struct pt_regs *regs,
 	int signr, sigset_t *set)
 {
-- 
1.4.4.3.ge6d4

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

* [PATCH 8/10] signal32: no need to save c0_status register in setup_sigcontext32()
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (6 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 7/10] signal32: reduce {setup,restore}_sigcontext32 sizes Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-05 14:24 ` [PATCH 9/10] signal: do not use save_static_function() anymore Franck Bui-Huu
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

All the information in the MIPS c0_status register is priviledged.
Nothing that would constitute part of the thread context.

The one flag one could possibly argument about might be c0_status.fr
but none of the ABIs or tools or application software can make use
of it.

So for consistency with restore_sigcontext32(), which does not
restore c0_status register, this patch remove the saving part.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal32.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index 5d102ef..0994d6e 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -170,7 +170,6 @@ static int setup_sigcontext32(struct pt_regs *regs,
 	int i;
 
 	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
-	err |= __put_user(regs->cp0_status, &sc->sc_status);
 
 	err |= __put_user(0, &sc->sc_regs[0]);
 	for (i = 1; i < 32; i++)
-- 
1.4.4.3.ge6d4

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

* [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (7 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 8/10] signal32: no need to save c0_status register in setup_sigcontext32() Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-07 15:40   ` Atsushi Nemoto
  2007-02-05 14:24 ` [PATCH 10/10] signal: do not inline handle_signal() Franck Bui-Huu
  2007-02-07 16:04 ` [PATCH 0/10] Clean up signal code [take #3] Atsushi Nemoto
  10 siblings, 1 reply; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

This macro was used to save static registers before calling
sys_sigsuspend() and sys_sigreturn().

For the sys_sigreturn() case, there's no point to save them
since they have been already saved by setup_sigcontext()
before calling the signal handler.

For the sys_sigsuspend() case, I don't see any reasons...

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal.c     |   16 ++++------------
 arch/mips/kernel/signal32.c   |   16 ++++------------
 arch/mips/kernel/signal_n32.c |    8 ++------
 3 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index d3f6b0a..32d4022 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -201,9 +201,7 @@ int install_sigtramp(unsigned int __user *tramp, unsigned int syscall)
  */
 
 #ifdef CONFIG_TRAD_SIGNALS
-save_static_function(sys_sigsuspend);
-__attribute_used__ noinline static int
-_sys_sigsuspend(nabi_no_regargs struct pt_regs regs)
+asmlinkage int sys_sigsuspend(nabi_no_regargs struct pt_regs regs)
 {
 	sigset_t newset;
 	sigset_t __user *uset;
@@ -226,9 +224,7 @@ _sys_sigsuspend(nabi_no_regargs struct pt_regs regs)
 }
 #endif
 
-save_static_function(sys_rt_sigsuspend);
-__attribute_used__ noinline static int
-_sys_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
+asmlinkage int sys_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 {
 	sigset_t newset;
 	sigset_t __user *unewset;
@@ -307,9 +303,7 @@ asmlinkage int sys_sigaltstack(nabi_no_regargs struct pt_regs regs)
 }
 
 #ifdef CONFIG_TRAD_SIGNALS
-save_static_function(sys_sigreturn);
-__attribute_used__ noinline static void
-_sys_sigreturn(nabi_no_regargs struct pt_regs regs)
+asmlinkage void sys_sigreturn(nabi_no_regargs struct pt_regs regs)
 {
 	struct sigframe __user *frame;
 	sigset_t blocked;
@@ -344,9 +338,7 @@ badframe:
 }
 #endif /* CONFIG_TRAD_SIGNALS */
 
-save_static_function(sys_rt_sigreturn);
-__attribute_used__ noinline static void
-_sys_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
+asmlinkage void sys_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 {
 	struct rt_sigframe __user *frame;
 	sigset_t set;
diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index 0994d6e..183fc7e 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -308,9 +308,7 @@ static inline int get_sigset(sigset_t *kbuf, const compat_sigset_t __user *ubuf)
  * Atomically swap in the new signal mask, and wait for a signal.
  */
 
-save_static_function(sys32_sigsuspend);
-__attribute_used__ noinline static int
-_sys32_sigsuspend(nabi_no_regargs struct pt_regs regs)
+asmlinkage int sys32_sigsuspend(nabi_no_regargs struct pt_regs regs)
 {
 	compat_sigset_t __user *uset;
 	sigset_t newset;
@@ -332,9 +330,7 @@ _sys32_sigsuspend(nabi_no_regargs struct pt_regs regs)
 	return -ERESTARTNOHAND;
 }
 
-save_static_function(sys32_rt_sigsuspend);
-__attribute_used__ noinline static int
-_sys32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
+asmlinkage int sys32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 {
 	compat_sigset_t __user *uset;
 	sigset_t newset;
@@ -495,9 +491,7 @@ int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
 	return err;
 }
 
-save_static_function(sys32_sigreturn);
-__attribute_used__ noinline static void
-_sys32_sigreturn(nabi_no_regargs struct pt_regs regs)
+asmlinkage void sys32_sigreturn(nabi_no_regargs struct pt_regs regs)
 {
 	struct sigframe __user *frame;
 	sigset_t blocked;
@@ -531,9 +525,7 @@ badframe:
 	force_sig(SIGSEGV, current);
 }
 
-save_static_function(sys32_rt_sigreturn);
-__attribute_used__ noinline static void
-_sys32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
+asmlinkage void sys32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 {
 	struct rt_sigframe32 __user *frame;
 	mm_segment_t old_fs;
diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c
index a6c1e67..35bfbc7 100644
--- a/arch/mips/kernel/signal_n32.c
+++ b/arch/mips/kernel/signal_n32.c
@@ -87,9 +87,7 @@ struct rt_sigframe_n32 {
 
 extern void sigset_from_compat (sigset_t *set, compat_sigset_t *compat);
 
-save_static_function(sysn32_rt_sigsuspend);
-__attribute_used__ noinline static int
-_sysn32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
+asmlinkage int sysn32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 {
 	compat_sigset_t __user *unewset;
 	compat_sigset_t uset;
@@ -119,9 +117,7 @@ _sysn32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 	return -ERESTARTNOHAND;
 }
 
-save_static_function(sysn32_rt_sigreturn);
-__attribute_used__ noinline static void
-_sysn32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
+asmlinkage void sysn32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 {
 	struct rt_sigframe_n32 __user *frame;
 	sigset_t set;
-- 
1.4.4.3.ge6d4

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

* [PATCH 10/10] signal: do not inline handle_signal()
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (8 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 9/10] signal: do not use save_static_function() anymore Franck Bui-Huu
@ 2007-02-05 14:24 ` Franck Bui-Huu
  2007-02-07 16:04 ` [PATCH 0/10] Clean up signal code [take #3] Atsushi Nemoto
  10 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-05 14:24 UTC (permalink / raw)
  To: ralf; +Cc: linux-mips, Franck Bui-Huu

From: Franck Bui-Huu <fbuihuu@gmail.com>

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/signal.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 32d4022..229276a 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -484,7 +484,7 @@ give_sigsegv:
 	return -EFAULT;
 }
 
-static inline int handle_signal(unsigned long sig, siginfo_t *info,
+static int handle_signal(unsigned long sig, siginfo_t *info,
 	struct k_sigaction *ka, sigset_t *oldset, struct pt_regs *regs)
 {
 	int ret;
-- 
1.4.4.3.ge6d4

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

* Re: [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-05 14:24 ` [PATCH 9/10] signal: do not use save_static_function() anymore Franck Bui-Huu
@ 2007-02-07 15:40   ` Atsushi Nemoto
  2007-02-08  8:53     ` Franck Bui-Huu
  0 siblings, 1 reply; 21+ messages in thread
From: Atsushi Nemoto @ 2007-02-07 15:40 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ralf, linux-mips, fbuihuu

On Mon,  5 Feb 2007 15:24:27 +0100, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> For the sys_sigsuspend() case, I don't see any reasons...

Maybe that was needed before commit
7b3e2fc847c8325a7b35185fa1fc2f1729ed9c5b.  At that time
sys_sigsuspend() called do_signal() (which includes
setup_sigcontext()) directly.  So all registers must be saved to
kernel stack.

Now do_signal() is called only from do_notify_resume(), so I agree
save_static_function is not needed.
---
Atsushi Nemoto

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

* Re: [PATCH 0/10] Clean up signal code [take #3]
  2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
                   ` (9 preceding siblings ...)
  2007-02-05 14:24 ` [PATCH 10/10] signal: do not inline handle_signal() Franck Bui-Huu
@ 2007-02-07 16:04 ` Atsushi Nemoto
  2007-02-08  8:55   ` Franck Bui-Huu
  10 siblings, 1 reply; 21+ messages in thread
From: Atsushi Nemoto @ 2007-02-07 16:04 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ralf, linux-mips

On Mon,  5 Feb 2007 15:24:18 +0100, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> Here is the third version of this patchset.
> I ran (on a 32-bits kernel _only_) all LTP signal testcases and
> they all passed. I haven't time to look at what these tests exactly
> do though. 

Thank you for good job.  All looks good for me.
# Though [PATCH 6/10] failed due to recent whitespace cleanup ...

I hope this patchset applied before updating my fp_context patches.
---
Atsushi Nemoto

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

* Re: [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-07 15:40   ` Atsushi Nemoto
@ 2007-02-08  8:53     ` Franck Bui-Huu
  2007-02-08 13:36       ` Atsushi Nemoto
  0 siblings, 1 reply; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-08  8:53 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: ralf, linux-mips

Hi Atsushi,

On 2/7/07, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> On Mon,  5 Feb 2007 15:24:27 +0100, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> > For the sys_sigsuspend() case, I don't see any reasons...
>
> Maybe that was needed before commit
> 7b3e2fc847c8325a7b35185fa1fc2f1729ed9c5b.  At that time
> sys_sigsuspend() called do_signal() (which includes
> setup_sigcontext()) directly.  So all registers must be saved to
> kernel stack.
>

Well, I haven't look at the old code you pointed out. So I dunno, but
I still really think we currently don't need to save/restore static
registers at all...

I tried the following patch:

diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 229276a..046fb1b 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -68,7 +68,9 @@ int setup_sigcontext(struct pt_regs *regs, struct
sigcontext __user *sc)
 	err |= __put_user(regs->cp0_epc, &sc->sc_pc);

 	err |= __put_user(0, &sc->sc_regs[0]);
-	for (i = 1; i < 32; i++)
+	for (i = 1; i < 16; i++)
+		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
+	for (i = 24; i < 32; i++)
 		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);

 	err |= __put_user(regs->hi, &sc->sc_mdhi);
@@ -126,7 +128,9 @@ int restore_sigcontext(struct pt_regs *regs,
struct sigcontext __user *sc)
 		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
 	}

-	for (i = 1; i < 32; i++)
+	for (i = 1; i < 16; i++)
+		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
+	for (i = 24; i < 32; i++)
 		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);

 	err |= __get_user(used_math, &sc->sc_used_math);

...and it still passes LTP tests.

Someone reported that not saving/restoring static registers may break
user tools but the gain is important I think. If you interested,
please take a look to the following thread:

http://marc.theaimsgroup.com/?l=linux-mips&m=117032669701164&w=2

Thanks
-- 
               Franck

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

* Re: [PATCH 0/10] Clean up signal code [take #3]
  2007-02-07 16:04 ` [PATCH 0/10] Clean up signal code [take #3] Atsushi Nemoto
@ 2007-02-08  8:55   ` Franck Bui-Huu
  2007-02-08  9:16     ` Franck Bui-Huu
  0 siblings, 1 reply; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-08  8:55 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: ralf, linux-mips

Hi Atsushi

Thanks for you review !

On 2/7/07, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> # Though [PATCH 6/10] failed due to recent whitespace cleanup ...
>

I can rebase it if needed,

> I hope this patchset applied before updating my fp_context patches.

yeah I hope too...

-- 
               Franck

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

* Re: [PATCH 0/10] Clean up signal code [take #3]
  2007-02-08  8:55   ` Franck Bui-Huu
@ 2007-02-08  9:16     ` Franck Bui-Huu
  2007-02-08 13:15       ` Atsushi Nemoto
  0 siblings, 1 reply; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-08  9:16 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: ralf, linux-mips

On 2/8/07, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> Hi Atsushi
>
> Thanks for you review !
>
> On 2/7/07, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> > # Though [PATCH 6/10] failed due to recent whitespace cleanup ...
> >
>
> I can rebase it if needed,
>
> > I hope this patchset applied before updating my fp_context patches.
>
> yeah I hope too...
>

BTW, did you have time to try it on a 64-bits kernel ?

Thanks
-- 
               Franck

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

* Re: [PATCH 0/10] Clean up signal code [take #3]
  2007-02-08  9:16     ` Franck Bui-Huu
@ 2007-02-08 13:15       ` Atsushi Nemoto
  0 siblings, 0 replies; 21+ messages in thread
From: Atsushi Nemoto @ 2007-02-08 13:15 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ralf, linux-mips

On Thu, 8 Feb 2007 10:16:59 +0100, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> BTW, did you have time to try it on a 64-bits kernel ?

Yes.  No problem for now although not heavily tested.

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

* Re: [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-08  8:53     ` Franck Bui-Huu
@ 2007-02-08 13:36       ` Atsushi Nemoto
  2007-02-08 15:39         ` Franck Bui-Huu
  0 siblings, 1 reply; 21+ messages in thread
From: Atsushi Nemoto @ 2007-02-08 13:36 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ralf, linux-mips

On Thu, 8 Feb 2007 09:53:18 +0100, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> I tried the following patch:
> 
> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
> index 229276a..046fb1b 100644
> --- a/arch/mips/kernel/signal.c
> +++ b/arch/mips/kernel/signal.c
> @@ -68,7 +68,9 @@ int setup_sigcontext(struct pt_regs *regs, struct
> sigcontext __user *sc)
>  	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
> 
>  	err |= __put_user(0, &sc->sc_regs[0]);
> -	for (i = 1; i < 32; i++)
> +	for (i = 1; i < 16; i++)
> +		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
> +	for (i = 24; i < 32; i++)
>  		err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
> 
>  	err |= __put_user(regs->hi, &sc->sc_mdhi);
> @@ -126,7 +128,9 @@ int restore_sigcontext(struct pt_regs *regs,
> struct sigcontext __user *sc)
>  		err |= __get_user(treg, &sc->sc_dsp); wrdsp(treg, DSP_MASK);
>  	}
> 
> -	for (i = 1; i < 32; i++)
> +	for (i = 1; i < 16; i++)
> +		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
> +	for (i = 24; i < 32; i++)
>  		err |= __get_user(regs->regs[i], &sc->sc_regs[i]);
> 
>  	err |= __get_user(used_math, &sc->sc_used_math);
> 
> ...and it still passes LTP tests.
> 
> Someone reported that not saving/restoring static registers may break
> user tools but the gain is important I think.

NO!  This change might silently corrupt static registers!

If you did not restore static registers in kernel stack on
restore_sigcontext(), succeeding RESTORE_STATIC in restore_all will
load garbages to static registers.

Note that any hardware interrupts in middle of signal handler
overwrite pt_regs area in kernel stack.

I can still remember random static register corruption bug and how
hard to debug ...

---
Atsushi Nemoto

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

* Re: [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-08 13:36       ` Atsushi Nemoto
@ 2007-02-08 15:39         ` Franck Bui-Huu
  2007-02-08 16:35           ` Atsushi Nemoto
  0 siblings, 1 reply; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-08 15:39 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: ralf, linux-mips

Atsushi Nemoto wrote:
> If you did not restore static registers in kernel stack on
> restore_sigcontext(), succeeding RESTORE_STATIC in restore_all will
> load garbages to static registers.
>

You're right the patch I sent is not sufficient. However, we actually
could restore save_static_function (well if we do it, I think it's
much better to do it in assembly code...) for sys_sigreturn() _only_.
In that case RESTORE_STATIC should load correct values, shouldn't it ?

But the points are:

	- get rid of saving static registers in setup_sigcontext()
	- get rid of restoring static registers in restore_sigcontext()
	- free space in the signal frame

what do you think ?
-- 
               Franck

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

* Re: [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-08 15:39         ` Franck Bui-Huu
@ 2007-02-08 16:35           ` Atsushi Nemoto
  2007-02-08 20:05             ` Franck Bui-Huu
  0 siblings, 1 reply; 21+ messages in thread
From: Atsushi Nemoto @ 2007-02-08 16:35 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ralf, linux-mips

On Thu, 8 Feb 2007 16:39:42 +0100, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> You're right the patch I sent is not sufficient. However, we actually
> could restore save_static_function (well if we do it, I think it's
> much better to do it in assembly code...) for sys_sigreturn() _only_.
> In that case RESTORE_STATIC should load correct values, shouldn't it ?

Yes.  I think you are right.

> But the points are:
> 
> 	- get rid of saving static registers in setup_sigcontext()
> 	- get rid of restoring static registers in restore_sigcontext()
> 	- free space in the signal frame

I'm afraid of ABI compatibility.  Someone might try to handle SIGSEGV
and dump all registers to debug the program without debugger...

---
Atsushi Nemoto

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

* Re: [PATCH 9/10] signal: do not use save_static_function() anymore
  2007-02-08 16:35           ` Atsushi Nemoto
@ 2007-02-08 20:05             ` Franck Bui-Huu
  0 siblings, 0 replies; 21+ messages in thread
From: Franck Bui-Huu @ 2007-02-08 20:05 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: ralf, linux-mips

On 2/8/07, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> On Thu, 8 Feb 2007 16:39:42 +0100, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> > But the points are:
> >
> >       - get rid of saving static registers in setup_sigcontext()
> >       - get rid of restoring static registers in restore_sigcontext()
> >       - free space in the signal frame
>
> I'm afraid of ABI compatibility.  Someone might try to handle SIGSEGV
> and dump all registers to debug the program without debugger...
>

Yes that's the main issue with this change. We could make it
configurable with an option which would depend on CONFIG_EMBEDDED or
something. Therefore someone can turn on the optimization if he really
wants it on his platform. But we would still lose the extra space gain
in the signal frame.

Note: I think that such programs can have trouble with current code
anyway... What would happen if the sig handler is run when returning
from a syscall ? In this case wouldn't sig context contain almost
garbage ?

-- 
               Franck

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

end of thread, other threads:[~2007-02-08 20:06 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-05 14:24 [PATCH 0/10] Clean up signal code [take #3] Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 1/10] signals: reduce {setup,restore}_sigcontext sizes Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 2/10] signal: do not inline functions in signal-common.h Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 3/10] signal: clean up sigframe structure Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 4/10] signal32: remove code duplication Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 5/10] signal: test return value of install_sigtramp() Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 6/10] signal: factorize debug code Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 7/10] signal32: reduce {setup,restore}_sigcontext32 sizes Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 8/10] signal32: no need to save c0_status register in setup_sigcontext32() Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 9/10] signal: do not use save_static_function() anymore Franck Bui-Huu
2007-02-07 15:40   ` Atsushi Nemoto
2007-02-08  8:53     ` Franck Bui-Huu
2007-02-08 13:36       ` Atsushi Nemoto
2007-02-08 15:39         ` Franck Bui-Huu
2007-02-08 16:35           ` Atsushi Nemoto
2007-02-08 20:05             ` Franck Bui-Huu
2007-02-05 14:24 ` [PATCH 10/10] signal: do not inline handle_signal() Franck Bui-Huu
2007-02-07 16:04 ` [PATCH 0/10] Clean up signal code [take #3] Atsushi Nemoto
2007-02-08  8:55   ` Franck Bui-Huu
2007-02-08  9:16     ` Franck Bui-Huu
2007-02-08 13:15       ` Atsushi Nemoto

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.