linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Commit breaks strace: ARM: entry: allow ARM-private syscalls to be restarted
@ 2013-09-17 21:57 Jason Gunthorpe
  2013-09-18  9:17 ` Will Deacon
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Gunthorpe @ 2013-09-17 21:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Will,

I was just testing v3.12-rc1 (on kirkwood) and noticed that strace is
not working:

$ strace /bin/ls
mmap2(0xb6f79000, 9552, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb6f79000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fad000
set_tls(0xb6fad4c0, 0xb6fadb98, 0xb6fb1050, 0xb6fad4c0, 0xb6fb1050) = -1 ENOSYS (Function not implemented)
writev(2, [{"cannot set up thread-local stora"..., 36}, {"unknown error", 13}, {"\n", 1}], 3cannot set up thread-local storage: unknown error
) = 50
exit_group(127)                         = ?
+++ exited with 127 +++

I determined that reverting your commit below makes strace work again.
I've tested 3.10.12 and it is OK.

This happens with the latest git head for strace, as well as the 4.7
release.

Hopefully you can cook up a fix :)

Regards,
Jason

commit 377747c40657eb35ad98a56439606d96a928425a
Author: Will Deacon <will.deacon@arm.com>
Date:   Mon May 13 19:16:34 2013 +0100

    ARM: entry: allow ARM-private syscalls to be restarted
    
    System calls will only be restarted after signal handling if they (a)
    return an error code indicating that a restart is required and (b) have
    `why' set to a non-zero value, to indicate that the signal interrupted
    them.
    
    This patch leaves `why' set to a non-zero value for ARM-private syscalls
    , and only zeroes it for syscalls that are not implemented.
    
    Signed-off-by: Will Deacon <will.deacon@arm.com>

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 94104bf..74ad15d1 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -442,10 +442,10 @@ local_restart:
        ldrcc   pc, [tbl, scno, lsl #2]         @ call sys_* routine
 
        add     r1, sp, #S_OFF
-2:     mov     why, #0                         @ no longer a real syscall
        cmp     scno, #(__ARM_NR_BASE - __NR_SYSCALL_BASE)
        eor     r0, scno, #__NR_SYSCALL_BASE    @ put OS number back
-       bcs     arm_syscall     
+       bcs     arm_syscall
+2:     mov     why, #0                         @ no longer a real syscall
        b       sys_ni_syscall                  @ not private func
 
 #if defined(CONFIG_OABI_COMPAT) || !defined(CONFIG_AEABI)

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

* Commit breaks strace: ARM: entry: allow ARM-private syscalls to be restarted
  2013-09-17 21:57 Commit breaks strace: ARM: entry: allow ARM-private syscalls to be restarted Jason Gunthorpe
@ 2013-09-18  9:17 ` Will Deacon
  2013-09-18  9:50   ` Will Deacon
  0 siblings, 1 reply; 4+ messages in thread
From: Will Deacon @ 2013-09-18  9:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 17, 2013 at 10:57:49PM +0100, Jason Gunthorpe wrote:
> Hi Will,

Hi Jason,

> I was just testing v3.12-rc1 (on kirkwood) and noticed that strace is
> not working:

Thanks for the report!

> $ strace /bin/ls
> mmap2(0xb6f79000, 9552, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb6f79000
> close(3)                                = 0
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb6fad000
> set_tls(0xb6fad4c0, 0xb6fadb98, 0xb6fb1050, 0xb6fad4c0, 0xb6fb1050) = -1 ENOSYS (Function not implemented)
> writev(2, [{"cannot set up thread-local stora"..., 36}, {"unknown error", 13}, {"\n", 1}], 3cannot set up thread-local storage: unknown error
> ) = 50
> exit_group(127)                         = ?
> +++ exited with 127 +++
> 
> I determined that reverting your commit below makes strace work again.
> I've tested 3.10.12 and it is OK.

Hmm, so if you don't run with strace, does `/bin/ls' work as expected? From
the trace above, it seems that set_tls is failing rather than anything in
strace (unless of course, the trace is just plain wrong).

> This happens with the latest git head for strace, as well as the 4.7
> release.
> 
> Hopefully you can cook up a fix :)

