* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-15 6:36 [PATCH v2] m68k: use conventional function parameters for do_sigreturn Greg Ungerer
@ 2016-02-15 10:03 ` Philippe De Muyter
2016-02-15 13:06 ` Greg Ungerer
2016-02-24 1:10 ` Greg Ungerer
2016-02-29 9:36 ` Geert Uytterhoeven
2 siblings, 1 reply; 8+ messages in thread
From: Philippe De Muyter @ 2016-02-15 10:03 UTC (permalink / raw)
To: Greg Ungerer; +Cc: linux-m68k
Hi Greg,
On Mon, Feb 15, 2016 at 04:36:29PM +1000, Greg Ungerer wrote:
> Create conventional stack parameters for the calls to do_sigreturn and
> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
> dig into the stack to create local pointers to the saved switch stack
> and the pt_regs structs.
>
> The motivation for this change is a problem with non-MMU targets that
> have broken signal return paths on newer versions of gcc. It appears as
> though gcc has determined that the pointers into the saved stack structs,
> and the saved structs themselves, are function parameters and updates to
> them will be lost on function return, so they are optimized away. This
> results in large parts of restore_sigcontext() and mangle_kernel_stack()
> functions being removed. Of course this results in non-functional code
> causing kernel oops. This problem has been observed with gcc version
> 5.2 and 5.3, and probably exists in earlier versions as well.
>
> Using conventional stack parameter pointers passed to these functions has
> the advantage of the code here not needing to know the exact details of
> how the underlying entry handler layed these structs out on the stack.
> So the rather ugly pointer setup casting and arg referencing can be
> removed.
>
> The resulting code after this change is a few bytes larger (due to the
> overhead of creating the stack args and their tear down). Not being hot
> paths I don't think this is too much of a problem here.
As an intermediate solution, I think that you can avoid part of that overhead ...
>
> An alternative solution is to put a barrier() in the do_sigreturn() code,
> but this doesn't feel quite as clean as this solution.
>
> This change has been compile tested on all defconfigs, and run tested on
> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
> no-MMU (QEMU and M5208EVB).
>
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> ---
> v2: reworded the commit log message with better description of problem
>
> arch/m68k/kernel/entry.S | 6 ++++++
> arch/m68k/kernel/signal.c | 8 ++------
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
> index b54ac7a..97cd3ea 100644
> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>
> ENTRY(sys_sigreturn)
> SAVE_SWITCH_STACK
> + movel %sp,%sp@- | switch_stack pointer
> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
if you remove the above asm instruction ...
> jbsr do_sigreturn
> + addql #8,%sp
change the 8 by 4 here ...
> RESTORE_SWITCH_STACK
> rts
>
> ENTRY(sys_rt_sigreturn)
> SAVE_SWITCH_STACK
> + movel %sp,%sp@- | switch_stack pointer
> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
ditto here ...
> jbsr do_rt_sigreturn
> + addql #8,%sp
and here ...
> RESTORE_SWITCH_STACK
> rts
>
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index af1c4f3..2dcee3a 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -737,10 +737,8 @@ badframe:
> return 1;
> }
>
> -asmlinkage int do_sigreturn(unsigned long __unused)
> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
and if you use the following prototype :
asmlinkage int do_sigreturn(struct switch_stack *sw) ...
> {
> - struct switch_stack *sw = (struct switch_stack *) &__unused;
> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
and keep the 'C' instruction above
> unsigned long usp = rdusp();
> struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
> sigset_t set;
> @@ -764,10 +762,8 @@ badframe:
> return 0;
> }
>
> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
same here
> {
> - struct switch_stack *sw = (struct switch_stack *) &__unused;
> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
and here
> unsigned long usp = rdusp();
> struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
> sigset_t set;
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-15 10:03 ` Philippe De Muyter
@ 2016-02-15 13:06 ` Greg Ungerer
2016-02-15 14:57 ` Philippe De Muyter
0 siblings, 1 reply; 8+ messages in thread
From: Greg Ungerer @ 2016-02-15 13:06 UTC (permalink / raw)
To: Philippe De Muyter; +Cc: linux-m68k
Hi Philippe,
On 15/02/16 20:03, Philippe De Muyter wrote:
> On Mon, Feb 15, 2016 at 04:36:29PM +1000, Greg Ungerer wrote:
>> Create conventional stack parameters for the calls to do_sigreturn and
>> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
>> dig into the stack to create local pointers to the saved switch stack
>> and the pt_regs structs.
>>
>> The motivation for this change is a problem with non-MMU targets that
>> have broken signal return paths on newer versions of gcc. It appears as
>> though gcc has determined that the pointers into the saved stack structs,
>> and the saved structs themselves, are function parameters and updates to
>> them will be lost on function return, so they are optimized away. This
>> results in large parts of restore_sigcontext() and mangle_kernel_stack()
>> functions being removed. Of course this results in non-functional code
>> causing kernel oops. This problem has been observed with gcc version
>> 5.2 and 5.3, and probably exists in earlier versions as well.
>>
>> Using conventional stack parameter pointers passed to these functions has
>> the advantage of the code here not needing to know the exact details of
>> how the underlying entry handler layed these structs out on the stack.
>> So the rather ugly pointer setup casting and arg referencing can be
>> removed.
>>
>> The resulting code after this change is a few bytes larger (due to the
>> overhead of creating the stack args and their tear down). Not being hot
>> paths I don't think this is too much of a problem here.
>
> As an intermediate solution, I think that you can avoid part of that overhead ...
>
>>
>> An alternative solution is to put a barrier() in the do_sigreturn() code,
>> but this doesn't feel quite as clean as this solution.
>>
>> This change has been compile tested on all defconfigs, and run tested on
>> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
>> no-MMU (QEMU and M5208EVB).
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>> ---
>> v2: reworded the commit log message with better description of problem
>>
>> arch/m68k/kernel/entry.S | 6 ++++++
>> arch/m68k/kernel/signal.c | 8 ++------
>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
>> index b54ac7a..97cd3ea 100644
>> --- a/arch/m68k/kernel/entry.S
>> +++ b/arch/m68k/kernel/entry.S
>> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>>
>> ENTRY(sys_sigreturn)
>> SAVE_SWITCH_STACK
>> + movel %sp,%sp@- | switch_stack pointer
>> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>
> if you remove the above asm instruction ...
>
>> jbsr do_sigreturn
>> + addql #8,%sp
>
> change the 8 by 4 here ...
>
>> RESTORE_SWITCH_STACK
>> rts
>>
>> ENTRY(sys_rt_sigreturn)
>> SAVE_SWITCH_STACK
>> + movel %sp,%sp@- | switch_stack pointer
>> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>
> ditto here ...
>
>> jbsr do_rt_sigreturn
>> + addql #8,%sp
>
> and here ...
>
>> RESTORE_SWITCH_STACK
>> rts
>>
>> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
>> index af1c4f3..2dcee3a 100644
>> --- a/arch/m68k/kernel/signal.c
>> +++ b/arch/m68k/kernel/signal.c
>> @@ -737,10 +737,8 @@ badframe:
>> return 1;
>> }
>>
>> -asmlinkage int do_sigreturn(unsigned long __unused)
>> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
>
> and if you use the following prototype :
> asmlinkage int do_sigreturn(struct switch_stack *sw) ...
>
>> {
>> - struct switch_stack *sw = (struct switch_stack *) &__unused;
>> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>
> and keep the 'C' instruction above
>
>> unsigned long usp = rdusp();
>> struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
>> sigset_t set;
>> @@ -764,10 +762,8 @@ badframe:
>> return 0;
>> }
>>
>> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
>> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
> same here
>> {
>> - struct switch_stack *sw = (struct switch_stack *) &__unused;
>> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
> and here
>> unsigned long usp = rdusp();
>> struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
>> sigset_t set;
Those changes ultimately make no difference to code size.
Although the entry point code is smaller the resulting
generated code for do_sigreturn() and do_rt_sigreturn()
is slightly larger. (At least that is the case with the gcc-5.3
compiler I am currently using).
Personally I don't like having to keep that ugly stack referencing
definition of regs based on sw either. I would like to remove that.
"sw" isn't actually used in do_sigreturn() at all, so that could
be removed from the its parameters. I chose to keep it there in this
patch for consistency with do_rt_sigreturn().
Regards
Greg
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-15 13:06 ` Greg Ungerer
@ 2016-02-15 14:57 ` Philippe De Muyter
0 siblings, 0 replies; 8+ messages in thread
From: Philippe De Muyter @ 2016-02-15 14:57 UTC (permalink / raw)
To: Greg Ungerer; +Cc: linux-m68k
Hi Greg,
On Mon, Feb 15, 2016 at 11:06:15PM +1000, Greg Ungerer wrote:
> Hi Philippe,
>
> On 15/02/16 20:03, Philippe De Muyter wrote:
>> On Mon, Feb 15, 2016 at 04:36:29PM +1000, Greg Ungerer wrote:
>>> Create conventional stack parameters for the calls to do_sigreturn and
>>> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
>>> dig into the stack to create local pointers to the saved switch stack
>>> and the pt_regs structs.
>>>
>>> The motivation for this change is a problem with non-MMU targets that
>>> have broken signal return paths on newer versions of gcc. It appears as
>>> though gcc has determined that the pointers into the saved stack structs,
>>> and the saved structs themselves, are function parameters and updates to
>>> them will be lost on function return, so they are optimized away. This
>>> results in large parts of restore_sigcontext() and mangle_kernel_stack()
>>> functions being removed. Of course this results in non-functional code
>>> causing kernel oops. This problem has been observed with gcc version
>>> 5.2 and 5.3, and probably exists in earlier versions as well.
>>>
>>> Using conventional stack parameter pointers passed to these functions has
>>> the advantage of the code here not needing to know the exact details of
>>> how the underlying entry handler layed these structs out on the stack.
>>> So the rather ugly pointer setup casting and arg referencing can be
>>> removed.
>>>
>>> The resulting code after this change is a few bytes larger (due to the
>>> overhead of creating the stack args and their tear down). Not being hot
>>> paths I don't think this is too much of a problem here.
>>
>> As an intermediate solution, I think that you can avoid part of that
>> overhead ...
>>
>>>
>>> An alternative solution is to put a barrier() in the do_sigreturn() code,
>>> but this doesn't feel quite as clean as this solution.
>>>
>>> This change has been compile tested on all defconfigs, and run tested on
>>> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
>>> no-MMU (QEMU and M5208EVB).
>>>
>>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>>> ---
>>> v2: reworded the commit log message with better description of problem
>>>
>>> arch/m68k/kernel/entry.S | 6 ++++++
>>> arch/m68k/kernel/signal.c | 8 ++------
>>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
>>> index b54ac7a..97cd3ea 100644
>>> --- a/arch/m68k/kernel/entry.S
>>> +++ b/arch/m68k/kernel/entry.S
>>> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>>>
>>> ENTRY(sys_sigreturn)
>>> SAVE_SWITCH_STACK
>>> + movel %sp,%sp@- | switch_stack pointer
>>> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>>
>> if you remove the above asm instruction ...
>>
>>> jbsr do_sigreturn
>>> + addql #8,%sp
>>
>> change the 8 by 4 here ...
>>
>>> RESTORE_SWITCH_STACK
>>> rts
>>>
>>> ENTRY(sys_rt_sigreturn)
>>> SAVE_SWITCH_STACK
>>> + movel %sp,%sp@- | switch_stack pointer
>>> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>>
>> ditto here ...
>>
>>> jbsr do_rt_sigreturn
>>> + addql #8,%sp
>>
>> and here ...
>>
>>> RESTORE_SWITCH_STACK
>>> rts
>>>
>>> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
>>> index af1c4f3..2dcee3a 100644
>>> --- a/arch/m68k/kernel/signal.c
>>> +++ b/arch/m68k/kernel/signal.c
>>> @@ -737,10 +737,8 @@ badframe:
>>> return 1;
>>> }
>>>
>>> -asmlinkage int do_sigreturn(unsigned long __unused)
>>> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack
>>> *sw)
>>
>> and if you use the following prototype :
>> asmlinkage int do_sigreturn(struct switch_stack *sw) ...
>>
>>> {
>>> - struct switch_stack *sw = (struct switch_stack *) &__unused;
>>> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>>
>> and keep the 'C' instruction above
>>
>>> unsigned long usp = rdusp();
>>> struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
>>> sigset_t set;
>>> @@ -764,10 +762,8 @@ badframe:
>>> return 0;
>>> }
>>>
>>> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
>>> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack
>>> *sw)
>> same here
>>> {
>>> - struct switch_stack *sw = (struct switch_stack *) &__unused;
>>> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>> and here
>>> unsigned long usp = rdusp();
>>> struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp -
>>> 4);
>>> sigset_t set;
>
> Those changes ultimately make no difference to code size.
> Although the entry point code is smaller the resulting
> generated code for do_sigreturn() and do_rt_sigreturn()
> is slightly larger. (At least that is the case with the gcc-5.3
> compiler I am currently using).
>
> Personally I don't like having to keep that ugly stack referencing
> definition of regs based on sw either. I would like to remove that.
>
> "sw" isn't actually used in do_sigreturn() at all, so that could
> be removed from the its parameters. I chose to keep it there in this
> patch for consistency with do_rt_sigreturn().
OK. Thanks for the attention and explanation.
>
> Regards
> Greg
Best regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-15 6:36 [PATCH v2] m68k: use conventional function parameters for do_sigreturn Greg Ungerer
2016-02-15 10:03 ` Philippe De Muyter
@ 2016-02-24 1:10 ` Greg Ungerer
2016-02-25 10:42 ` Geert Uytterhoeven
2016-02-29 9:36 ` Geert Uytterhoeven
2 siblings, 1 reply; 8+ messages in thread
From: Greg Ungerer @ 2016-02-24 1:10 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: linux-m68k
Hi Geert,
Do you have any objections or issues with this change?
Andreas and Philippe kindly gave feedback, which I have addressed
(I think to everyone's satisfaction?)
I would very much like to see this applied to resolve the compilation
issue for non-MMU with modern versions of gcc. I don't mind pushing
through my tree. I just want to make sure first that nobody objects
to this change or has any remaining issues with it.
Regards
Greg
On 15/02/16 16:36, Greg Ungerer wrote:
> Create conventional stack parameters for the calls to do_sigreturn and
> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
> dig into the stack to create local pointers to the saved switch stack
> and the pt_regs structs.
>
> The motivation for this change is a problem with non-MMU targets that
> have broken signal return paths on newer versions of gcc. It appears as
> though gcc has determined that the pointers into the saved stack structs,
> and the saved structs themselves, are function parameters and updates to
> them will be lost on function return, so they are optimized away. This
> results in large parts of restore_sigcontext() and mangle_kernel_stack()
> functions being removed. Of course this results in non-functional code
> causing kernel oops. This problem has been observed with gcc version
> 5.2 and 5.3, and probably exists in earlier versions as well.
>
> Using conventional stack parameter pointers passed to these functions has
> the advantage of the code here not needing to know the exact details of
> how the underlying entry handler layed these structs out on the stack.
> So the rather ugly pointer setup casting and arg referencing can be
> removed.
>
> The resulting code after this change is a few bytes larger (due to the
> overhead of creating the stack args and their tear down). Not being hot
> paths I don't think this is too much of a problem here.
>
> An alternative solution is to put a barrier() in the do_sigreturn() code,
> but this doesn't feel quite as clean as this solution.
>
> This change has been compile tested on all defconfigs, and run tested on
> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
> no-MMU (QEMU and M5208EVB).
>
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> ---
> v2: reworded the commit log message with better description of problem
>
> arch/m68k/kernel/entry.S | 6 ++++++
> arch/m68k/kernel/signal.c | 8 ++------
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
> index b54ac7a..97cd3ea 100644
> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>
> ENTRY(sys_sigreturn)
> SAVE_SWITCH_STACK
> + movel %sp,%sp@- | switch_stack pointer
> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
> jbsr do_sigreturn
> + addql #8,%sp
> RESTORE_SWITCH_STACK
> rts
>
> ENTRY(sys_rt_sigreturn)
> SAVE_SWITCH_STACK
> + movel %sp,%sp@- | switch_stack pointer
> + pea %sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
> jbsr do_rt_sigreturn
> + addql #8,%sp
> RESTORE_SWITCH_STACK
> rts
>
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index af1c4f3..2dcee3a 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -737,10 +737,8 @@ badframe:
> return 1;
> }
>
> -asmlinkage int do_sigreturn(unsigned long __unused)
> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
> {
> - struct switch_stack *sw = (struct switch_stack *) &__unused;
> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
> unsigned long usp = rdusp();
> struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
> sigset_t set;
> @@ -764,10 +762,8 @@ badframe:
> return 0;
> }
>
> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
> {
> - struct switch_stack *sw = (struct switch_stack *) &__unused;
> - struct pt_regs *regs = (struct pt_regs *) (sw + 1);
> unsigned long usp = rdusp();
> struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
> sigset_t set;
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-24 1:10 ` Greg Ungerer
@ 2016-02-25 10:42 ` Geert Uytterhoeven
2016-02-25 10:55 ` Andreas Schwab
0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2016-02-25 10:42 UTC (permalink / raw)
To: Greg Ungerer, Andreas Schwab; +Cc: Linux/m68k
Hi Greg,
On Wed, Feb 24, 2016 at 2:10 AM, Greg Ungerer <gerg@uclinux.org> wrote:
> Do you have any objections or issues with this change?
>
> Andreas and Philippe kindly gave feedback, which I have addressed
> (I think to everyone's satisfaction?)
>
> I would very much like to see this applied to resolve the compilation
> issue for non-MMU with modern versions of gcc. I don't mind pushing
> through my tree. I just want to make sure first that nobody objects
> to this change or has any remaining issues with it.
If Andreas is happy with it, I will apply it.
Andreas?
> On 15/02/16 16:36, Greg Ungerer wrote:
>> Create conventional stack parameters for the calls to do_sigreturn and
>> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
>> dig into the stack to create local pointers to the saved switch stack
>> and the pt_regs structs.
>>
>> The motivation for this change is a problem with non-MMU targets that
>> have broken signal return paths on newer versions of gcc. It appears as
>> though gcc has determined that the pointers into the saved stack structs,
>> and the saved structs themselves, are function parameters and updates to
>> them will be lost on function return, so they are optimized away. This
>> results in large parts of restore_sigcontext() and mangle_kernel_stack()
>> functions being removed. Of course this results in non-functional code
>> causing kernel oops. This problem has been observed with gcc version
>> 5.2 and 5.3, and probably exists in earlier versions as well.
>>
>> Using conventional stack parameter pointers passed to these functions has
>> the advantage of the code here not needing to know the exact details of
>> how the underlying entry handler layed these structs out on the stack.
>> So the rather ugly pointer setup casting and arg referencing can be
>> removed.
>>
>> The resulting code after this change is a few bytes larger (due to the
>> overhead of creating the stack args and their tear down). Not being hot
>> paths I don't think this is too much of a problem here.
>>
>> An alternative solution is to put a barrier() in the do_sigreturn() code,
>> but this doesn't feel quite as clean as this solution.
>>
>> This change has been compile tested on all defconfigs, and run tested on
>> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
>> no-MMU (QEMU and M5208EVB).
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-25 10:42 ` Geert Uytterhoeven
@ 2016-02-25 10:55 ` Andreas Schwab
0 siblings, 0 replies; 8+ messages in thread
From: Andreas Schwab @ 2016-02-25 10:55 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Greg Ungerer, Linux/m68k
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> Hi Greg,
>
> On Wed, Feb 24, 2016 at 2:10 AM, Greg Ungerer <gerg@uclinux.org> wrote:
>> Do you have any objections or issues with this change?
>>
>> Andreas and Philippe kindly gave feedback, which I have addressed
>> (I think to everyone's satisfaction?)
>>
>> I would very much like to see this applied to resolve the compilation
>> issue for non-MMU with modern versions of gcc. I don't mind pushing
>> through my tree. I just want to make sure first that nobody objects
>> to this change or has any remaining issues with it.
>
> If Andreas is happy with it, I will apply it.
No objections.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn
2016-02-15 6:36 [PATCH v2] m68k: use conventional function parameters for do_sigreturn Greg Ungerer
2016-02-15 10:03 ` Philippe De Muyter
2016-02-24 1:10 ` Greg Ungerer
@ 2016-02-29 9:36 ` Geert Uytterhoeven
2 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2016-02-29 9:36 UTC (permalink / raw)
To: Greg Ungerer; +Cc: Linux/m68k, Andreas Schwab
On Mon, Feb 15, 2016 at 7:36 AM, Greg Ungerer <gerg@uclinux.org> wrote:
> Create conventional stack parameters for the calls to do_sigreturn and
> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
> dig into the stack to create local pointers to the saved switch stack
> and the pt_regs structs.
>
> The motivation for this change is a problem with non-MMU targets that
> have broken signal return paths on newer versions of gcc. It appears as
> though gcc has determined that the pointers into the saved stack structs,
> and the saved structs themselves, are function parameters and updates to
> them will be lost on function return, so they are optimized away. This
> results in large parts of restore_sigcontext() and mangle_kernel_stack()
> functions being removed. Of course this results in non-functional code
> causing kernel oops. This problem has been observed with gcc version
> 5.2 and 5.3, and probably exists in earlier versions as well.
>
> Using conventional stack parameter pointers passed to these functions has
> the advantage of the code here not needing to know the exact details of
> how the underlying entry handler layed these structs out on the stack.
> So the rather ugly pointer setup casting and arg referencing can be
> removed.
>
> The resulting code after this change is a few bytes larger (due to the
> overhead of creating the stack args and their tear down). Not being hot
> paths I don't think this is too much of a problem here.
>
> An alternative solution is to put a barrier() in the do_sigreturn() code,
> but this doesn't feel quite as clean as this solution.
>
> This change has been compile tested on all defconfigs, and run tested on
> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
> no-MMU (QEMU and M5208EVB).
>
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
Thanks,.applied and queued for v4.6 with Andreas' Ack.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread