* [PATCH 0/2] Minor signal clean up
@ 2015-08-20 11:45 ` Markos Chandras
0 siblings, 0 replies; 12+ messages in thread
From: Markos Chandras @ 2015-08-20 11:45 UTC (permalink / raw)
To: linux-mips
Hi,
The following two patches contain a minor clean up in the signal setup code.
The first one makes o32 behave the same on 64-bit kernels as with the 32-bit
ones. The second one drops the extra arguments from traditional signal
handlers.
Markos Chandras (2):
MIPS: asm: signal.h: Fix traditional signal handling for o32 on 64-bit
kernels
MIPS: kernel: signal: Drop unused arguments for traditional signal
handlers
arch/mips/include/asm/signal.h | 8 +++-----
arch/mips/kernel/signal.c | 8 ++------
arch/mips/kernel/signal32.c | 6 +-----
arch/mips/kernel/signal_n32.c | 2 +-
4 files changed, 7 insertions(+), 17 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 0/2] Minor signal clean up @ 2015-08-20 11:45 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-20 11:45 UTC (permalink / raw) To: linux-mips Hi, The following two patches contain a minor clean up in the signal setup code. The first one makes o32 behave the same on 64-bit kernels as with the 32-bit ones. The second one drops the extra arguments from traditional signal handlers. Markos Chandras (2): MIPS: asm: signal.h: Fix traditional signal handling for o32 on 64-bit kernels MIPS: kernel: signal: Drop unused arguments for traditional signal handlers arch/mips/include/asm/signal.h | 8 +++----- arch/mips/kernel/signal.c | 8 ++------ arch/mips/kernel/signal32.c | 6 +----- arch/mips/kernel/signal_n32.c | 2 +- 4 files changed, 7 insertions(+), 17 deletions(-) -- 2.5.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] MIPS: asm: signal.h: Fix traditional signal handling for o32 on 64-bit kernels @ 2015-08-20 11:45 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-20 11:45 UTC (permalink / raw) To: linux-mips CONFIG_TRAD_SIGNALS is used to denote that legacy signal handlers are supported (ie !SA_SIGINFO). However, this symbol is only available on 32-bit kernels, so o32 running on 64-bit kernels with !SA_SIGINFO was treated as if SA_SIGINFO was set so there was extra overhead setting up the signal handling code within the kernel. o32 should work in the same way for both 32-bit and 64-bit kernels so we fix the sig_uses_siginfo definition to allow traditional signal handling for o32 even on 64-bit kernels. This has been tested booting a MIPS32R6 userland on a 64-bit MIPS64R6 kernel. Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> --- arch/mips/include/asm/signal.h | 8 +++----- arch/mips/kernel/signal.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/arch/mips/include/asm/signal.h b/arch/mips/include/asm/signal.h index 003e273eff4c..cedc53b0ab69 100644 --- a/arch/mips/include/asm/signal.h +++ b/arch/mips/include/asm/signal.h @@ -12,11 +12,9 @@ #include <uapi/asm/signal.h> -#ifdef CONFIG_TRAD_SIGNALS -#define sig_uses_siginfo(ka) ((ka)->sa.sa_flags & SA_SIGINFO) -#else -#define sig_uses_siginfo(ka) (1) -#endif +#define sig_uses_siginfo(abi, ka) \ + (((ka)->sa.sa_flags & SA_SIGINFO) || \ + (!config_enabled(CONFIG_TRAD_SIGNALS) && !(abi)->setup_frame)) #include <asm/sigcontext.h> #include <asm/siginfo.h> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c index 359fb5829f66..be3ac5f7cbbb 100644 --- a/arch/mips/kernel/signal.c +++ b/arch/mips/kernel/signal.c @@ -801,7 +801,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) regs->regs[0] = 0; /* Don't deal with this again. */ } - if (sig_uses_siginfo(&ksig->ka)) + if (sig_uses_siginfo(abi, &ksig->ka)) ret = abi->setup_rt_frame(vdso + abi->rt_signal_return_offset, ksig, regs, oldset); else -- 2.5.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/2] MIPS: asm: signal.h: Fix traditional signal handling for o32 on 64-bit kernels @ 2015-08-20 11:45 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-20 11:45 UTC (permalink / raw) To: linux-mips CONFIG_TRAD_SIGNALS is used to denote that legacy signal handlers are supported (ie !SA_SIGINFO). However, this symbol is only available on 32-bit kernels, so o32 running on 64-bit kernels with !SA_SIGINFO was treated as if SA_SIGINFO was set so there was extra overhead setting up the signal handling code within the kernel. o32 should work in the same way for both 32-bit and 64-bit kernels so we fix the sig_uses_siginfo definition to allow traditional signal handling for o32 even on 64-bit kernels. This has been tested booting a MIPS32R6 userland on a 64-bit MIPS64R6 kernel. Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> --- arch/mips/include/asm/signal.h | 8 +++----- arch/mips/kernel/signal.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/arch/mips/include/asm/signal.h b/arch/mips/include/asm/signal.h index 003e273eff4c..cedc53b0ab69 100644 --- a/arch/mips/include/asm/signal.h +++ b/arch/mips/include/asm/signal.h @@ -12,11 +12,9 @@ #include <uapi/asm/signal.h> -#ifdef CONFIG_TRAD_SIGNALS -#define sig_uses_siginfo(ka) ((ka)->sa.sa_flags & SA_SIGINFO) -#else -#define sig_uses_siginfo(ka) (1) -#endif +#define sig_uses_siginfo(abi, ka) \ + (((ka)->sa.sa_flags & SA_SIGINFO) || \ + (!config_enabled(CONFIG_TRAD_SIGNALS) && !(abi)->setup_frame)) #include <asm/sigcontext.h> #include <asm/siginfo.h> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c index 359fb5829f66..be3ac5f7cbbb 100644 --- a/arch/mips/kernel/signal.c +++ b/arch/mips/kernel/signal.c @@ -801,7 +801,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) regs->regs[0] = 0; /* Don't deal with this again. */ } - if (sig_uses_siginfo(&ksig->ka)) + if (sig_uses_siginfo(abi, &ksig->ka)) ret = abi->setup_rt_frame(vdso + abi->rt_signal_return_offset, ksig, regs, oldset); else -- 2.5.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers @ 2015-08-20 11:45 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-20 11:45 UTC (permalink / raw) To: linux-mips Traditional signal handlers (ie !SA_SIGINFO) only need only argument holding the signal number so we drop the additional arguments and fix the related comments. We also update the comments for the SA_SIGINFO case where the second argument is a pointer to a siginfo_t structure. Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> --- arch/mips/kernel/signal.c | 6 +----- arch/mips/kernel/signal32.c | 6 +----- arch/mips/kernel/signal_n32.c | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c index be3ac5f7cbbb..3a125331bf8b 100644 --- a/arch/mips/kernel/signal.c +++ b/arch/mips/kernel/signal.c @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) - * a2 = pointer to struct sigcontext * * $25 and c0_epc point to the signal handler, $29 points to the * struct sigframe. */ regs->regs[ 4] = ksig->sig; - regs->regs[ 5] = 0; - regs->regs[ 6] = (unsigned long) &frame->sf_sc; regs->regs[29] = (unsigned long) frame; regs->regs[31] = (unsigned long) sig_return; regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler; @@ -730,7 +726,7 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) + * a1 = pointer to siginfo_t * a2 = pointer to ucontext * * $25 and c0_epc point to the signal handler, $29 points to diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c index 3059f36bfc89..6f6f79435edf 100644 --- a/arch/mips/kernel/signal32.c +++ b/arch/mips/kernel/signal32.c @@ -336,15 +336,11 @@ static int setup_frame_32(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) - * a2 = pointer to struct sigcontext * * $25 and c0_epc point to the signal handler, $29 points to the * struct sigframe. */ regs->regs[ 4] = ksig->sig; - regs->regs[ 5] = 0; - regs->regs[ 6] = (unsigned long) &frame->sf_sc; regs->regs[29] = (unsigned long) frame; regs->regs[31] = (unsigned long) sig_return; regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler; @@ -383,7 +379,7 @@ static int setup_rt_frame_32(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) + * a1 = pointer to siginfo_t * a2 = pointer to ucontext * * $25 and c0_epc point to the signal handler, $29 points to diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c index 0d017fdcaf07..04c4a2a7df13 100644 --- a/arch/mips/kernel/signal_n32.c +++ b/arch/mips/kernel/signal_n32.c @@ -129,7 +129,7 @@ static int setup_rt_frame_n32(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) + * a1 = pointer to siginfo_t * a2 = pointer to ucontext * * $25 and c0_epc point to the signal handler, $29 points to -- 2.5.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers @ 2015-08-20 11:45 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-20 11:45 UTC (permalink / raw) To: linux-mips Traditional signal handlers (ie !SA_SIGINFO) only need only argument holding the signal number so we drop the additional arguments and fix the related comments. We also update the comments for the SA_SIGINFO case where the second argument is a pointer to a siginfo_t structure. Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> --- arch/mips/kernel/signal.c | 6 +----- arch/mips/kernel/signal32.c | 6 +----- arch/mips/kernel/signal_n32.c | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c index be3ac5f7cbbb..3a125331bf8b 100644 --- a/arch/mips/kernel/signal.c +++ b/arch/mips/kernel/signal.c @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) - * a2 = pointer to struct sigcontext * * $25 and c0_epc point to the signal handler, $29 points to the * struct sigframe. */ regs->regs[ 4] = ksig->sig; - regs->regs[ 5] = 0; - regs->regs[ 6] = (unsigned long) &frame->sf_sc; regs->regs[29] = (unsigned long) frame; regs->regs[31] = (unsigned long) sig_return; regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler; @@ -730,7 +726,7 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) + * a1 = pointer to siginfo_t * a2 = pointer to ucontext * * $25 and c0_epc point to the signal handler, $29 points to diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c index 3059f36bfc89..6f6f79435edf 100644 --- a/arch/mips/kernel/signal32.c +++ b/arch/mips/kernel/signal32.c @@ -336,15 +336,11 @@ static int setup_frame_32(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) - * a2 = pointer to struct sigcontext * * $25 and c0_epc point to the signal handler, $29 points to the * struct sigframe. */ regs->regs[ 4] = ksig->sig; - regs->regs[ 5] = 0; - regs->regs[ 6] = (unsigned long) &frame->sf_sc; regs->regs[29] = (unsigned long) frame; regs->regs[31] = (unsigned long) sig_return; regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler; @@ -383,7 +379,7 @@ static int setup_rt_frame_32(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) + * a1 = pointer to siginfo_t * a2 = pointer to ucontext * * $25 and c0_epc point to the signal handler, $29 points to diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c index 0d017fdcaf07..04c4a2a7df13 100644 --- a/arch/mips/kernel/signal_n32.c +++ b/arch/mips/kernel/signal_n32.c @@ -129,7 +129,7 @@ static int setup_rt_frame_n32(void *sig_return, struct ksignal *ksig, * Arguments to signal handler: * * a0 = signal number - * a1 = 0 (should be cause) + * a1 = pointer to siginfo_t * a2 = pointer to ucontext * * $25 and c0_epc point to the signal handler, $29 points to -- 2.5.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers 2015-08-20 11:45 ` Markos Chandras (?) @ 2015-08-21 17:03 ` David Daney 2015-08-24 8:01 ` Markos Chandras -1 siblings, 1 reply; 12+ messages in thread From: David Daney @ 2015-08-21 17:03 UTC (permalink / raw) To: Markos Chandras; +Cc: linux-mips On 08/20/2015 04:45 AM, Markos Chandras wrote: > Traditional signal handlers (ie !SA_SIGINFO) only need only argument > holding the signal number so we drop the additional arguments and fix > the related comments. We also update the comments for the SA_SIGINFO > case where the second argument is a pointer to a siginfo_t structure. > > Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> > --- > arch/mips/kernel/signal.c | 6 +----- > arch/mips/kernel/signal32.c | 6 +----- > arch/mips/kernel/signal_n32.c | 2 +- > 3 files changed, 3 insertions(+), 11 deletions(-) > > diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c > index be3ac5f7cbbb..3a125331bf8b 100644 > --- a/arch/mips/kernel/signal.c > +++ b/arch/mips/kernel/signal.c > @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct ksignal *ksig, > * Arguments to signal handler: > * > * a0 = signal number > - * a1 = 0 (should be cause) > - * a2 = pointer to struct sigcontext > * > * $25 and c0_epc point to the signal handler, $29 points to the > * struct sigframe. > */ > regs->regs[ 4] = ksig->sig; > - regs->regs[ 5] = 0; > - regs->regs[ 6] = (unsigned long) &frame->sf_sc; This changes the kernel ABI. Have you tested this change against all userspace applications that use signals to make sure it doesn't break anything? David Daney ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers @ 2015-08-24 8:01 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-24 8:01 UTC (permalink / raw) To: David Daney; +Cc: linux-mips On 08/21/2015 06:03 PM, David Daney wrote: > On 08/20/2015 04:45 AM, Markos Chandras wrote: >> Traditional signal handlers (ie !SA_SIGINFO) only need only argument >> holding the signal number so we drop the additional arguments and fix >> the related comments. We also update the comments for the SA_SIGINFO >> case where the second argument is a pointer to a siginfo_t structure. >> >> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> >> --- >> arch/mips/kernel/signal.c | 6 +----- >> arch/mips/kernel/signal32.c | 6 +----- >> arch/mips/kernel/signal_n32.c | 2 +- >> 3 files changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c >> index be3ac5f7cbbb..3a125331bf8b 100644 >> --- a/arch/mips/kernel/signal.c >> +++ b/arch/mips/kernel/signal.c >> @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct >> ksignal *ksig, >> * Arguments to signal handler: >> * >> * a0 = signal number >> - * a1 = 0 (should be cause) >> - * a2 = pointer to struct sigcontext >> * >> * $25 and c0_epc point to the signal handler, $29 points to the >> * struct sigframe. >> */ >> regs->regs[ 4] = ksig->sig; >> - regs->regs[ 5] = 0; >> - regs->regs[ 6] = (unsigned long) &frame->sf_sc; > > This changes the kernel ABI. > > Have you tested this change against all userspace applications that use > signals to make sure it doesn't break anything? > > David Daney i am confident there is no userland application that uses inline asm to fetch additional arguments from (*sa_handler) when using !SA_SIGINFO -- markos ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers @ 2015-08-24 8:01 ` Markos Chandras 0 siblings, 0 replies; 12+ messages in thread From: Markos Chandras @ 2015-08-24 8:01 UTC (permalink / raw) To: David Daney; +Cc: linux-mips On 08/21/2015 06:03 PM, David Daney wrote: > On 08/20/2015 04:45 AM, Markos Chandras wrote: >> Traditional signal handlers (ie !SA_SIGINFO) only need only argument >> holding the signal number so we drop the additional arguments and fix >> the related comments. We also update the comments for the SA_SIGINFO >> case where the second argument is a pointer to a siginfo_t structure. >> >> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> >> --- >> arch/mips/kernel/signal.c | 6 +----- >> arch/mips/kernel/signal32.c | 6 +----- >> arch/mips/kernel/signal_n32.c | 2 +- >> 3 files changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c >> index be3ac5f7cbbb..3a125331bf8b 100644 >> --- a/arch/mips/kernel/signal.c >> +++ b/arch/mips/kernel/signal.c >> @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct >> ksignal *ksig, >> * Arguments to signal handler: >> * >> * a0 = signal number >> - * a1 = 0 (should be cause) >> - * a2 = pointer to struct sigcontext >> * >> * $25 and c0_epc point to the signal handler, $29 points to the >> * struct sigframe. >> */ >> regs->regs[ 4] = ksig->sig; >> - regs->regs[ 5] = 0; >> - regs->regs[ 6] = (unsigned long) &frame->sf_sc; > > This changes the kernel ABI. > > Have you tested this change against all userspace applications that use > signals to make sure it doesn't break anything? > > David Daney i am confident there is no userland application that uses inline asm to fetch additional arguments from (*sa_handler) when using !SA_SIGINFO -- markos ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers @ 2015-08-25 20:38 ` Paul Burton 0 siblings, 0 replies; 12+ messages in thread From: Paul Burton @ 2015-08-25 20:38 UTC (permalink / raw) To: Markos Chandras; +Cc: David Daney, linux-mips On Mon, Aug 24, 2015 at 09:01:09AM +0100, Markos Chandras wrote: > On 08/21/2015 06:03 PM, David Daney wrote: > > On 08/20/2015 04:45 AM, Markos Chandras wrote: > >> Traditional signal handlers (ie !SA_SIGINFO) only need only argument > >> holding the signal number so we drop the additional arguments and fix > >> the related comments. We also update the comments for the SA_SIGINFO > >> case where the second argument is a pointer to a siginfo_t structure. > >> > >> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> > >> --- > >> arch/mips/kernel/signal.c | 6 +----- > >> arch/mips/kernel/signal32.c | 6 +----- > >> arch/mips/kernel/signal_n32.c | 2 +- > >> 3 files changed, 3 insertions(+), 11 deletions(-) > >> > >> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c > >> index be3ac5f7cbbb..3a125331bf8b 100644 > >> --- a/arch/mips/kernel/signal.c > >> +++ b/arch/mips/kernel/signal.c > >> @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct > >> ksignal *ksig, > >> * Arguments to signal handler: > >> * > >> * a0 = signal number > >> - * a1 = 0 (should be cause) > >> - * a2 = pointer to struct sigcontext > >> * > >> * $25 and c0_epc point to the signal handler, $29 points to the > >> * struct sigframe. > >> */ > >> regs->regs[ 4] = ksig->sig; > >> - regs->regs[ 5] = 0; > >> - regs->regs[ 6] = (unsigned long) &frame->sf_sc; > > > > This changes the kernel ABI. > > > > Have you tested this change against all userspace applications that use > > signals to make sure it doesn't break anything? > > > > David Daney > > i am confident there is no userland application that uses inline asm to > fetch additional arguments from (*sa_handler) when using !SA_SIGINFO > > -- > markos I'm not sure where you get the idea that you'd need inline asm to use the a1/a2 values - you'd just need to declare the extra parameters to your signal handling function. This does seem like a scary change! I'm pretty confident you haven't actually checked every bit of userland code. Would we perhaps be better off leaving the registers set (which is a trivial cost) and document the behaviour instead? Thanks, Paul ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers @ 2015-08-25 20:38 ` Paul Burton 0 siblings, 0 replies; 12+ messages in thread From: Paul Burton @ 2015-08-25 20:38 UTC (permalink / raw) To: Markos Chandras; +Cc: David Daney, linux-mips On Mon, Aug 24, 2015 at 09:01:09AM +0100, Markos Chandras wrote: > On 08/21/2015 06:03 PM, David Daney wrote: > > On 08/20/2015 04:45 AM, Markos Chandras wrote: > >> Traditional signal handlers (ie !SA_SIGINFO) only need only argument > >> holding the signal number so we drop the additional arguments and fix > >> the related comments. We also update the comments for the SA_SIGINFO > >> case where the second argument is a pointer to a siginfo_t structure. > >> > >> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> > >> --- > >> arch/mips/kernel/signal.c | 6 +----- > >> arch/mips/kernel/signal32.c | 6 +----- > >> arch/mips/kernel/signal_n32.c | 2 +- > >> 3 files changed, 3 insertions(+), 11 deletions(-) > >> > >> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c > >> index be3ac5f7cbbb..3a125331bf8b 100644 > >> --- a/arch/mips/kernel/signal.c > >> +++ b/arch/mips/kernel/signal.c > >> @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct > >> ksignal *ksig, > >> * Arguments to signal handler: > >> * > >> * a0 = signal number > >> - * a1 = 0 (should be cause) > >> - * a2 = pointer to struct sigcontext > >> * > >> * $25 and c0_epc point to the signal handler, $29 points to the > >> * struct sigframe. > >> */ > >> regs->regs[ 4] = ksig->sig; > >> - regs->regs[ 5] = 0; > >> - regs->regs[ 6] = (unsigned long) &frame->sf_sc; > > > > This changes the kernel ABI. > > > > Have you tested this change against all userspace applications that use > > signals to make sure it doesn't break anything? > > > > David Daney > > i am confident there is no userland application that uses inline asm to > fetch additional arguments from (*sa_handler) when using !SA_SIGINFO > > -- > markos I'm not sure where you get the idea that you'd need inline asm to use the a1/a2 values - you'd just need to declare the extra parameters to your signal handling function. This does seem like a scary change! I'm pretty confident you haven't actually checked every bit of userland code. Would we perhaps be better off leaving the registers set (which is a trivial cost) and document the behaviour instead? Thanks, Paul ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers 2015-08-24 8:01 ` Markos Chandras (?) (?) @ 2015-08-25 21:02 ` David Daney -1 siblings, 0 replies; 12+ messages in thread From: David Daney @ 2015-08-25 21:02 UTC (permalink / raw) To: Markos Chandras; +Cc: linux-mips On 08/24/2015 01:01 AM, Markos Chandras wrote: > On 08/21/2015 06:03 PM, David Daney wrote: >> On 08/20/2015 04:45 AM, Markos Chandras wrote: >>> Traditional signal handlers (ie !SA_SIGINFO) only need only argument >>> holding the signal number so we drop the additional arguments and fix >>> the related comments. We also update the comments for the SA_SIGINFO >>> case where the second argument is a pointer to a siginfo_t structure. >>> >>> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com> >>> --- >>> arch/mips/kernel/signal.c | 6 +----- >>> arch/mips/kernel/signal32.c | 6 +----- >>> arch/mips/kernel/signal_n32.c | 2 +- >>> 3 files changed, 3 insertions(+), 11 deletions(-) >>> >>> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c >>> index be3ac5f7cbbb..3a125331bf8b 100644 >>> --- a/arch/mips/kernel/signal.c >>> +++ b/arch/mips/kernel/signal.c >>> @@ -683,15 +683,11 @@ static int setup_frame(void *sig_return, struct >>> ksignal *ksig, >>> * Arguments to signal handler: >>> * >>> * a0 = signal number >>> - * a1 = 0 (should be cause) >>> - * a2 = pointer to struct sigcontext >>> * >>> * $25 and c0_epc point to the signal handler, $29 points to the >>> * struct sigframe. >>> */ >>> regs->regs[ 4] = ksig->sig; >>> - regs->regs[ 5] = 0; >>> - regs->regs[ 6] = (unsigned long) &frame->sf_sc; >> >> This changes the kernel ABI. >> >> Have you tested this change against all userspace applications that use >> signals to make sure it doesn't break anything? >> >> David Daney > > i am confident there is no userland application that uses inline asm to > fetch additional arguments from (*sa_handler) when using !SA_SIGINFO You don't need inline asm. All you have to do is use a C cast on the function pointer. I think your confidence is misplaced. I have a program that this change will break. You are changing the de facto kernel ABI in such a manner that it could break existing programs without a strong reason to do so. Answer this: Why is it important to you to change this? The change log gives no justification other than we can, and it doesn't seem to break limited set of test cases. If you are fixing some real bug, then OK. But if you don't have a reason to do this, then: NAK. David Daney > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-08-25 21:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-20 11:45 [PATCH 0/2] Minor signal clean up Markos Chandras 2015-08-20 11:45 ` Markos Chandras 2015-08-20 11:45 ` [PATCH 1/2] MIPS: asm: signal.h: Fix traditional signal handling for o32 on 64-bit kernels Markos Chandras 2015-08-20 11:45 ` Markos Chandras 2015-08-20 11:45 ` [PATCH 2/2] MIPS: kernel: signal: Drop unused arguments for traditional signal handlers Markos Chandras 2015-08-20 11:45 ` Markos Chandras 2015-08-21 17:03 ` David Daney 2015-08-24 8:01 ` Markos Chandras 2015-08-24 8:01 ` Markos Chandras 2015-08-25 20:38 ` Paul Burton 2015-08-25 20:38 ` Paul Burton 2015-08-25 21:02 ` David Daney
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.