diff mbox

[20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

Message ID 5731AE2E.2050403@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

zhangjian May 10, 2016, 9:47 a.m. UTC
Hi, Arnd

On 2016/5/10 16:36, Arnd Bergmann wrote:
> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
>> On 2016/5/6 20:37, Yury Norov wrote:
>>> On Fri, May 06, 2016 at 08:16:48PM +0800, Zhangjian (Bamvor) wrote:
>>>
>>> AFAIR, here we don't shift offset, as it's 64-bit both in user-
>>> and kernel-space,
>> In your ilp32-2.22 branch, you wrapper mmap to mmap2 in which type of
>> offset is off_t. And off_t is 32bit in ilp32, correct?
>> "sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c"
>> /* mmap is provided by mmap as they are the same. */
>> void *__mmap (void *__addr, size_t __len, int __prot,
>>                        int __flags, int __fd, __off_t __offset)
>> {
>>      void *result;
>>      result = (void *)
>>        INLINE_SYSCALL (mmap2, 6, __addr,
>>                        __len, __prot, __flags, __fd, __offset);
>>      return result;
>> }
>
> __off_t should be 'long long' on new architectures, and map
> to __kernel_loff_t.
>
> Can you see how it is defined?
For kernel part, in "include/uapi/asm-generic/posix_types.h":
     #ifndef __kernel_long_t
     typedef long            __kernel_long_t;
     typedef unsigned long   __kernel_ulong_t;
     #endif
     typedef __kernel_long_t __kernel_off_t;
in "include/linux/types.h":
     typedef __kernel_off_t          off_t;

"include/uapi/asm-generic/posix_types.h" is uapi, we could not check
"ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which
mean it is 32bit in ILP32. should we define something like x32?
     ```
     diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h
     index 7985ff6..9baa8d3 100644
     --- a/arch/arm64/include/uapi/asm/posix_types.h
     +++ b/arch/arm64/include/uapi/asm/posix_types.h
     @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t;
      typedef unsigned short __kernel_old_gid_t;
       #define __kernel_old_uid_t __kernel_old_uid_t

      +typedef long long __kernel_long_t;
      +typedef unsigned long long __kernel_ulong_t;
      +
       #include <asm-generic/posix_types.h>

       #endif /*  __ASM_POSIX_TYPES_H */u
     ```
After this definition, the following kernel types is 64bit in ILP32:
     typedef __kernel_long_t         __kernel_suseconds_t;
     typedef __kernel_long_t __kernel_ssize_t;
     typedef __kernel_long_t __kernel_ptrdiff_t;
     typedef __kernel_long_t __kernel_off_t;
     typedef __kernel_long_t __kernel_time_t;
     typedef __kernel_long_t __kernel_clock_t;
     typedef __kernel_ulong_t __kernel_ino_t;
     typedef __kernel_ulong_t __kernel_size_t;

But it is not a generic way to define off_t to 64bit if
ARCH_32BIT_OFF_T is not defined. How about this one? We need to
define `__kernel_off_t` on all the old 32bit architecture like
arm:
```
```

On the other hand, glibc define it own off_t in "bits/types.h":
     __STD_TYPE __OFF_T_TYPE __off_t;        /* Type of file sizes and offsets.  */
     __STD_TYPE __OFF64_T_TYPE __off64_t;    /* Type of file sizes and offsets (LFS).  */

in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h":
     #define __OFF_T_TYPE            __SLONGWORD_TYPE
     #define __OFF64_T_TYPE          __SQUAD_TYPE

If we define off_t as 64bit in glibc:
     #define __OFF_T_TYPE           __SQUAD_TYPE

Should We need to align all the off_t syscall to 64bit syscall in
kernel?

Regards

Bamvor


> 	Arnd
>

Comments

Arnd Bergmann May 10, 2016, 11:48 a.m. UTC | #1
On Tuesday 10 May 2016 17:47:26 Zhangjian wrote:
> On 2016/5/10 16:36, Arnd Bergmann wrote:
> > On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
> >> On 2016/5/6 20:37, Yury Norov wrote:
> 
> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check
> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which
> mean it is 32bit in ILP32. should we define something like x32?
>      ```
>      diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h
>      index 7985ff6..9baa8d3 100644
>      --- a/arch/arm64/include/uapi/asm/posix_types.h
>      +++ b/arch/arm64/include/uapi/asm/posix_types.h

glibc does not use the definition of __kernel_off_t, it has its own
copy, so changing the kernel headers would do nothing.

>      @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t;
>       typedef unsigned short __kernel_old_gid_t;
>        #define __kernel_old_uid_t __kernel_old_uid_t
> 
>       +typedef long long __kernel_long_t;
>       +typedef unsigned long long __kernel_ulong_t;
>       +
>        #include <asm-generic/posix_types.h>
> 
>        #endif /*  __ASM_POSIX_TYPES_H */u
>      ```

This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t
are not just used for off_t but also other things.

> 
> On the other hand, glibc define it own off_t in "bits/types.h":
>      __STD_TYPE __OFF_T_TYPE __off_t;        /* Type of file sizes and offsets.  */
>      __STD_TYPE __OFF64_T_TYPE __off64_t;    /* Type of file sizes and offsets (LFS).  */
> 
> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h":
>      #define __OFF_T_TYPE            __SLONGWORD_TYPE
>      #define __OFF64_T_TYPE          __SQUAD_TYPE
> 
> If we define off_t as 64bit in glibc:
>      #define __OFF_T_TYPE           __SQUAD_TYPE
> 
> Should We need to align all the off_t syscall to 64bit syscall in
> kernel?
> 

Yes, this is the change that I think we need to make, along with
the same change for __INO_T_TYPE and 

#define __OFF_T_MATCHES_OFF64_T        1
#define __INO_T_MATCHES_INO64_T        1

If I read the rest of the glibc headers right, that should be all we need
to ensure that both off_t and off64_t match the __kernel_loff_t based
syscalls.

	Arnd
zhangjian May 10, 2016, 12:39 p.m. UTC | #2
Hi,

On 2016/5/10 19:48, Arnd Bergmann wrote:
> On Tuesday 10 May 2016 17:47:26 Zhangjian wrote:
>> On 2016/5/10 16:36, Arnd Bergmann wrote:
>>> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
>>>> On 2016/5/6 20:37, Yury Norov wrote:
>>
>> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check
>> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which
>> mean it is 32bit in ILP32. should we define something like x32?
>>       ```
>>       diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h
>>       index 7985ff6..9baa8d3 100644
>>       --- a/arch/arm64/include/uapi/asm/posix_types.h
>>       +++ b/arch/arm64/include/uapi/asm/posix_types.h
>
> glibc does not use the definition of __kernel_off_t, it has its own
> copy, so changing the kernel headers would do nothing.
Yes, I think so. I am puzzle that how do we ensure that all the new
32bit architecture will define off_t as 64bit after introduce
ARCH_32BIT_OFF_T and do not change any other code?
>
>>       @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t;
>>        typedef unsigned short __kernel_old_gid_t;
>>         #define __kernel_old_uid_t __kernel_old_uid_t
>>
>>        +typedef long long __kernel_long_t;
>>        +typedef unsigned long long __kernel_ulong_t;
>>        +
>>         #include <asm-generic/posix_types.h>
>>
>>         #endif /*  __ASM_POSIX_TYPES_H */u
>>       ```
>
> This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t
> are not just used for off_t but also other things.
Yes. But if we do not change __kernel_long_t and __kernel_ulong_t, the
application of ilp32 will think size of long in kernel is 32bit. Is that
correct?
>
>>
>> On the other hand, glibc define it own off_t in "bits/types.h":
>>       __STD_TYPE __OFF_T_TYPE __off_t;        /* Type of file sizes and offsets.  */
>>       __STD_TYPE __OFF64_T_TYPE __off64_t;    /* Type of file sizes and offsets (LFS).  */
>>
>> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h":
>>       #define __OFF_T_TYPE            __SLONGWORD_TYPE
>>       #define __OFF64_T_TYPE          __SQUAD_TYPE
>>
>> If we define off_t as 64bit in glibc:
>>       #define __OFF_T_TYPE           __SQUAD_TYPE
>>
>> Should We need to align all the off_t syscall to 64bit syscall in
>> kernel?
>>
>
> Yes, this is the change that I think we need to make, along with
> the same change for __INO_T_TYPE and
>
> #define __OFF_T_MATCHES_OFF64_T        1
> #define __INO_T_MATCHES_INO64_T        1
>
> If I read the rest of the glibc headers right, that should be all we need
> to ensure that both off_t and off64_t match the __kernel_loff_t based
> syscalls.
Ok, I will test the ltp syscall test.
With this changes, the issue I mentioned should be fixed. But we still
use mmap2 syscall for ILP32 application when we pass the offset instead
of page offset. Is it correct?

Regards

Bamvor

>
> 	Arnd
>
Arnd Bergmann May 10, 2016, 12:50 p.m. UTC | #3
On Tuesday 10 May 2016 20:39:41 Zhangjian wrote:
> Hi,
> 
> On 2016/5/10 19:48, Arnd Bergmann wrote:
> > On Tuesday 10 May 2016 17:47:26 Zhangjian wrote:
> >> On 2016/5/10 16:36, Arnd Bergmann wrote:
> >>> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
> >>>> On 2016/5/6 20:37, Yury Norov wrote:
> >>
> >> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check
> >> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which
> >> mean it is 32bit in ILP32. should we define something like x32?
> >>       ```
> >>       diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h
> >>       index 7985ff6..9baa8d3 100644
> >>       --- a/arch/arm64/include/uapi/asm/posix_types.h
> >>       +++ b/arch/arm64/include/uapi/asm/posix_types.h
> >
> > glibc does not use the definition of __kernel_off_t, it has its own
> > copy, so changing the kernel headers would do nothing.
> Yes, I think so. I am puzzle that how do we ensure that all the new
> 32bit architecture will define off_t as 64bit after introduce
> ARCH_32BIT_OFF_T and do not change any other code?

This is for historic reasons: we have two sets of system calls passing
file offsets: the original ones passing a 'long' are named e.g. lseek,
pread, pwrite, fadvise, ...

The replacement syscalls that pass a 'long long' __kernel_loff_t are only
available on 32-bit architectures and have different names, e.g. llseek,
pread64, pwrite64, fadvise64, ...

On 64-bit architectures, we only provide the first set, because
__kernel_off_t was already wide enough. On old 32-bit architectures
we have both, and on new 32-bit architectures we only have the second
set, which lets us use the same definitions on all 32-bit architectures.

> >>       @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t;
> >>        typedef unsigned short __kernel_old_gid_t;
> >>         #define __kernel_old_uid_t __kernel_old_uid_t
> >>
> >>        +typedef long long __kernel_long_t;
> >>        +typedef unsigned long long __kernel_ulong_t;
> >>        +
> >>         #include <asm-generic/posix_types.h>
> >>
> >>         #endif /*  __ASM_POSIX_TYPES_H */u
> >>       ```
> >
> > This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t
> > are not just used for off_t but also other things.
> Yes. But if we do not change __kernel_long_t and __kernel_ulong_t, the
> application of ilp32 will think size of long in kernel is 32bit. Is that
> correct?

No, __kernel_long_t refers to the definition of 'long' in the kernel/user
ABI, not in the kernel (except on the x86-64 "x32" ABI, which is weird).

A 32-bit user space application must not care about how 'long' is defined
in the kernel, it should run on either 32-bit kernels or 64-bit kernels.
We don't have any plans to do an ilp32-mode kernel, but if we ever want
one, it must use the exact same ABI as the 64-bit kernel when running
ilp32 user space.

> >>
> >> On the other hand, glibc define it own off_t in "bits/types.h":
> >>       __STD_TYPE __OFF_T_TYPE __off_t;        /* Type of file sizes and offsets.  */
> >>       __STD_TYPE __OFF64_T_TYPE __off64_t;    /* Type of file sizes and offsets (LFS).  */
> >>
> >> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h":
> >>       #define __OFF_T_TYPE            __SLONGWORD_TYPE
> >>       #define __OFF64_T_TYPE          __SQUAD_TYPE
> >>
> >> If we define off_t as 64bit in glibc:
> >>       #define __OFF_T_TYPE           __SQUAD_TYPE
> >>
> >> Should We need to align all the off_t syscall to 64bit syscall in
> >> kernel?
> >>
> >
> > Yes, this is the change that I think we need to make, along with
> > the same change for __INO_T_TYPE and
> >
> > #define __OFF_T_MATCHES_OFF64_T        1
> > #define __INO_T_MATCHES_INO64_T        1
> >
> > If I read the rest of the glibc headers right, that should be all we need
> > to ensure that both off_t and off64_t match the __kernel_loff_t based
> > syscalls.
> Ok, I will test the ltp syscall test.
> With this changes, the issue I mentioned should be fixed. But we still
> use mmap2 syscall for ILP32 application when we pass the offset instead
> of page offset. Is it correct?

I don't remember. It's probably not important whether we have the shift
in there, as long as it's independent of the actual kernel page size and
user space and kernel agree on the calling conventions.

	Arnd
zhangjian May 11, 2016, 2:04 a.m. UTC | #4
Hi,

On 2016/5/10 20:50, Arnd Bergmann wrote:
> On Tuesday 10 May 2016 20:39:41 Zhangjian wrote:
>> Hi,
>>
>> On 2016/5/10 19:48, Arnd Bergmann wrote:
>>> On Tuesday 10 May 2016 17:47:26 Zhangjian wrote:
>>>> On 2016/5/10 16:36, Arnd Bergmann wrote:
>>>>> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
>>>>>> On 2016/5/6 20:37, Yury Norov wrote:
>>>>
>>>> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check
>>>> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which
>>>> mean it is 32bit in ILP32. should we define something like x32?
>>>>        ```
>>>>        diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h
>>>>        index 7985ff6..9baa8d3 100644
>>>>        --- a/arch/arm64/include/uapi/asm/posix_types.h
>>>>        +++ b/arch/arm64/include/uapi/asm/posix_types.h
>>>
>>> glibc does not use the definition of __kernel_off_t, it has its own
>>> copy, so changing the kernel headers would do nothing.
>> Yes, I think so. I am puzzle that how do we ensure that all the new
>> 32bit architecture will define off_t as 64bit after introduce
>> ARCH_32BIT_OFF_T and do not change any other code?
>
> This is for historic reasons: we have two sets of system calls passing
> file offsets: the original ones passing a 'long' are named e.g. lseek,
> pread, pwrite, fadvise, ...
>
> The replacement syscalls that pass a 'long long' __kernel_loff_t are only
> available on 32-bit architectures and have different names, e.g. llseek,
> pread64, pwrite64, fadvise64, ...
>
> On 64-bit architectures, we only provide the first set, because
> __kernel_off_t was already wide enough. On old 32-bit architectures
> we have both, and on new 32-bit architectures we only have the second
> set, which lets us use the same definitions on all 32-bit architectures.
>
>>>>        @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t;
>>>>         typedef unsigned short __kernel_old_gid_t;
>>>>          #define __kernel_old_uid_t __kernel_old_uid_t
>>>>
>>>>         +typedef long long __kernel_long_t;
>>>>         +typedef unsigned long long __kernel_ulong_t;
>>>>         +
>>>>          #include <asm-generic/posix_types.h>
>>>>
>>>>          #endif /*  __ASM_POSIX_TYPES_H */u
>>>>        ```
>>>
>>> This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t
>>> are not just used for off_t but also other things.
>> Yes. But if we do not change __kernel_long_t and __kernel_ulong_t, the
>> application of ilp32 will think size of long in kernel is 32bit. Is that
>> correct?
>
> No, __kernel_long_t refers to the definition of 'long' in the kernel/user
> ABI, not in the kernel (except on the x86-64 "x32" ABI, which is weird).
Thanks, that is answer my question. I ask this because I saw the posix_types.h
for x32.
> A 32-bit user space application must not care about how 'long' is defined
> in the kernel, it should run on either 32-bit kernels or 64-bit kernels.
> We don't have any plans to do an ilp32-mode kernel, but if we ever want
> one, it must use the exact same ABI as the 64-bit kernel when running
> ilp32 user space.
>
>>>>
>>>> On the other hand, glibc define it own off_t in "bits/types.h":
>>>>        __STD_TYPE __OFF_T_TYPE __off_t;        /* Type of file sizes and offsets.  */
>>>>        __STD_TYPE __OFF64_T_TYPE __off64_t;    /* Type of file sizes and offsets (LFS).  */
>>>>
>>>> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h":
>>>>        #define __OFF_T_TYPE            __SLONGWORD_TYPE
>>>>        #define __OFF64_T_TYPE          __SQUAD_TYPE
>>>>
>>>> If we define off_t as 64bit in glibc:
>>>>        #define __OFF_T_TYPE           __SQUAD_TYPE
>>>>
>>>> Should We need to align all the off_t syscall to 64bit syscall in
>>>> kernel?
>>>>
>>>
>>> Yes, this is the change that I think we need to make, along with
>>> the same change for __INO_T_TYPE and
>>>
>>> #define __OFF_T_MATCHES_OFF64_T        1
>>> #define __INO_T_MATCHES_INO64_T        1
>>>
>>> If I read the rest of the glibc headers right, that should be all we need
>>> to ensure that both off_t and off64_t match the __kernel_loff_t based
>>> syscalls.
>> Ok, I will test the ltp syscall test.
>> With this changes, the issue I mentioned should be fixed. But we still
>> use mmap2 syscall for ILP32 application when we pass the offset instead
>> of page offset. Is it correct?
>
> I don't remember. It's probably not important whether we have the shift
> in there, as long as it's independent of the actual kernel page size and
> user space and kernel agree on the calling conventions.
Well. I am ok with where to shift the pages size because we get the same
result. I was just thinking if we should get rid of the name of mmap2 in our
ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
if they do not know the implementations.

Regards

Bamvor

>
> 	Arnd
>
Yury Norov May 11, 2016, 8:04 a.m. UTC | #5
On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
[...]

> >>Ok, I will test the ltp syscall test.
> >>With this changes, the issue I mentioned should be fixed. But we still
> >>use mmap2 syscall for ILP32 application when we pass the offset instead
> >>of page offset. Is it correct?
> >
> >I don't remember. It's probably not important whether we have the shift
> >in there, as long as it's independent of the actual kernel page size and
> >user space and kernel agree on the calling conventions.
> Well. I am ok with where to shift the pages size because we get the same
> result. I was just thinking if we should get rid of the name of mmap2 in our
> ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
> if they do not know the implementations.
> 

This is what generic unistd.h does. If you want to change it, you'd
change each arch that uses generic unistd.h.

> Regards
> 
> Bamvor
> 
> >
> >	Arnd
> >
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Arnd Bergmann May 11, 2016, 8:09 a.m. UTC | #6
On Wednesday 11 May 2016 10:04:16 Zhangjian wrote:
> > I don't remember. It's probably not important whether we have the shift
> > in there, as long as it's independent of the actual kernel page size and
> > user space and kernel agree on the calling conventions.
> Well. I am ok with where to shift the pages size because we get the same
> result. I was just thinking if we should get rid of the name of mmap2 in our
> ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
> if they do not know the implementations.

That is a good point: If the implementation matches the mmap() behavior rather than
mmap2(), we should rename the macro by doing

#undef __NR_mmap2
#define __NR_mmap 222

in the uapi/asm/unistd.h file for ilp32 mode. Alternatively we can keep the
__NR_mmap2 definition but then we need to pass the pgoff (value shifted by
12 bits) argument rather than the size in bytes.

	Arnd
Arnd Bergmann May 11, 2016, 10:55 a.m. UTC | #7
On Wednesday 11 May 2016 11:04:38 Yury Norov wrote:
> On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
> [...]
> 
> > >>Ok, I will test the ltp syscall test.
> > >>With this changes, the issue I mentioned should be fixed. But we still
> > >>use mmap2 syscall for ILP32 application when we pass the offset instead
> > >>of page offset. Is it correct?
> > >
> > >I don't remember. It's probably not important whether we have the shift
> > >in there, as long as it's independent of the actual kernel page size and
> > >user space and kernel agree on the calling conventions.
> > Well. I am ok with where to shift the pages size because we get the same
> > result. I was just thinking if we should get rid of the name of mmap2 in our
> > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
> > if they do not know the implementations.
> > 
> 
> This is what generic unistd.h does. If you want to change it, you'd
> change each arch that uses generic unistd.h.
> 

Generic unistd.h has this:

#ifdef __SYSCALL_COMPAT
#define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp)
#else
#define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64)
#endif

#define __NR3264_mmap 222
__SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)


