Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1360516 > unrolled thread

Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-18 11:40 +0100
    Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Alexander Graf <agraf@suse.de> - 2016-03-18 17:00 +0100
      Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-20 09:20 +0100
        Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-21 12:30 +0100
          Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Yury Norov <ynorov@caviumnetworks.com> - 2016-03-21 20:20 +0100
    Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Andreas Schwab <schwab@suse.de> - 2016-03-21 10:10 +0100
      Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-21 10:50 +0100
        Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Andreas Schwab <schwab@suse.de> - 2016-03-21 12:00 +0100
          Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-21 18:10 +0100
        Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-26 13:40 +0100
          Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-29 13:00 +0200
            Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-29 14:50 +0200
            Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-29 15:30 +0200
              Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-29 15:30 +0200
                Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Joseph Myers <joseph@codesourcery.com> - 2016-03-29 18:00 +0200
                  Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-29 21:40 +0200
                    Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Joseph Myers <joseph@codesourcery.com> - 2016-03-29 22:20 +0200
                      Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-29 22:30 +0200
                        Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Joseph Myers <joseph@codesourcery.com> - 2016-03-29 23:10 +0200
                          Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 Arnd Bergmann <arnd@arndb.de> - 2016-03-29 23:50 +0200
                Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-31 09:40 +0200
      Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-26 14:20 +0100
      Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64 "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-03-26 14:50 +0100

Page 1 of 2  [1] 2  Next page →

#1360516 — Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Yury

We are trying to test ilp32 in our arm64 board. But we got more
failure compare with you. So, I am wondering if we could align
the test environment with you. The source code we used:

1.  glibc: the new-api branch of glibc from
     git@code.huawei.com:gnu/norov_glibc.git.

2.  Kernel: rfc5 from https://github.com/norov/linux.git. Is the
     rc6 in your branch is the latest one? From the commit message, I
     do not find any difference. Is it just a rebase?

3.  Toolchain 4.9.3, could you provide the binary of toolchain(both
     cross and native, gcc, gdb...)? It may be very useful for me or
     other guys who interested build their own filesystem from
     buildroot or something.

4.  LTP: master.

For the glibc part, I found that there are 11 patches of ilp32 in top,
but the original 28 patches of ilp32 is not in the top, there are more
than 900 patches between them(referece the list below). Are you
willing rebase all the ilp32 relative patches. It is very useful for
reviewing and debugging.  I saw andrew request the account in glibc,
maybe it has already been in processs?).

Regards

Bamvor

ILP32 relative patch in norov's glibc:
       1 b5c4968 fix stat
       2 8332b7a fix3
       3 d16a202 fix statfs
       4 351b872 fix IPC_64 for msgctl semctl shmctl
       5 07f8ead fix readdir ????
       6 4be481a fix lseek
       7 4231518 fix getdents
       8 de55857 fstat fix
       9 5d42904 stat
      10 3f70665 mmap, stat syscalls
      11 574af33 Make ilp32 use the compat (old 32bit ABI).

     972 8addb06 [AARCH64] Make lp64 and ilp32 directories.
     973 bfe0619 [AARCH64] Add typesizes.h for ILP32
     974 916581b [AARCH64] Fix up ucontext for ILP32
     975 0f646ee [AARCH64] Add sigstack.h header for ILP32 reasons.
     976 68e11d8 [AARCH64] Add kernel_sigaction.h for AARCH64 ILP32
     977 a0087ba [AARCH64] Add ldd-rewrite.sed so that ilp32 ld.so can be found
     978 0f800d9 [AARCH64] Add ILP32 ld.so to the known interpreter names.
     979 45d74fd [AARCH64] Add support to ldconfig for ILP32 and libilp32
     980 9926da9 [AARCH64] Add ILP32 to makefiles
     981 987b522 [AARCH64] Set up wordsize for ILP32.
     982 5e34e4c [AARCH64] Add ILP32 support to elf_machine_load_address.
     983 1ff4dd9 [AARCH64] Reformat inline-asm in elf_machine_load_address.
     984 f7ff9aa [AARCH64] Syscalls for ILP32 are passed always via 64bit values.
     985 9316667 [AARCH64] Detect ILP32 in configure scripts.
     986 ff0dca1 [AARCH64] Use PTR_REG in getcontext.S.
     987 7c9f1b0 [AARCH64] Use PTR_* in start.S
     988 ed747ff [AARCH64] Use PTR_* macros in dl-trampoline.S
     989 71f6986 [AARCH64] Use PTR_REG/PTR_SIZE/PTR_SIZE_LOG in dl-tlsesc.S
     990 3294b6f [AARCH64] Use PTR_REG in crti.S.
     991 c8fef18 [AARCH64] Add PTR_REG, PTR_LOG_SIZE, and PTR_SIZE.  Use it in LDST_PCREL and LDST_GLOBAL.
     992 585197d Add dynamic ILP32 AARCH64 relocations to elf.h
     993 49f1345 [AARCH64] Add header guards to sysdep.h headers.
     994 2594068 Allow generic stat and statfs not have padding for 32bit targets
     995 86f0aa7 Allow some fields of siginfo to be different from the generic one
     996 fdffbc5 Allow fd_mask type not be an array of long.
     997 453987a Allow rusage work on a big-endian 32bit-on-64bit target
     998 70e6485 Add ability for the IPC structures (msqid_ds, semid_ds, shmid_ds, etc.) to have time_t being 64bit
     999 9ff1730 Allow sigset be an array of a different type
    1000 dc09040 [AARCH64] Fix utmp struct for compatibility reasons.


