zdtm/static/fd failure on aarch64

Submitted by Dmitry Safonov on June 12, 2018, 4:33 p.m.

Details

Message ID CAJwJo6YBwiWxM_yw04A=vgL3qXjgmFLGveZw637cAGC5ty78dA@mail.gmail.com
State New
Series "zdtm/static/fd failure on aarch64"
Headers show

Commit Message

Dmitry Safonov June 12, 2018, 4:33 p.m.
2018-06-12 17:23 GMT+01:00 Adrian Reber <adrian@lisas.de>:
> On Tue, Jun 12, 2018 at 04:13:05PM +0100, Dmitry Safonov wrote:
>> 2018-06-12 7:48 GMT+01:00 Adrian Reber <adrian@lisas.de>:
>> > Output is always like this:
>> >
>> > 02:20:50.173: 11691: ERR: ../lib/lock.h:149: futex *0xffff86100000 = 0, c = 2249195520 (errno = 11 (Resource temporarily unavailable))
>> >
>> > 11691 mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0) = 0xffff86100000
>> > 11691 futex(0xffff86100000, FUTEX_WAIT, 2249195521, NULL) = -1 EAGAIN (Resource temporarily unavailable)
>> > 11691 brk(NULL)                         = 0x149f0000
>> > 11691 brk(0x14a20000)                   = 0x14a20000
>> > 11691 brk(NULL)                         = 0x14a20000
>> > 11691 openat(AT_FDCWD, "/etc/localtime", O_RDONLY|O_CLOEXEC) = 4
>> > 11691 fstat(4, {st_mode=S_IFREG|0644, st_size=3519, ...}) = 0
>> > 11691 fstat(4, {st_mode=S_IFREG|0644, st_size=3519, ...}) = 0
>> > 11691 mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xffff860f0000
>> > 11691 read(4, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\4\0\0\0\4\0\0\0\0"..., 8192) = 3519
>> > 11691 lseek(4, -2252, SEEK_CUR)         = 1267
>> > 11691 read(4, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\5\0\0\0\5\0\0\0\0"..., 8192) = 2252
>> > 11691 close(4)                          = 0
>> > 11691 munmap(0xffff860f0000, 65536)     = 0
>> > 11691 write(2, "02:20:50.173: 11691: ERR: ../lib"..., 135) = 135
>> > 11691 futex(0xffff86100000, FUTEX_WAIT, 2249195521, NULL) = -1 EAGAIN (Resource temporarily unavailable)
>> > 11691 newfstatat(AT_FDCWD, "/etc/localtime", {st_mode=S_IFREG|0644, st_size=3519, ...}, 0) = 0
>> > 11691 write(2, "02:20:50.175: 11691: ERR: ../lib"..., 135) = 135
>> >
>> > How I understand it is that atomic_inc() should return the new value but the returned value
>> > is completely different from what atomic_get() returns, right?
>>
>> Well, atomic_inc() should return the old value, AFAICS from code.
>> atomic_inc_return() should return the new value.
>>
>> I think it's not due cast we've some garbage value, it looks as if
>> we're doing 16-bit operations on 32-bit values.. But we aren't as far
>> as I can catch from arm64 asm.
>>
>> Could you try with this to see, if atomic ops work?
>
> 11:47:58.974: 15442: ERR: ../lib/lock.h:152: futex *0xffffb1d50000 = 0, c = 2983526400, +0 = 2983526400, -0 = 2983526400 (errno = 11 (Resource temporarily unavailable))
>
> Looks wrong. But looking at the lower bits of 0xffffb1d50000 I see that
> 0xb1d50000 is the same as 2983526400. So instead of the value we get the
> lower bits of the address.
>
>> Probably also worth to try to zero-init `tmp'/`result' inside ops to check
>> if the result is garbage from the stack in ops function.
>
> I already tried that, but it did not change anything.

Probably, could you try with this?
(haven't compile-tested on arm64)

Patch hide | download patch | download mbox

diff --git a/test/zdtm/lib/arch/aarch64/include/asm/atomic.h b/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
index ccf08e700478..ddf4ad9f60e8 100644
--- a/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
+++ b/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
@@ -33,7 +33,7 @@  static inline int atomic_add_return(int i, atomic_t *v)
 "	add	%w0, %w0, %w3\n"
 "	stlxr	%w1, %w0, %2\n"
 "	cbnz	%w1, 1b"
-	: "=&r" (result), "=&r" (tmp), "+Q" (v)
+	: "=&r" (result), "=&r" (tmp), "+Q" (*v)
 	: "Ir" (i)
 	: "cc", "memory");
 
@@ -51,7 +51,7 @@  static inline int atomic_sub_return(int i, atomic_t *v)
 "	sub	%w0, %w0, %w3\n"
 "	stlxr	%w1, %w0, %2\n"
 "	cbnz	%w1, 1b"
-	: "=&r" (result), "=&r" (tmp), "+Q" (v)
+	: "=&r" (result), "=&r" (tmp), "+Q" (*v)
 	: "Ir" (i)
 	: "cc", "memory");
 

Comments

Adrian Reber June 12, 2018, 4:46 p.m.
On Tue, Jun 12, 2018 at 05:33:56PM +0100, Dmitry Safonov wrote:
> 2018-06-12 17:23 GMT+01:00 Adrian Reber <adrian@lisas.de>:
> > On Tue, Jun 12, 2018 at 04:13:05PM +0100, Dmitry Safonov wrote:
> >> 2018-06-12 7:48 GMT+01:00 Adrian Reber <adrian@lisas.de>:
> >> > Output is always like this:
> >> >
> >> > 02:20:50.173: 11691: ERR: ../lib/lock.h:149: futex *0xffff86100000 = 0, c = 2249195520 (errno = 11 (Resource temporarily unavailable))
> >> >
> >> > 11691 mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0) = 0xffff86100000
> >> > 11691 futex(0xffff86100000, FUTEX_WAIT, 2249195521, NULL) = -1 EAGAIN (Resource temporarily unavailable)
> >> > 11691 brk(NULL)                         = 0x149f0000
> >> > 11691 brk(0x14a20000)                   = 0x14a20000
> >> > 11691 brk(NULL)                         = 0x14a20000
> >> > 11691 openat(AT_FDCWD, "/etc/localtime", O_RDONLY|O_CLOEXEC) = 4
> >> > 11691 fstat(4, {st_mode=S_IFREG|0644, st_size=3519, ...}) = 0
> >> > 11691 fstat(4, {st_mode=S_IFREG|0644, st_size=3519, ...}) = 0
> >> > 11691 mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xffff860f0000
> >> > 11691 read(4, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\4\0\0\0\4\0\0\0\0"..., 8192) = 3519
> >> > 11691 lseek(4, -2252, SEEK_CUR)         = 1267
> >> > 11691 read(4, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\5\0\0\0\5\0\0\0\0"..., 8192) = 2252
> >> > 11691 close(4)                          = 0
> >> > 11691 munmap(0xffff860f0000, 65536)     = 0
> >> > 11691 write(2, "02:20:50.173: 11691: ERR: ../lib"..., 135) = 135
> >> > 11691 futex(0xffff86100000, FUTEX_WAIT, 2249195521, NULL) = -1 EAGAIN (Resource temporarily unavailable)
> >> > 11691 newfstatat(AT_FDCWD, "/etc/localtime", {st_mode=S_IFREG|0644, st_size=3519, ...}, 0) = 0
> >> > 11691 write(2, "02:20:50.175: 11691: ERR: ../lib"..., 135) = 135
> >> >
> >> > How I understand it is that atomic_inc() should return the new value but the returned value
> >> > is completely different from what atomic_get() returns, right?
> >>
> >> Well, atomic_inc() should return the old value, AFAICS from code.
> >> atomic_inc_return() should return the new value.
> >>
> >> I think it's not due cast we've some garbage value, it looks as if
> >> we're doing 16-bit operations on 32-bit values.. But we aren't as far
> >> as I can catch from arm64 asm.
> >>
> >> Could you try with this to see, if atomic ops work?
> >
> > 11:47:58.974: 15442: ERR: ../lib/lock.h:152: futex *0xffffb1d50000 = 0, c = 2983526400, +0 = 2983526400, -0 = 2983526400 (errno = 11 (Resource temporarily unavailable))
> >
> > Looks wrong. But looking at the lower bits of 0xffffb1d50000 I see that
> > 0xb1d50000 is the same as 2983526400. So instead of the value we get the
> > lower bits of the address.
> >
> >> Probably also worth to try to zero-init `tmp'/`result' inside ops to check
> >> if the result is garbage from the stack in ops function.
> >
> > I already tried that, but it did not change anything.
> 
> Probably, could you try with this?
> (haven't compile-tested on arm64)

I thought I tried exactly that, because that is the required fix. This
also fixes zdtm/transition/maps007. All your other changes are not
applied right now.

Now only cow01 is failing. Thanks for remote debugging.

> diff --git a/test/zdtm/lib/arch/aarch64/include/asm/atomic.h b/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
> index ccf08e700478..ddf4ad9f60e8 100644
> --- a/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
> +++ b/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
> @@ -33,7 +33,7 @@ static inline int atomic_add_return(int i, atomic_t *v)
>  "	add	%w0, %w0, %w3\n"
>  "	stlxr	%w1, %w0, %2\n"
>  "	cbnz	%w1, 1b"
> -	: "=&r" (result), "=&r" (tmp), "+Q" (v)
> +	: "=&r" (result), "=&r" (tmp), "+Q" (*v)
>  	: "Ir" (i)
>  	: "cc", "memory");
>  
> @@ -51,7 +51,7 @@ static inline int atomic_sub_return(int i, atomic_t *v)
>  "	sub	%w0, %w0, %w3\n"
>  "	stlxr	%w1, %w0, %2\n"
>  "	cbnz	%w1, 1b"
> -	: "=&r" (result), "=&r" (tmp), "+Q" (v)
> +	: "=&r" (result), "=&r" (tmp), "+Q" (*v)
>  	: "Ir" (i)
>  	: "cc", "memory");
>  


		Adrian
Dmitry Safonov June 12, 2018, 5 p.m.
2018-06-12 17:46 GMT+01:00 Adrian Reber <adrian@lisas.de>:
>
> I thought I tried exactly that, because that is the required fix. This
> also fixes zdtm/transition/maps007. All your other changes are not
> applied right now.
>
> Now only cow01 is failing. Thanks for remote debugging.

Sure, thanks for testing!
Do you want me to send the patch, or will you do it?
Also, I think it's still worth to correct -EWOULDBLOCK => EWOULDBLOCK
and definition of sys_futex(). Like the patch from mail-id:
<CAJwJo6ZGatGFWaYmnk8TyBqBGVPdqVLS1MO3xX4i_x1o4ZjpWw@mail.gmail.com>

>> diff --git a/test/zdtm/lib/arch/aarch64/include/asm/atomic.h b/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
>> index ccf08e700478..ddf4ad9f60e8 100644
>> --- a/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
>> +++ b/test/zdtm/lib/arch/aarch64/include/asm/atomic.h
>> @@ -33,7 +33,7 @@ static inline int atomic_add_return(int i, atomic_t *v)
>>  "    add     %w0, %w0, %w3\n"
>>  "    stlxr   %w1, %w0, %2\n"
>>  "    cbnz    %w1, 1b"
>> -     : "=&r" (result), "=&r" (tmp), "+Q" (v)
>> +     : "=&r" (result), "=&r" (tmp), "+Q" (*v)
>>       : "Ir" (i)
>>       : "cc", "memory");
>>
>> @@ -51,7 +51,7 @@ static inline int atomic_sub_return(int i, atomic_t *v)
>>  "    sub     %w0, %w0, %w3\n"
>>  "    stlxr   %w1, %w0, %2\n"
>>  "    cbnz    %w1, 1b"
>> -     : "=&r" (result), "=&r" (tmp), "+Q" (v)
>> +     : "=&r" (result), "=&r" (tmp), "+Q" (*v)
>>       : "Ir" (i)
>>       : "cc", "memory");
>>
Adrian Reber June 12, 2018, 7:10 p.m.
On Tue, Jun 12, 2018 at 06:00:20PM +0100, Dmitry Safonov wrote:
> 2018-06-12 17:46 GMT+01:00 Adrian Reber <adrian@lisas.de>:
> >
> > I thought I tried exactly that, because that is the required fix. This
> > also fixes zdtm/transition/maps007. All your other changes are not
> > applied right now.
> >
> > Now only cow01 is failing. Thanks for remote debugging.
> 
> Sure, thanks for testing!
> Do you want me to send the patch, or will you do it?
> Also, I think it's still worth to correct -EWOULDBLOCK => EWOULDBLOCK
> and definition of sys_futex(). Like the patch from mail-id:
> <CAJwJo6ZGatGFWaYmnk8TyBqBGVPdqVLS1MO3xX4i_x1o4ZjpWw@mail.gmail.com>

I will send that patch and the fix for the inline assembler with both
our signed-off's tomorrow after some more testing.

		Adrian
Dmitry Safonov June 12, 2018, 7:18 p.m.
2018-06-12 20:10 GMT+01:00 Adrian Reber <adrian@lisas.de>:
> On Tue, Jun 12, 2018 at 06:00:20PM +0100, Dmitry Safonov wrote:
>> 2018-06-12 17:46 GMT+01:00 Adrian Reber <adrian@lisas.de>:
>> >
>> > I thought I tried exactly that, because that is the required fix. This
>> > also fixes zdtm/transition/maps007. All your other changes are not
>> > applied right now.
>> >
>> > Now only cow01 is failing. Thanks for remote debugging.
>>
>> Sure, thanks for testing!
>> Do you want me to send the patch, or will you do it?
>> Also, I think it's still worth to correct -EWOULDBLOCK => EWOULDBLOCK
>> and definition of sys_futex(). Like the patch from mail-id:
>> <CAJwJo6ZGatGFWaYmnk8TyBqBGVPdqVLS1MO3xX4i_x1o4ZjpWw@mail.gmail.com>
>
> I will send that patch and the fix for the inline assembler with both
> our signed-off's tomorrow after some more testing.

You are grand! :-)

Thanks,
             Dmitry