#if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT)
#define __NR_mmap __NR3264_mmap
#else
#define __NR_mmap2 __NR3264_mmap
#endif

So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but
__NR_mmap and sys_mmap on 64-bit ABIs, as it should be.

The problem is that arch/arm64/kernel/sys_ilp32.c now overrides
this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think
we should either override both the implementation and the number,
or neither of them.

I think what happened is that you chose to override sys_mmap because
sys_mmap2 sometimes takes an off_t argument, however the kernel
just treats it as 'unsigned long', with a range of 44 bits for the
offset. Using sys_mmap with a __kernel_loff_t argument gives
us 64-bit range, but if we do that, I think it should be __NR_mmap.

	Arnd
Catalin Marinas May 11, 2016, 4:59 p.m. UTC | #8
On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote:
> On Wednesday 11 May 2016 11:04:38 Yury Norov wrote:
> > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
> > [...]
> > 
> > > >>Ok, I will test the ltp syscall test.
> > > >>With this changes, the issue I mentioned should be fixed. But we still
> > > >>use mmap2 syscall for ILP32 application when we pass the offset instead
> > > >>of page offset. Is it correct?
> > > >
> > > >I don't remember. It's probably not important whether we have the shift
> > > >in there, as long as it's independent of the actual kernel page size and
> > > >user space and kernel agree on the calling conventions.
> > > Well. I am ok with where to shift the pages size because we get the same
> > > result. I was just thinking if we should get rid of the name of mmap2 in our
> > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
> > > if they do not know the implementations.
> > > 
> > 
> > This is what generic unistd.h does. If you want to change it, you'd
> > change each arch that uses generic unistd.h.
> 
> Generic unistd.h has this:
> 
> #ifdef __SYSCALL_COMPAT
> #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp)
> #else
> #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64)
> #endif
> 
> #define __NR3264_mmap 222
> __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
> 
> 
> #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT)
> #define __NR_mmap __NR3264_mmap
> #else
> #define __NR_mmap2 __NR3264_mmap
> #endif
> 
> So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but
> __NR_mmap and sys_mmap on 64-bit ABIs, as it should be.
> 
> The problem is that arch/arm64/kernel/sys_ilp32.c now overrides
> this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think
> we should either override both the implementation and the number,
> or neither of them.