On 2016/2/26 4:28, Yury Norov wrote:
> On Thu, Feb 25, 2016 at 11:50:31AM +0100, Andreas Schwab wrote:
>> Yury Norov <ynorov@caviumnetworks.com> writes:
>>
>>> I have new glibc that follows new ABI:
>>> https://github.com/norov/glibc/tree/new-api
>>
>> sysdeps/unix/sysv/linux/aarch64/ilp32/getdents64.c is wrong, struct
>> dirent64 is not the same as struct dirent.  The file needs to be renamed
>> to sysdeps/unix/sysv/linux/aarch64/ilp32/getdents.c so that __getdents64
>> comes from sysdeps/unix/sysv/linux/generic/getdents64.c.
>>
>> sysdeps/unix/sysv/linux/aarch64/ilp32/*xstat*.c should not set errno,
>> INLINE_SYSCALL already does that, and returns -1 on error.
>>
>> Andreas.
>>
>
> Thank you. I'll fix it
>
>> --
>> Andreas Schwab, SUSE Labs, schwab@suse.de
>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
>> "And now for something completely different."

[toc] | [next] | [standalone]

#1360755

On 18.03.16 16:49, Yury Norov wrote:
> On Fri, Mar 18, 2016 at 06:28:29PM +0800, Zhangjian (Bamvor) wrote:
>>
>> For the glibc part, I found that there are 11 patches of ilp32 in top,
>> but the original 28 patches of ilp32 is not in the top, there are more
>> than 900 patches between them(referece the list below). Are you
>> willing rebase all the ilp32 relative patches. It is very useful for
>> reviewing and debugging.  I saw andrew request the account in glibc,
>> maybe it has already been in processs?).
>>
> 
> I already told there's mess there, and I'd prefer to make things work
> first and then do cleanup. 

So how is progress going overall? The last submission I've seen is
already 2 months ago. Are there particular bits holding you up?


Alex

[toc] | [prev] | [next] | [standalone]

#1361229

Hi, Yury

On 2016/3/19 0:46, Yury Norov wrote:
> On Fri, Mar 18, 2016 at 04:55:26PM +0100, Alexander Graf wrote:
>>
>>
>> On 18.03.16 16:49, Yury Norov wrote:
>>> On Fri, Mar 18, 2016 at 06:28:29PM +0800, Zhangjian (Bamvor) wrote:
>>>>
>>>> For the glibc part, I found that there are 11 patches of ilp32 in top,
>>>> but the original 28 patches of ilp32 is not in the top, there are more
>>>> than 900 patches between them(referece the list below). Are you
>>>> willing rebase all the ilp32 relative patches. It is very useful for
>>>> reviewing and debugging.  I saw andrew request the account in glibc,
>>>> maybe it has already been in processs?).
>>>>
>>>
>>> I already told there's mess there, and I'd prefer to make things work
>>> first and then do cleanup.
>>
>> So how is progress going overall? The last submission I've seen is
>> already 2 months ago. Are there particular bits holding you up?
>>
>>
>> Alex
>
> Hi Alexander,
>
> For last time I mostly work on library, as it needs to be reworked
> well. But yes, there's one serious bug puzzling me.
>
> Tests like umount or pathconf fail but I see no major problem with
> it, as it's most probably structure padding mismatch between kernel and
> glibc. But there's (at least) one major problem I see.
>
> Float tests fail due to NULL-dereferencing (0x14 actually) at
> pthread_join(). It calls tgkill(), and after that child thread crashes.
> See stack trace at the end.
>
> The minimal test reproducing it is attached. The similar test where
> parent forks a child and then kills it, works fine. (Attached too).
>
> I see that in case of pthread, there's much more stuff that is cloned.
> Other's looking similar.
>
> pthread_create():
> clone(child_stack=0xb953cea0, flags=CLONE_VM|CLONE_FS|CLONE_FILES
>          |CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
>          |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
>          parent_tidptr=0xb953d398, tls=0xb953d7c0, child_tidptr=0xb953d398) = 1650
>
> fork():
> clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
>          child_tidptr=0xe5af6278) = 30537
>
> So this most probably means that ilp32 code doesn't handle one of cloned
> item properly. I have already discovered a bug where child processes
> used parent TLS,
It is a kernel bug or glibc bug? Could you please explain it or show the patch?
The current ILP32 patches looks good to me. Recently, I backport these patches
to our 4.1 kernel. And I saw crash frequently even if I only do a single print
or infinite loop. There is some small changes about tls register after 4.1. I
am not sure if it is a similar issue. It is great if you have some suggestions/
ideas.

Thanks.

Bamvor
 > so maybe this is something similar...
>
> Except of this, I think ILP32 series is looking pretty well, at least
> kernel part.
>
> If you have any ideas/suggestions, I'll really appreciate it.
>
> Yury.
>
> strace -f ./trigo
> [...]
> clone(child_stack=0xdbbfb000,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND
> |CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
> |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0xdbbfb4f8, tls=0xdbbfb920, child_tidptr=0xdbbfb4f8) = 32030
> rt_sigprocmask(SIG_BLOCK, [CHLD], Process 32030 attached [], 8) = 0
> [pid 32029] rt_sigaction(SIGCHLD, NULL,  <unfinished ...>
> [pid 32030] set_robust_list(0xdbbfb504, 12 <unfinished ...>
> [pid 32029] <... rt_sigaction resumed> {SIG_DFL, [ILL ABRT SEGV URG], 0}, 8) = 0
> [pid 32030] <... set_robust_list resumed> ) = 0
> [pid 32029] rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
> [pid 32030] write(1, "started\n", 8started
> <unfinished ...>
> [pid 32029] nanosleep({1, 65536},  <unfinished ...>
> [pid 32030] <... write resumed> )       = 8
> [pid 32030] rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
> [pid 32030] rt_sigsuspend([] <unfinished ...>
> [pid 32029] <... nanosleep resumed> 0xfff9fd98) = 0
> [pid 32029] write(1, "stoping...\n", 11stoping...) = 11
> [pid 32029] openat(AT_FDCWD, "/root/sys-root/libilp32/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3
> [pid 32029] read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\267\0\1\0\0\0 \0\0004\0\0\0"..., 512) = 512
> [pid 32029] fstat(3, {st_mode=S_IFREG|0644, st_size=429138, ...}) = 0
> [pid 32029] mmap(NULL, 135104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xdb3db000
> [pid 32029] mprotect(0xdb3ec000, 61440, PROT_NONE) = 0
> [pid 32029] mmap(0xdb3fb000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x10000) = 0xdb3fb000
> [pid 32029] close(3)                    = 0
> [pid 32029] tgkill(32029, 32030, SIGRTMIN) = 0
> [pid 32030] <... rt_sigsuspend resumed> ) = ? ERESTARTNOHAND (To be
> restarted if no handler)
> [pid 32029] write(1, "pthread_cancel == 0\n", 20pthread_cancel == 0) = 20
> [pid 32030] --- SIGRTMIN {si_signo=SIGRTMIN, si_code=SI_TKILL, si_pid=32029, si_uid=0} ---
> [pid 32029] write(1, "stopped\n", 8stopped
> <unfinished ...>
> [pid 32030] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x14} ---
> [pid 32029] <... write resumed> )       = ? <unavailable>
> [pid 32030] +++ killed by SIGSEGV +++
> +++ killed by SIGSEGV +++
> Segmentation fault
>
> dmesg:
> trigo[32246]: unhandled level 2 translation fault (11) at 0x00000014,
> esr 0x90000006
> pgd = ffffffc009335000
> [00000014] *pgd=000000007917c003, *pud=000000007917c003,
> *pmd=0000000000000000
>
> CPU: 2 PID: 32246 Comm: trigo Not tainted 4.5.0+ #91
> Hardware name: linux,dummy-virt (DT)
> task: ffffffc00900e400 ti: ffffffc009078000 task.ti: ffffffc009078000
> PC is at 0xda6853f0
> LR is at 0xda6d5440
> pc : [<00000000da6853f0>] lr : [<00000000da6d5440>] pstate: 60000000
> sp : 00000000da511bc0
> x29: 00000000da512e10 x28: 00000000da6a7000
> x27: 0000000000000000 x26: 00000000da513490
> x25: 0000000000000000 x24: 0000000000400820
> x23: 00000000da6a9000 x22: 00000000ff869acb
> x21: 00000000da6a9000 x20: 00000000da512e50
> x19: 0000000000000000 x18: 0000000000000001
> x17: 0000000000410bd8 x16: 00000000da691138
> x15: 0000000000000000 x14: 0000000000000000
> x13: 00000000da535970 x12: 0000000000000038
> x11: 0000000000000028 x10: 0101010101010101
> x9 : ff63647371607372 x8 : 0000000000000085
> x7 : 0000000000007df5 x6 : 00000000da512e1c
> x5 : 00000000da513518 x4 : 0000000000000002
> x3 : 00000000da513920 x2 : 0000000000000000
> x1 : 0000000000000008 x0 : 00000000da513490
>

[toc] | [prev] | [next] | [standalone]

#1361730

Hi, Yury

On 2016/3/20 16:12, Zhangjian (Bamvor) wrote:
> Hi, Yury
>
> On 2016/3/19 0:46, Yury Norov wrote:
[...]
>> The minimal test reproducing it is attached. The similar test where
>> parent forks a child and then kills it, works fine. (Attached too).
>>
>> I see that in case of pthread, there's much more stuff that is cloned.
>> Other's looking similar.
>>
>> pthread_create():
>> clone(child_stack=0xb953cea0, flags=CLONE_VM|CLONE_FS|CLONE_FILES
>>          |CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
>>          |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
>>          parent_tidptr=0xb953d398, tls=0xb953d7c0, child_tidptr=0xb953d398) = 1650
>>
>> fork():
>> clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
>>          child_tidptr=0xe5af6278) = 30537
>>
>> So this most probably means that ilp32 code doesn't handle one of cloned
>> item properly. I have already discovered a bug where child processes
>> used parent TLS,
> It is a kernel bug or glibc bug? Could you please explain it or show the patch?
> The current ILP32 patches looks good to me. Recently, I backport these patches
> to our 4.1 kernel. And I saw crash frequently even if I only do a single print
> or infinite loop. There is some small changes about tls register after 4.1. I
> am not sure if it is a similar issue. It is great if you have some suggestions/
> ideas.
My issue is because I forget to change is_compat_task to
is_a32_compat_task in arch/arm64/kernel/process.c such piece of code
is delete after commit d00a3810c162 ("arm64: context-switch user tls
register tpidr_el0 for compat tasks). It is not exist in upstream
kernel, never mind.

Meanwhile, I found that it seem that there is another is_compat_task
in tls_thread_flush. Is it relative the issue you mentioned?

```
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 432b094..9ab968c 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -209,7 +209,7 @@ static void tls_thread_flush(void)
  {
         asm ("msr tpidr_el0, xzr");

-       if (is_compat_task()) {
+       if (is_a32_compat_task()) {
                 current->thread.tp_value = 0;

                 /*
```

Regards

Bamvor

> Thanks.
>
> Bamvor
>  > so maybe this is something similar...
>>
>> Except of this, I think ILP32 series is looking pretty well, at least
>> kernel part.
>>
>> If you have any ideas/suggestions, I'll really appreciate it.
>>
>> Yury.
>>
>> strace -f ./trigo
>> [...]
>> clone(child_stack=0xdbbfb000,
>> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND
>> |CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
>> |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
>> parent_tidptr=0xdbbfb4f8, tls=0xdbbfb920, child_tidptr=0xdbbfb4f8) = 32030
>> rt_sigprocmask(SIG_BLOCK, [CHLD], Process 32030 attached [], 8) = 0
>> [pid 32029] rt_sigaction(SIGCHLD, NULL,  <unfinished ...>
>> [pid 32030] set_robust_list(0xdbbfb504, 12 <unfinished ...>
>> [pid 32029] <... rt_sigaction resumed> {SIG_DFL, [ILL ABRT SEGV URG], 0}, 8) = 0
>> [pid 32030] <... set_robust_list resumed> ) = 0
>> [pid 32029] rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
>> [pid 32030] write(1, "started\n", 8started
>> <unfinished ...>
>> [pid 32029] nanosleep({1, 65536},  <unfinished ...>
>> [pid 32030] <... write resumed> )       = 8
>> [pid 32030] rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
>> [pid 32030] rt_sigsuspend([] <unfinished ...>
>> [pid 32029] <... nanosleep resumed> 0xfff9fd98) = 0
>> [pid 32029] write(1, "stoping...\n", 11stoping...) = 11
>> [pid 32029] openat(AT_FDCWD, "/root/sys-root/libilp32/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3
>> [pid 32029] read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\267\0\1\0\0\0 \0\0004\0\0\0"..., 512) = 512
>> [pid 32029] fstat(3, {st_mode=S_IFREG|0644, st_size=429138, ...}) = 0
>> [pid 32029] mmap(NULL, 135104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xdb3db000
>> [pid 32029] mprotect(0xdb3ec000, 61440, PROT_NONE) = 0
>> [pid 32029] mmap(0xdb3fb000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x10000) = 0xdb3fb000
>> [pid 32029] close(3)                    = 0
>> [pid 32029] tgkill(32029, 32030, SIGRTMIN) = 0
>> [pid 32030] <... rt_sigsuspend resumed> ) = ? ERESTARTNOHAND (To be
>> restarted if no handler)
>> [pid 32029] write(1, "pthread_cancel == 0\n", 20pthread_cancel == 0) = 20
>> [pid 32030] --- SIGRTMIN {si_signo=SIGRTMIN, si_code=SI_TKILL, si_pid=32029, si_uid=0} ---
>> [pid 32029] write(1, "stopped\n", 8stopped
>> <unfinished ...>
>> [pid 32030] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x14} ---
>> [pid 32029] <... write resumed> )       = ? <unavailable>
>> [pid 32030] +++ killed by SIGSEGV +++
>> +++ killed by SIGSEGV +++
>> Segmentation fault
>>
>> dmesg:
>> trigo[32246]: unhandled level 2 translation fault (11) at 0x00000014,
>> esr 0x90000006
>> pgd = ffffffc009335000
>> [00000014] *pgd=000000007917c003, *pud=000000007917c003,
>> *pmd=0000000000000000
>>
>> CPU: 2 PID: 32246 Comm: trigo Not tainted 4.5.0+ #91
>> Hardware name: linux,dummy-virt (DT)
>> task: ffffffc00900e400 ti: ffffffc009078000 task.ti: ffffffc009078000
>> PC is at 0xda6853f0
>> LR is at 0xda6d5440
>> pc : [<00000000da6853f0>] lr : [<00000000da6d5440>] pstate: 60000000
>> sp : 00000000da511bc0
>> x29: 00000000da512e10 x28: 00000000da6a7000
>> x27: 0000000000000000 x26: 00000000da513490
>> x25: 0000000000000000 x24: 0000000000400820
>> x23: 00000000da6a9000 x22: 00000000ff869acb
>> x21: 00000000da6a9000 x20: 00000000da512e50
>> x19: 0000000000000000 x18: 0000000000000001
>> x17: 0000000000410bd8 x16: 00000000da691138
>> x15: 0000000000000000 x14: 0000000000000000
>> x13: 00000000da535970 x12: 0000000000000038
>> x11: 0000000000000028 x10: 0101010101010101
>> x9 : ff63647371607372 x8 : 0000000000000085
>> x7 : 0000000000007df5 x6 : 00000000da512e1c
>> x5 : 00000000da513518 x4 : 0000000000000002
>> x3 : 00000000da513920 x2 : 0000000000000000
>> x1 : 0000000000000008 x0 : 00000000da513490
>>
>

[toc] | [prev] | [next] | [standalone]

#1362072

On Mon, Mar 21, 2016 at 07:23:28PM +0800, Zhangjian (Bamvor) wrote:
> >>So this most probably means that ilp32 code doesn't handle one of cloned
> >>item properly. I have already discovered a bug where child processes
> >>used parent TLS,
> >It is a kernel bug or glibc bug? Could you please explain it or show the patch?
> >The current ILP32 patches looks good to me. Recently, I backport these patches
> >to our 4.1 kernel. And I saw crash frequently even if I only do a single print
> >or infinite loop. There is some small changes about tls register after 4.1. I
> >am not sure if it is a similar issue. It is great if you have some suggestions/
> >ideas.
> My issue is because I forget to change is_compat_task to
> is_a32_compat_task in arch/arm64/kernel/process.c such piece of code
> is delete after commit d00a3810c162 ("arm64: context-switch user tls
> register tpidr_el0 for compat tasks). It is not exist in upstream
> kernel, never mind.
> 
> Meanwhile, I found that it seem that there is another is_compat_task
> in tls_thread_flush. Is it relative the issue you mentioned?
> 
> ```
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 432b094..9ab968c 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -209,7 +209,7 @@ static void tls_thread_flush(void)
>  {
>         asm ("msr tpidr_el0, xzr");
> 
> -       if (is_compat_task()) {
> +       if (is_a32_compat_task()) {
>                 current->thread.tp_value = 0;
> 
>                 /*
> ```
> 
> Regards
> 
> Bamvor

Hi,

This fix looks correct, though doesn't fix issue.
Thank you.

Yury.

[toc] | [prev] | [next] | [standalone]

#1361625

This patch may fix a few LTP tests.

Andreas.

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
index 3631903..d1010db 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
@@ -25,18 +25,29 @@
 #define __O_NOFOLLOW	0100000
 #define __O_DIRECT	0200000
 
-#define __O_LARGEFILE	0
+#ifdef __ILP32__
+# define __O_LARGEFILE	0400000
+#else
+# define __O_LARGEFILE	0
+#endif
 
+#ifndef __ILP32__
 # define F_GETLK64	5
 # define F_SETLK64	6
 # define F_SETLKW64	7
+#endif
 
 struct flock
   {
     short int l_type;	/* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK.	*/
     short int l_whence;	/* Where `l_start' is relative to (like `lseek').  */