I'll take a look today, thanks.

Will

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

* Commit breaks strace: ARM: entry: allow ARM-private syscalls to be restarted
  2013-09-18  9:17 ` Will Deacon
@ 2013-09-18  9:50   ` Will Deacon
  2013-10-04 17:24     ` Arokux X
  0 siblings, 1 reply; 4+ messages in thread
From: Will Deacon @ 2013-09-18  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 18, 2013 at 10:17:48AM +0100, Will Deacon wrote:
> On Tue, Sep 17, 2013 at 10:57:49PM +0100, Jason Gunthorpe wrote:
> > This happens with the latest git head for strace, as well as the 4.7
> > release.
> > 
> > Hopefully you can cook up a fix :)
> 
> I'll take a look today, thanks.

Ok, looks like the backwards branch to the syscall dispatching logic from
the trace entry is busted. Can you try the patch below please?

I might also take a look at unifying the function prototypes for arm_syscall
and syscall_trace_enter, since it should help us remove some of the argument
shuffling in the trace path.

Will

--->8

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 74ad15d1..bc6bd96 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -442,10 +442,10 @@ local_restart:
        ldrcc   pc, [tbl, scno, lsl #2]         @ call sys_* routine
 
        add     r1, sp, #S_OFF
-       cmp     scno, #(__ARM_NR_BASE - __NR_SYSCALL_BASE)
+2:     cmp     scno, #(__ARM_NR_BASE - __NR_SYSCALL_BASE)
        eor     r0, scno, #__NR_SYSCALL_BASE    @ put OS number back
        bcs     arm_syscall
-2:     mov     why, #0                         @ no longer a real syscall
+       mov     why, #0                         @ no longer a real syscall
        b       sys_ni_syscall                  @ not private func
 
 #if defined(CONFIG_OABI_COMPAT) || !defined(CONFIG_AEABI)

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

* Commit breaks strace: ARM: entry: allow ARM-private syscalls to be restarted
  2013-09-18  9:50   ` Will Deacon
@ 2013-10-04 17:24     ` Arokux X
  0 siblings, 0 replies; 4+ messages in thread
From: Arokux X @ 2013-10-04 17:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 18, 2013 at 11:50 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Sep 18, 2013 at 10:17:48AM +0100, Will Deacon wrote:
>> On Tue, Sep 17, 2013 at 10:57:49PM +0100, Jason Gunthorpe wrote:
>> > This happens with the latest git head for strace, as well as the 4.7
>> > release.
>> >
>> > Hopefully you can cook up a fix :)
>>
>> I'll take a look today, thanks.
>
> Ok, looks like the backwards branch to the syscall dispatching logic from
> the trace entry is busted. Can you try the patch below please?
>
> I might also take a look at unifying the function prototypes for arm_syscall
> and syscall_trace_enter, since it should help us remove some of the argument
> shuffling in the trace path.
>
> Will
>
> --->8
>
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 74ad15d1..bc6bd96 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -442,10 +442,10 @@ local_restart:
>         ldrcc   pc, [tbl, scno, lsl #2]         @ call sys_* routine
>
>         add     r1, sp, #S_OFF
> -       cmp     scno, #(__ARM_NR_BASE - __NR_SYSCALL_BASE)
> +2:     cmp     scno, #(__ARM_NR_BASE - __NR_SYSCALL_BASE)
>         eor     r0, scno, #__NR_SYSCALL_BASE    @ put OS number back
>         bcs     arm_syscall
> -2:     mov     why, #0                         @ no longer a real syscall
> +       mov     why, #0                         @ no longer a real syscall
>         b       sys_ni_syscall                  @ not private func
>
>  #if defined(CONFIG_OABI_COMPAT) || !defined(CONFIG_AEABI)

Just stumbled upon the same issue. The patch seems to cure the problem. Thanks.

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

end of thread, other threads:[~2013-10-04 17:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 21:57 Commit breaks strace: ARM: entry: allow ARM-private syscalls to be restarted Jason Gunthorpe
2013-09-18  9:17 ` Will Deacon
2013-09-18  9:50   ` Will Deacon
2013-10-04 17:24     ` Arokux X

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).