I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2)
to keep it close to new 32-bit architectures, even though we would have
some shifts by 12 in both glibc and kernel.
Arnd Bergmann May 11, 2016, 7:30 p.m. UTC | #9
On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote:
> On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote:
> > On Wednesday 11 May 2016 11:04:38 Yury Norov wrote:
> > > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
> > > [...]
> > > 
> > > > >>Ok, I will test the ltp syscall test.
> > > > >>With this changes, the issue I mentioned should be fixed. But we still
> > > > >>use mmap2 syscall for ILP32 application when we pass the offset instead
> > > > >>of page offset. Is it correct?
> > > > >
> > > > >I don't remember. It's probably not important whether we have the shift
> > > > >in there, as long as it's independent of the actual kernel page size and
> > > > >user space and kernel agree on the calling conventions.
> > > > Well. I am ok with where to shift the pages size because we get the same
> > > > result. I was just thinking if we should get rid of the name of mmap2 in our
> > > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
> > > > if they do not know the implementations.
> > > > 
> > > 
> > > This is what generic unistd.h does. If you want to change it, you'd
> > > change each arch that uses generic unistd.h.
> > 
> > Generic unistd.h has this:
> > 
> > #ifdef __SYSCALL_COMPAT
> > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp)
> > #else
> > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64)
> > #endif
> > 
> > #define __NR3264_mmap 222
> > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
> > 
> > 
> > #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT)
> > #define __NR_mmap __NR3264_mmap
> > #else
> > #define __NR_mmap2 __NR3264_mmap
> > #endif
> > 
> > So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but
> > __NR_mmap and sys_mmap on 64-bit ABIs, as it should be.
> > 
> > The problem is that arch/arm64/kernel/sys_ilp32.c now overrides
> > this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think
> > we should either override both the implementation and the number,
> > or neither of them.
> 
> I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2)
> to keep it close to new 32-bit architectures, even though we would have
> some shifts by 12 in both glibc and kernel.