+#ifndef __USE_FILE_OFFSET64
     __off_t l_start;	/* Offset where the lock begins.  */
     __off_t l_len;	/* Size of the locked area; zero means until EOF.  */
+#else
+    __off64_t l_start;	/* Offset where the lock begins.  */
+    __off64_t l_len;	/* Size of the locked area; zero means until EOF.  */
+#endif
     __pid_t l_pid;	/* Process holding the lock.  */
   };
 
-- 
2.7.3

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[toc] | [prev] | [next] | [standalone]

#1361646

On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
> This patch may fix a few LTP tests.
> 

Thanks for analyzing.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> index 3631903..d1010db 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> @@ -25,18 +25,29 @@
>  #define __O_NOFOLLOW	0100000
>  #define __O_DIRECT	0200000
>  
> -#define __O_LARGEFILE	0
> +#ifdef __ILP32__
> +# define __O_LARGEFILE	0400000
> +#else
> +# define __O_LARGEFILE	0
> +#endif
>  

I guess this means I screwed up when I said I'd merged the kernel patch
that Yury did to fix it, sorry about that.

We need the patch to make all new architecture in the kernel default to
O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
patches as part of the ILP32 series after all, to make sure they are
merged at the point when they are needed.

> +#ifndef __ILP32__
>  # define F_GETLK64	5
>  # define F_SETLK64	6
>  # define F_SETLKW64	7
> +#endif
>  
>  struct flock
>    {
>      short int l_type;	/* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK.	*/
>      short int l_whence;	/* Where `l_start' is relative to (like `lseek').  */
> +#ifndef __USE_FILE_OFFSET64
>      __off_t l_start;	/* Offset where the lock begins.  */
>      __off_t l_len;	/* Size of the locked area; zero means until EOF.  */
> +#else
> +    __off64_t l_start;	/* Offset where the lock begins.  */
> +    __off64_t l_len;	/* Size of the locked area; zero means until EOF.  */
> +#endif
>      __pid_t l_pid;	/* Process holding the lock.  */
>    };

This looks like there is another bug as well, but I think this is in
libc, not in the kernel. I'm sure we had discussed this at some point
but I forgot what the outcome was. Defining 'struct flock' to have a
32-bit l_start and l_len member cannot be right if the kernel only
supports 64-bit offsets.

My guess is that the libc should either not define __off_t at all for
ILP32, and always use __off64_t in struct flock, or __off_t should
be defined as __kernel_loff_t a.k.a. long long so the #ifdef can be
avoided.

What exactly do you need to define F_GETLK64 for on LP64?

	Arnd

[toc] | [prev] | [next] | [standalone]

#1361704

Arnd Bergmann <arnd@arndb.de> writes:

> What exactly do you need to define F_GETLK64 for on LP64?

To override the generic definitions.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[toc] | [prev] | [next] | [standalone]

#1362000

On Monday 21 March 2016 11:52:54 Andreas Schwab wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> 
> > What exactly do you need to define F_GETLK64 for on LP64?
> 
> To override the generic definitions.


Ok, got it. I misread that part as adding definitions for LP64, but it
is correctly removing the definitions for ILP32.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1364938

Hi, Arnd

On 2016/3/21 17:43, Arnd Bergmann wrote:
> On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
>> This patch may fix a few LTP tests.
>>
>
> Thanks for analyzing.
>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>> index 3631903..d1010db 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>> @@ -25,18 +25,29 @@
>>   #define __O_NOFOLLOW	0100000
>>   #define __O_DIRECT	0200000
>>
>> -#define __O_LARGEFILE	0
>> +#ifdef __ILP32__
>> +# define __O_LARGEFILE	0400000
>> +#else
>> +# define __O_LARGEFILE	0
>> +#endif
>>
>
> I guess this means I screwed up when I said I'd merged the kernel patch
> that Yury did to fix it, sorry about that.
>
> We need the patch to make all new architecture in the kernel default to
> O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
> patches as part of the ILP32 series after all, to make sure they are
> merged at the point when they are needed.