I don't think the shifts are a problem, the main downside would be
the limit to 44 bits of file offsets (16TB files), but it's also
unclear if that is a practical problem at all. If it is, we run
into the same problem on all other 32-bit architectures too.

	Arnd
Catalin Marinas May 12, 2016, 9:17 a.m. UTC | #10
On Wed, May 11, 2016 at 09:30:07PM +0200, Arnd Bergmann wrote:
> On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote:
> > On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 11 May 2016 11:04:38 Yury Norov wrote:
> > > > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
> > > > [...]
> > > > 
> > > > > >>Ok, I will test the ltp syscall test.
> > > > > >>With this changes, the issue I mentioned should be fixed. But we still
> > > > > >>use mmap2 syscall for ILP32 application when we pass the offset instead
> > > > > >>of page offset. Is it correct?
> > > > > >
> > > > > >I don't remember. It's probably not important whether we have the shift
> > > > > >in there, as long as it's independent of the actual kernel page size and
> > > > > >user space and kernel agree on the calling conventions.
> > > > > Well. I am ok with where to shift the pages size because we get the same
> > > > > result. I was just thinking if we should get rid of the name of mmap2 in our
> > > > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
> > > > > if they do not know the implementations.
> > > > > 
> > > > 
> > > > This is what generic unistd.h does. If you want to change it, you'd
> > > > change each arch that uses generic unistd.h.
> > > 
> > > Generic unistd.h has this:
> > > 
> > > #ifdef __SYSCALL_COMPAT
> > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp)
> > > #else
> > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64)
> > > #endif
> > > 
> > > #define __NR3264_mmap 222
> > > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
> > > 
> > > 
> > > #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT)
> > > #define __NR_mmap __NR3264_mmap
> > > #else
> > > #define __NR_mmap2 __NR3264_mmap
> > > #endif
> > > 
> > > So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but
> > > __NR_mmap and sys_mmap on 64-bit ABIs, as it should be.
> > > 
> > > The problem is that arch/arm64/kernel/sys_ilp32.c now overrides
> > > this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think
> > > we should either override both the implementation and the number,
> > > or neither of them.
> > 
> > I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2)
> > to keep it close to new 32-bit architectures, even though we would have
> > some shifts by 12 in both glibc and kernel.
> 
> I don't think the shifts are a problem, the main downside would be
> the limit to 44 bits of file offsets (16TB files), but it's also
> unclear if that is a practical problem at all. If it is, we run
> into the same problem on all other 32-bit architectures too.