I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
the new 32bit architecture should use 64bit off_t.

Should we define off_t in aarch64(for both ilp32 and lp64) in
typesize.h as following?

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
index 7073493..13b77c5 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
@@ -33,7 +33,7 @@
  #define __INO64_T_TYPE         __UQUAD_TYPE
  #define __MODE_T_TYPE          __U32_TYPE
  #define __NLINK_T_TYPE         __U32_TYPE
-#define __OFF_T_TYPE           __SLONGWORD_TYPE
+#define __OFF_T_TYPE           __SQUAD_TYPE
  #define __OFF64_T_TYPE         __SQUAD_TYPE
  #define __PID_T_TYPE           __S32_TYPE
  #define __RLIM_T_TYPE          __ULONGWORD_TYPE

Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
aarch64. And truncate and ftruncate is same as truncate64 and
ftruncate64.

Otherwise we need to handle the pad like yury do it in
stat.h, and we need to handle the bigendian as well:

 From 61949bf70527b9cc450e7bbdba9182f7f120c5bd Sun March 26 00:00:00 2016
From: j00321192 <jijun2@huawei.com>
Date: Thu, 24 Mar 2016 22:10:25 +0800
Subject: [PATCH] Fix endian issue in struct stat and stat64

There is endian issue in the existence bits/stat.h in aarch64. Fix
it by add the __AARCH64EB__ with proper pad.

Tested in our arm64 hardware. It could fix all the *stat test cases.

Signed-off-by: Yongliang Gao <gaoyongliang@huawei.com>
Signed-off-by: Jun Ji <jijun2@huawei.com>
Signed-off-by: Yang Liu(Young) <young.liuyang@huawei.com>
Signed-off-by: Zhang Jian(Bamvor) <bamvor.zhangjian@huawei.com>
---
  sysdeps/unix/sysv/linux/aarch64/bits/stat.h | 38 +++++++++++++++++++++++++----
  1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/stat.h b/sysdeps/unix/sysv/linux/aarch64/bits/stat.h