I hope people are seriously thinking of moving to an LP64 ABI if they
have such large file offset needs.
Arnd Bergmann May 12, 2016, 9:21 a.m. UTC | #11
On Thursday 12 May 2016 10:17:58 Catalin Marinas wrote:
> On Wed, May 11, 2016 at 09:30:07PM +0200, Arnd Bergmann wrote:
> > On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote:
> > 
> > I don't think the shifts are a problem, the main downside would be
> > the limit to 44 bits of file offsets (16TB files), but it's also
> > unclear if that is a practical problem at all. If it is, we run
> > into the same problem on all other 32-bit architectures too.
> 
> I hope people are seriously thinking of moving to an LP64 ABI if they
> have such large file offset needs.

Good point. 44 bits of file size is certainly enough for mmap()
on a 32-bit task: you would only be able to map a very small fraction
of the file anyway, and if you want to map larger files, and should
move to 64-bit tasks long before this becomes a limitation.

	Arnd
diff mbox

Patch

diff --git a/arch/arm/include/uapi/asm/posix_types.h b/arch/arm/include/uapi/asm/posix_types.h
index d2de9cb..f9d065c 100644
--- a/arch/arm/include/uapi/asm/posix_types.h
+++ b/arch/arm/include/uapi/asm/posix_types.h
@@ -32,6 +32,9 @@  typedef unsigned short                __kernel_gid_t;
  typedef unsigned short         __kernel_old_dev_t;
  #define __kernel_old_dev_t __kernel_old_dev_t

+typedef __kernel_long_t        __kernel_off_t;
+#define __kernel_off_t
+
  #include <asm-generic/posix_types.h>

  #endif
```
And We could change the generic posix_types.h a little bit:
```
diff --git a/include/uapi/asm-generic/posix_types.h b/include/uapi/asm-generic/posix_types.h
index fe74fcc..7bbaf04 100644
--- a/include/uapi/asm-generic/posix_types.h
+++ b/include/uapi/asm-generic/posix_types.h
@@ -80,10 +80,13 @@  typedef struct {
  } __kernel_fsid_t;
  #endif

+#ifndef __kernel_off_t
+typedef long long __kernel_off_t;
+#endif
+
  /*
   * anything below here should be completely generic
   */
-typedef __kernel_long_t        __kernel_off_t;
  typedef long long      __kernel_loff_t;
  typedef __kernel_long_t        __kernel_time_t;
  typedef __kernel_long_t        __kernel_clock_t;