index 3d50e7a..4c6e072 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/stat.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/stat.h
@@ -35,12 +35,21 @@ struct stat
    {
      __dev_t st_dev;			/* Device.  */
  #ifdef __ILP32__
+
+#if !defined(__AARCH64EB__)
      unsigned int __st_ino_pad;
+#endif
+
  # ifndef __USE_FILE_OFFSET64
      __ino_t st_ino;			/* File serial number.	*/
  # else
      __ino_t __st_ino;			/* 32bit file serial number.	*/
  # endif
+
+#if defined(__AARCH64EB__)
+    unsigned int __st_ino_pad;
+#endif
+
  #else
  # ifndef __USE_FILE_OFFSET64
      __ino_t st_ino;			/* File serial number.	*/
@@ -55,10 +64,17 @@ struct stat
      __dev_t st_rdev;			/* Device number, if device.  */
      __dev_t __pad1;
  #ifndef __USE_FILE_OFFSET64
+
+#if defined(__ILP32__) && defined(__AARCH64EB__)
+    int __st_size_pad;
+#endif
+
      __off_t st_size;			/* Size of file, in bytes.  */
-# ifdef __ILP32__
+
+#if defined(__ILP32__) && !defined(__AARCH64EB__)
      int __st_size_pad;
-# endif
+#endif
+
  #else
      __off64_t st_size;			/* Size of file, in bytes.  */
  #endif
@@ -66,10 +82,17 @@ struct stat
      int __pad2;

  #ifndef __USE_FILE_OFFSET64
+
+#if defined (__ILP32__) && defined(__AARCH64EB__)
+    int __st_blocks_pad;
+#endif
+
      __blkcnt_t st_blocks;		/* Number 512-byte blocks allocated. */
-# ifdef __ILP32__
+
+#if defined (__ILP32__) && !defined(__AARCH64EB__)
      int __st_blocks_pad;
-# endif
+#endif
+
  #else
      __blkcnt64_t st_blocks;		/* Number 512-byte blocks allocated. */
  #endif
@@ -94,7 +117,7 @@ struct stat
      __time_t st_ctime;			/* Time of last status change.  */
      unsigned long int st_ctimensec;	/* Nsecs of last status change.  */
  #endif
-#if !defined __ILP32__ || !defined __USE_FILE_OFFSET64
+#if !defined (__ILP32__) || !defined __USE_FILE_OFFSET64
      int __glibc_reserved[2];
  #else
      __ino64_t st_ino;			/* File serial number.	*/
@@ -106,8 +129,13 @@ struct stat64
    {
      __dev_t st_dev;			/* Device.  */
  # ifdef __ILP32__
+    #if !defined(__AARCH64EB__)
      unsigned int __st_ino_pad;
+    #endif
      __ino_t __st_ino;			/* 32bit file serial number.	*/
+    #if defined(__AARCH64EB__)
+    unsigned int __st_ino_pad;
+    #endif
  # else
      __ino64_t st_ino;			/* File serial number.	*/
  # endif
-- 
1.8.4.5

Regards

bamvor


>> +#ifndef __ILP32__
>>   # define F_GETLK64	5
>>   # define F_SETLK64	6
>>   # define F_SETLKW64	7
>> +#endif
>>
>>   struct flock
>>     {
>>       short int l_type;	/* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK.	*/
>>       short int l_whence;	/* Where `l_start' is relative to (like `lseek').  */
>> +#ifndef __USE_FILE_OFFSET64
>>       __off_t l_start;	/* Offset where the lock begins.  */
>>       __off_t l_len;	/* Size of the locked area; zero means until EOF.  */
>> +#else
>> +    __off64_t l_start;	/* Offset where the lock begins.  */
>> +    __off64_t l_len;	/* Size of the locked area; zero means until EOF.  */
>> +#endif
>>       __pid_t l_pid;	/* Process holding the lock.  */
>>     };
>
> This looks like there is another bug as well, but I think this is in
> libc, not in the kernel. I'm sure we had discussed this at some point
> but I forgot what the outcome was. Defining 'struct flock' to have a
> 32-bit l_start and l_len member cannot be right if the kernel only
> supports 64-bit offsets.
>
> My guess is that the libc should either not define __off_t at all for
> ILP32, and always use __off64_t in struct flock, or __off_t should
> be defined as __kernel_loff_t a.k.a. long long so the #ifdef can be
> avoided.
>
> What exactly do you need to define F_GETLK64 for on LP64?
>
> 	Arnd
>

[toc] | [prev] | [next] | [standalone]

#1366155

On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
> Hi, Arnd
> 
> On 2016/3/21 17:43, Arnd Bergmann wrote:
> > On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
> >> This patch may fix a few LTP tests.
> >>
> >
> > Thanks for analyzing.
> >
> >> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> >> index 3631903..d1010db 100644
> >> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> >> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> >> @@ -25,18 +25,29 @@
> >>   #define __O_NOFOLLOW	0100000
> >>   #define __O_DIRECT	0200000
> >>
> >> -#define __O_LARGEFILE	0
> >> +#ifdef __ILP32__
> >> +# define __O_LARGEFILE	0400000
> >> +#else
> >> +# define __O_LARGEFILE	0
> >> +#endif
> >>
> >
> > I guess this means I screwed up when I said I'd merged the kernel patch
> > that Yury did to fix it, sorry about that.
> >
> > We need the patch to make all new architecture in the kernel default to
> > O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
> > patches as part of the ILP32 series after all, to make sure they are
> > merged at the point when they are needed.
> 
> I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
> ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
> the new 32bit architecture should use 64bit off_t.

Ah, so it is part of the series. I had not checked that here.

> Should we define off_t in aarch64(for both ilp32 and lp64) in
> typesize.h as following?
> 
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> index 7073493..13b77c5 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> @@ -33,7 +33,7 @@
>   #define __INO64_T_TYPE         __UQUAD_TYPE
>   #define __MODE_T_TYPE          __U32_TYPE
>   #define __NLINK_T_TYPE         __U32_TYPE
> -#define __OFF_T_TYPE           __SLONGWORD_TYPE
> +#define __OFF_T_TYPE           __SQUAD_TYPE
>   #define __OFF64_T_TYPE         __SQUAD_TYPE
>   #define __PID_T_TYPE           __S32_TYPE
>   #define __RLIM_T_TYPE          __ULONGWORD_TYPE
> 
> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> aarch64. And truncate and ftruncate is same as truncate64 and
> ftruncate64.

I don't know what the glibc developers prefer, but I think the
result needs to be something like that: either __OFF_T_TYPE is
defined as you write above as a 64-bit type, or the user-visible
off_t typedef unconditionally uses __OFF64_T_TYPE rather than
__OFF_T_TYPE.

> Otherwise we need to handle the pad like yury do it in
> stat.h, and we need to handle the bigendian as well:

I see.

> @@ -35,12 +35,21 @@ struct stat
>     {
>       __dev_t st_dev;			/* Device.  */
>   #ifdef __ILP32__
> +
> +#if !defined(__AARCH64EB__)
>       unsigned int __st_ino_pad;
> +#endif
> +
>   # ifndef __USE_FILE_OFFSET64
>       __ino_t st_ino;			/* File serial number.	*/
>   # else
>       __ino_t __st_ino;			/* 32bit file serial number.	*/
>   # endif
> +
> +#if defined(__AARCH64EB__)
> +    unsigned int __st_ino_pad;
> +#endif
> +
>   #else

This would indeed be silly, we really don't want anyone
to access the old __st_ino field or the 32-bit version of
the offset here.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1366234

On Tuesday 29 March 2016 15:01:47 Yury Norov wrote:
> On Tue, Mar 29, 2016 at 12:58:25PM +0200, Arnd Bergmann wrote:
> > On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
> > > 
> > > I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
> > > ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
> > > the new 32bit architecture should use 64bit off_t.
> > 
> > Ah, so it is part of the series. I had not checked that here.
> > 
> 
> I'm preparing new submission now. I can join off_t, s390 and ilp32
> patchsets. It seems, they will not be grabbed separately anyway, so
> this may decrease confusions like this.
> 
> Arnd?

Yes, that sounds good.

> > > Should we define off_t in aarch64(for both ilp32 and lp64) in
> > > typesize.h as following?
> > > 
> > > diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > > index 7073493..13b77c5 100644
> > > --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > > +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > > @@ -33,7 +33,7 @@
> > >   #define __INO64_T_TYPE         __UQUAD_TYPE
> > >   #define __MODE_T_TYPE          __U32_TYPE
> > >   #define __NLINK_T_TYPE         __U32_TYPE
> > > -#define __OFF_T_TYPE           __SLONGWORD_TYPE
> > > +#define __OFF_T_TYPE           __SQUAD_TYPE
> > >   #define __OFF64_T_TYPE         __SQUAD_TYPE
> > >   #define __PID_T_TYPE           __S32_TYPE
> > >   #define __RLIM_T_TYPE          __ULONGWORD_TYPE
> > > 
> > > Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> > > aarch64. And truncate and ftruncate is same as truncate64 and
> > > ftruncate64.
> > 
> > I don't know what the glibc developers prefer, but I think the
> > result needs to be something like that: either __OFF_T_TYPE is
> > defined as you write above as a 64-bit type, or the user-visible
> > off_t typedef unconditionally uses __OFF64_T_TYPE rather than
> > __OFF_T_TYPE.
> > 
> 
> I'm not the glibc developer as well, but I think it's OK.

Which of the two? I guess with the example that Bamvor gave
regarding struct stat, the latter is what we want, forcing the
use of __USE_FILE_OFFSET64 rather than changing the definition of
__OFF_T_TYPE.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1366284

Hi, Yury

On 2016/3/29 20:01, Yury Norov wrote:
> On Tue, Mar 29, 2016 at 12:58:25PM +0200, Arnd Bergmann wrote:
>> On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
>>> Hi, Arnd
>>>
>>> On 2016/3/21 17:43, Arnd Bergmann wrote:
>>>> On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
>>>>> This patch may fix a few LTP tests.
>>>>>
>>>>
>>>> Thanks for analyzing.
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>>>>> index 3631903..d1010db 100644
>>>>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>>>>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>>>>> @@ -25,18 +25,29 @@
>>>>>    #define __O_NOFOLLOW	0100000
>>>>>    #define __O_DIRECT	0200000
>>>>>
>>>>> -#define __O_LARGEFILE	0
>>>>> +#ifdef __ILP32__
>>>>> +# define __O_LARGEFILE	0400000
>>>>> +#else
>>>>> +# define __O_LARGEFILE	0
>>>>> +#endif
>>>>>
>>>>
>>>> I guess this means I screwed up when I said I'd merged the kernel patch
>>>> that Yury did to fix it, sorry about that.
>>>>
>>>> We need the patch to make all new architecture in the kernel default to
>>>> O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
>>>> patches as part of the ILP32 series after all, to make sure they are
>>>> merged at the point when they are needed.
>>>
>>> I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
>>> ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
>>> the new 32bit architecture should use 64bit off_t.
>>
>> Ah, so it is part of the series. I had not checked that here.
>>
>
> I'm preparing new submission now.
Cool:)
 > I can join off_t, s390 and ilp32
> patchsets. It seems, they will not be grabbed separately anyway, so
> this may decrease confusions like this.
>
> Arnd?
I am curious which one is more easily to get ack:p
>
>>> Should we define off_t in aarch64(for both ilp32 and lp64) in
>>> typesize.h as following?
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
>>> index 7073493..13b77c5 100644
>>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
>>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
>>> @@ -33,7 +33,7 @@
>>>    #define __INO64_T_TYPE         __UQUAD_TYPE
>>>    #define __MODE_T_TYPE          __U32_TYPE
>>>    #define __NLINK_T_TYPE         __U32_TYPE
>>> -#define __OFF_T_TYPE           __SLONGWORD_TYPE
>>> +#define __OFF_T_TYPE           __SQUAD_TYPE
>>>    #define __OFF64_T_TYPE         __SQUAD_TYPE
>>>    #define __PID_T_TYPE           __S32_TYPE
>>>    #define __RLIM_T_TYPE          __ULONGWORD_TYPE
>>>
>>> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
>>> aarch64. And truncate and ftruncate is same as truncate64 and
>>> ftruncate64.
>>
>> I don't know what the glibc developers prefer, but I think the
>> result needs to be something like that: either __OFF_T_TYPE is
>> defined as you write above as a 64-bit type, or the user-visible
>> off_t typedef unconditionally uses __OFF64_T_TYPE rather than
>> __OFF_T_TYPE.
>>
>
> I'm not the glibc developer as well, but I think it's OK.
IIUC, it is usually what glibc does.
If we want to define off_t to 64bit in ilp32, the follow syscall may
need to define as non-compat too:
sys_fadvise64
sys_sendfile
sys_sendfile64
sys_lseek
sys_splice
sys_sync_file_range2
sys_truncate
sys_ftruncate

Regards

Bamvor

>
>>> Otherwise we need to handle the pad like yury do it in
>>> stat.h, and we need to handle the bigendian as well:
>>
>> I see.
>>
>>> @@ -35,12 +35,21 @@ struct stat
>>>      {
>>>        __dev_t st_dev;			/* Device.  */
>>>    #ifdef __ILP32__
>>> +
>>> +#if !defined(__AARCH64EB__)
>>>        unsigned int __st_ino_pad;
>>> +#endif
>>> +
>>>    # ifndef __USE_FILE_OFFSET64
>>>        __ino_t st_ino;			/* File serial number.	*/
>>>    # else
>>>        __ino_t __st_ino;			/* 32bit file serial number.	*/
>>>    # endif
>>> +
>>> +#if defined(__AARCH64EB__)
>>> +    unsigned int __st_ino_pad;
>>> +#endif
>>> +
>>>    #else
>>
>> This would indeed be silly, we really don't want anyone
>> to access the old __st_ino field or the 32-bit version of
>> the offset here.
>>
>> 	Arnd

[toc] | [prev] | [next] | [standalone]

#1366289

On Tuesday 29 March 2016 21:21:49 Zhangjian wrote:
> >>>
> >>> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> >>> aarch64. And truncate and ftruncate is same as truncate64 and
> >>> ftruncate64.
> >>
> >> I don't know what the glibc developers prefer, but I think the
> >> result needs to be something like that: either __OFF_T_TYPE is
> >> defined as you write above as a 64-bit type, or the user-visible
> >> off_t typedef unconditionally uses __OFF64_T_TYPE rather than
> >> __OFF_T_TYPE.
> >>
> >
> > I'm not the glibc developer as well, but I think it's OK.
> IIUC, it is usually what glibc does.
> If we want to define off_t to 64bit in ilp32, the follow syscall may
> need to define as non-compat too:
> sys_fadvise64
> sys_sendfile
> sys_sendfile64
> sys_lseek
> sys_splice
> sys_sync_file_range2
> sys_truncate
> sys_ftruncate

I'm not following here. Do you mean in the kernel or in glibc?

In the kernel, the list of syscalls is fine, because we already only
provide syscalls passing loff_t as I said, and that is 64-bit.

In glibc, I think we need to define fewer entry points, not more.
Instead of having both lseek and lseek64, only one of them should
be provided, and that should always take a 64-bit offset, calling
into the kernel with the _llseek syscall entry.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1366416

On Tue, 29 Mar 2016, Arnd Bergmann wrote:

> In glibc, I think we need to define fewer entry points, not more.
> Instead of having both lseek and lseek64, only one of them should
> be provided, and that should always take a 64-bit offset, calling
> into the kernel with the _llseek syscall entry.

lseek64 is part of the public API, on all platforms.  It should be aliased 
to lseek where possible.

Strictly, it would be possible to provide it in the API without it being 
part of the ABI, by arranging the headers so that calls to lseek64 result 
in objects with a reference to lseek (because it uses the off64_t typedef, 
it's not valid to declare it yourself rather than including a header that 
declares it).  I don't think it would be a good idea for a new 
sub-architecture port to try introducing such a difference from all other 
ports, however.

-- 
Joseph S. Myers
joseph@codesourcery.com

[toc] | [prev] | [next] | [standalone]

#1366587

On Tuesday 29 March 2016 15:54:52 Joseph Myers wrote:
> On Tue, 29 Mar 2016, Arnd Bergmann wrote:
> 
> > In glibc, I think we need to define fewer entry points, not more.
> > Instead of having both lseek and lseek64, only one of them should
> > be provided, and that should always take a 64-bit offset, calling
> > into the kernel with the _llseek syscall entry.
> 
> lseek64 is part of the public API, on all platforms.  It should be aliased 
> to lseek where possible.

Right, makes sense.

> Strictly, it would be possible to provide it in the API without it being 
> part of the ABI, by arranging the headers so that calls to lseek64 result 
> in objects with a reference to lseek (because it uses the off64_t typedef, 
> it's not valid to declare it yourself rather than including a header that 
> declares it).  I don't think it would be a good idea for a new 
> sub-architecture port to try introducing such a difference from all other 
> ports, however.

How do we do it then? Should we just define __USE_FILE_OFFSET64
unconditionally for all new 32-bit architectures and leave the
code dealing with 32-bit off_t/ino_t in place but unreachable, to
minimize the differences?

Or should all the obsolete types be defined the same way as their
replacements so we have 64-bit __OFF_T_TYPE/__INO_T_TYPE
and use the same binary implementation regardless of FILE_OFFSET_BITS?

	Arnd

[toc] | [prev] | [next] | [standalone]

#1366644

On Tue, 29 Mar 2016, Arnd Bergmann wrote:

> How do we do it then? Should we just define __USE_FILE_OFFSET64
> unconditionally for all new 32-bit architectures and leave the
> code dealing with 32-bit off_t/ino_t in place but unreachable, to
> minimize the differences?

Defining __USE_FILE_OFFSET64 unconditionally would prevent glibc from 
building (see: how the patches a while back prototyping changing the 
default had to disable the change when glibc itself is built).  A change 
in the default, though desired (someone needs to pick up those patches 
together with the analysis done of possible impact on distributions), 
should not be tied to a new port, and would need to be discussed 
thoroughly on libc-alpha.

> Or should all the obsolete types be defined the same way as their
> replacements so we have 64-bit __OFF_T_TYPE/__INO_T_TYPE
> and use the same binary implementation regardless of FILE_OFFSET_BITS?

I think so (along with using wordsize-64 sysdeps directories as far as 
possible, like x32 does).  But design questions for a glibc port really 
belong on libc-alpha to get any sort of community consensus.

-- 
Joseph S. Myers
joseph@codesourcery.com

[toc] | [prev] | [next] | [standalone]

#1366666

On Tuesday 29 March 2016 20:15:10 Joseph Myers wrote:
> On Tue, 29 Mar 2016, Arnd Bergmann wrote:
> 
> > How do we do it then? Should we just define __USE_FILE_OFFSET64
> > unconditionally for all new 32-bit architectures and leave the
> > code dealing with 32-bit off_t/ino_t in place but unreachable, to
> > minimize the differences?
> 
> Defining __USE_FILE_OFFSET64 unconditionally would prevent glibc from 
> building (see: how the patches a while back prototyping changing the 
> default had to disable the change when glibc itself is built).  A change 
> in the default, though desired (someone needs to pick up those patches 
> together with the analysis done of possible impact on distributions), 
> should not be tied to a new port, and would need to be discussed 
> thoroughly on libc-alpha.

Ok

> > Or should all the obsolete types be defined the same way as their
> > replacements so we have 64-bit __OFF_T_TYPE/__INO_T_TYPE
> > and use the same binary implementation regardless of FILE_OFFSET_BITS?
> 
> I think so (along with using wordsize-64 sysdeps directories as far as 
> possible, like x32 does).  But design questions for a glibc port really 
> belong on libc-alpha to get any sort of community consensus.

I thought the wordsize-64 stuff was for the x86 mode where they
define __kernel_long_t as 64-bit. We don't really want to do that in
the kernel for new 32-bit architectures, that would make the kernel
ABI different from all the existing architectures.

The kernel ABI for ilp32 follows the usual wordsize-32 definitions
to make it easy for glibc while avoiding the problems that came from
redefining __kernel_long_t.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1366710

On Tue, 29 Mar 2016, Arnd Bergmann wrote:

> > I think so (along with using wordsize-64 sysdeps directories as far as 
> > possible, like x32 does).  But design questions for a glibc port really 
> > belong on libc-alpha to get any sort of community consensus.
> 
> I thought the wordsize-64 stuff was for the x86 mode where they
> define __kernel_long_t as 64-bit. We don't really want to do that in
> the kernel for new 32-bit architectures, that would make the kernel
> ABI different from all the existing architectures.

In general the wordsize-64 directories cover various relations of the form 
"function X is an alias for function Y", which derive from "type X is 
ABI-compatible with type Y".  (Unfortunately, the precise set isn't 
well-defined, resulting in problems for cases that want a subset of those 
relations - e.g. MIPS n64 where struct stat and struct stat64 are 
different, and so sysdeps/unix/sysv/linux/wordsize-64 isn't used.)

The person doing the port will need to do a detailed review of the exact 
effects of the wordsize-64 directories in current glibc, and which of 
those effects are appropriate for this port, to determine what is 
appropriate, and to include that analysis with the port submission.

Many of the relations relate to things controlled by _FILE_OFFSET_BITS=64 
- if _FILE_OFFSET_BITS=64 does not affect the ABI of off_t, struct stat, 
etc., then many of the aliases are correct.  Some relations may relate to 
other things such as long and long long being ABI compatible - where an 
alias in the correct direction can be OK for long arguments (not returns) 
if a long argument is always sign-extended to long long when passed to a 
function, for example.

-- 
Joseph S. Myers
joseph@codesourcery.com

[toc] | [prev] | [next] | [standalone]

#1366737

On Tuesday 29 March 2016 21:00:56 Joseph Myers wrote:
> On Tue, 29 Mar 2016, Arnd Bergmann wrote:
> 
> > > I think so (along with using wordsize-64 sysdeps directories as far as 
> > > possible, like x32 does).  But design questions for a glibc port really 
> > > belong on libc-alpha to get any sort of community consensus.
> > 
> > I thought the wordsize-64 stuff was for the x86 mode where they
> > define __kernel_long_t as 64-bit. We don't really want to do that in
> > the kernel for new 32-bit architectures, that would make the kernel
> > ABI different from all the existing architectures.
> 
> In general the wordsize-64 directories cover various relations of the form 
> "function X is an alias for function Y", which derive from "type X is 
> ABI-compatible with type Y".  (Unfortunately, the precise set isn't 
> well-defined, resulting in problems for cases that want a subset of those 
> relations - e.g. MIPS n64 where struct stat and struct stat64 are 
> different, and so sysdeps/unix/sysv/linux/wordsize-64 isn't used.)

For all new 32-bit architectures like this, the kernel should at least
behave in a consistent way, but it's somewhere inbetween wordsize-32 and
wordsize-64 for 32-bit architectures, because off_t and ino_t are mapped
to the 64-bit __kernel_loff_t and __kernel_ino_t, while time_t, clock_t
and size_t are mapped to 32 bit types.

> The person doing the port will need to do a detailed review of the exact 
> effects of the wordsize-64 directories in current glibc, and which of 
> those effects are appropriate for this port, to determine what is 
> appropriate, and to include that analysis with the port submission.

I think the analysis will have to be about two separate things:

* Whether new 32-bit architectures in general should use wordsize-32
  or wordsize-64 or something else, based on what you write above.
  I would still guess that we are better off adapting wordsize-32
  to the current kernel ABI for 32-bit architectures while leaving
  wordsize-64 to real 64-bit architectures and x86/x32.

* How we deal with the special case of this architecture having
  nonstandard calling conventions for a couple of syscalls that
  take 64-bit arguments in a single register rather than two registers
  as every other 32-bit architecture does.

	Arnd

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web