Message ID | 1466770980-18933-1-git-send-email-ynorov@caviumnetworks.com |
---|---|
State | New |
Headers | show |
On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that > calculate register pair for off_t like this: > __LONG_LONG_PAIR (offset >> 31, offset) > > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit > off_t will be broken with it. This patch redirects affected syscalls > to their 64-bit versions. It also saves few instructions and symbols > in glibc, as 32-bit syscall wrappers are not generated anymore. If you have 64-bit register, should you use wordsize-64, like sysdeps/unix/sysv/linux/wordsize-64 H.J.
On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote: > On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that > > calculate register pair for off_t like this: > > __LONG_LONG_PAIR (offset >> 31, offset) > > > > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit > > off_t will be broken with it. This patch redirects affected syscalls > > to their 64-bit versions. It also saves few instructions and symbols > > in glibc, as 32-bit syscall wrappers are not generated anymore. > > If you have 64-bit register, should you use wordsize-64, like > > sysdeps/unix/sysv/linux/wordsize-64 > > H.J. Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit parameters as pair. (this is one of two options for ILP32 that is under discussion)
On Fri, Jun 24, 2016 at 5:41 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote: >> On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: >> > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that >> > calculate register pair for off_t like this: >> > __LONG_LONG_PAIR (offset >> 31, offset) >> > >> > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit >> > off_t will be broken with it. This patch redirects affected syscalls >> > to their 64-bit versions. It also saves few instructions and symbols >> > in glibc, as 32-bit syscall wrappers are not generated anymore. >> >> If you have 64-bit register, should you use wordsize-64, like >> >> sysdeps/unix/sysv/linux/wordsize-64 >> >> H.J. > > Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit > parameters as pair. (this is one of two options for ILP32 that is > under discussion) You should still use wordsize-64 and make special exceptions if needed.
On Friday, June 24, 2016 5:57:26 AM CEST H.J. Lu wrote: > On Fri, Jun 24, 2016 at 5:41 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > > On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote: > >> On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > >> > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that > >> > calculate register pair for off_t like this: > >> > __LONG_LONG_PAIR (offset >> 31, offset) > >> > > >> > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit > >> > off_t will be broken with it. This patch redirects affected syscalls > >> > to their 64-bit versions. It also saves few instructions and symbols > >> > in glibc, as 32-bit syscall wrappers are not generated anymore. > >> > >> If you have 64-bit register, should you use wordsize-64, like > >> > >> sysdeps/unix/sysv/linux/wordsize-64 > >> > >> H.J. > > > > Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit > > parameters as pair. (this is one of two options for ILP32 that is > > under discussion) > > You should still use wordsize-64 and make special exceptions if needed. Can the syscall ABI be a single exception then? I think at this point the syscall interface for aarch64 ILP32 is exactly the same as for 32-bit RISC-V. I guess it makes sense to use sysdeps/wordsize-64/ and sysdeps/ieee754/dbl-64/wordsize-64/, but the sysdeps/unix/sysv/linux/wordsize-64/ directory seems to only contain files for syscalls that differ between 32-bit and 64-bit architectures, so each one of them would otherwise need a separate override that redirects to the normal 32-bit syscall. Arnd
On Fri, Jun 24, 2016 at 03:15:50PM +0200, Arnd Bergmann wrote: > On Friday, June 24, 2016 5:57:26 AM CEST H.J. Lu wrote: > > On Fri, Jun 24, 2016 at 5:41 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > > > On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote: > > >> On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > > >> > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that > > >> > calculate register pair for off_t like this: > > >> > __LONG_LONG_PAIR (offset >> 31, offset) > > >> > > > >> > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit > > >> > off_t will be broken with it. This patch redirects affected syscalls > > >> > to their 64-bit versions. It also saves few instructions and symbols > > >> > in glibc, as 32-bit syscall wrappers are not generated anymore. > > >> > > >> If you have 64-bit register, should you use wordsize-64, like > > >> > > >> sysdeps/unix/sysv/linux/wordsize-64 > > >> > > >> H.J. > > > > > > Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit > > > parameters as pair. (this is one of two options for ILP32 that is > > > under discussion) > > > > You should still use wordsize-64 and make special exceptions if needed. > > Can the syscall ABI be a single exception then? I think at this > point the syscall interface for aarch64 ILP32 is exactly the same > as for 32-bit RISC-V. > > I guess it makes sense to use sysdeps/wordsize-64/ and > sysdeps/ieee754/dbl-64/wordsize-64/, but the > sysdeps/unix/sysv/linux/wordsize-64/ directory seems to only > contain files for syscalls that differ between 32-bit and > 64-bit architectures, so each one of them would otherwise > need a separate override that redirects to the normal 32-bit > syscall. > > Arnd > Hi Arnd, H.J. Lu, others, I'm not so experienced in the glibc, and it seems I lost your point. The whole idea of ILP32 patchset is to be a counterpart for kernel code that clears top halves of registers unconditionally at now. It means we cannot pass any 64-bit value in a single register, and that's what the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't understand how we can use it. Regarding this patch. As far as I understand, each ABI can define size of it's types with no relation to register size. And modern 32-bit ABIs should have off_t, ino_t etc 64-bit. So we have off_t 64-bit but pass it in a pair. That's what the code under sysdeps/unix/sysv/linux/ does, except that it does not do it right. I didn't find the limitation on sysdeps/unix/sysv/linux/ to have off_t exactly 32-bit, and it means it should work correct for both cases. And therefore my patch fixes real bug. In other hand, if sysdeps/unix/sysv/linux/ should work with 32-bit off_t only, I suggest to describe it explicitly with code like this: #ifdef __OFF_T_MATCHES_OFF64_T # error off_t is 32-bit only #endif where needed. But then I'll still have to use sysdeps/unix/sysv/linux/ for ILP32, and will redirect off_t-related syscalls in platform code. Yury
On Monday, June 27, 2016 9:26:46 AM CEST Yury Norov wrote: > On Fri, Jun 24, 2016 at 03:15:50PM +0200, Arnd Bergmann wrote: > > On Friday, June 24, 2016 5:57:26 AM CEST H.J. Lu wrote: > > > On Fri, Jun 24, 2016 at 5:41 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > > > > On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote: > > > >> On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: > > > >> > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that > > > >> > calculate register pair for off_t like this: > > > >> > __LONG_LONG_PAIR (offset >> 31, offset) > > > >> > > > > >> > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit > > > >> > off_t will be broken with it. This patch redirects affected syscalls > > > >> > to their 64-bit versions. It also saves few instructions and symbols > > > >> > in glibc, as 32-bit syscall wrappers are not generated anymore. > > > >> > > > >> If you have 64-bit register, should you use wordsize-64, like > > > >> > > > >> sysdeps/unix/sysv/linux/wordsize-64 > > > >> > > > >> H.J. > > > > > > > > Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit > > > > parameters as pair. (this is one of two options for ILP32 that is > > > > under discussion) > > > > > > You should still use wordsize-64 and make special exceptions if needed. > > > > Can the syscall ABI be a single exception then? I think at this > > point the syscall interface for aarch64 ILP32 is exactly the same > > as for 32-bit RISC-V. > > > > I guess it makes sense to use sysdeps/wordsize-64/ and > > sysdeps/ieee754/dbl-64/wordsize-64/, but the > > sysdeps/unix/sysv/linux/wordsize-64/ directory seems to only > > contain files for syscalls that differ between 32-bit and > > 64-bit architectures, so each one of them would otherwise > > need a separate override that redirects to the normal 32-bit > > syscall. > > > Hi Arnd, H.J. Lu, others, > > I'm not so experienced in the glibc, and it seems I lost your point. I'm also not very experienced in glibc, it's possible that I'm the one who's confused > The whole idea of ILP32 patchset is to be a counterpart for kernel code > that clears top halves of registers unconditionally at now. It means we > cannot pass any 64-bit value in a single register, and that's what > the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't > understand how we can use it. The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for the 64-bit syscall API, which is not appropriate here as the kernel port uses the 32-bit syscall API, now basically unchanged. This is the same as tile64/ilp32 does, but different from x86-64/ilp32 (x32). > Regarding this patch. As far as I understand, each ABI can define size > of it's types with no relation to register size. And modern > 32-bit ABIs should have off_t, ino_t etc 64-bit. So we have off_t > 64-bit but pass it in a pair. That's what the code under > sysdeps/unix/sysv/linux/ does, except that it does not do it right. > > I didn't find the limitation on sysdeps/unix/sysv/linux/ to have off_t > exactly 32-bit, and it means it should work correct for both cases. > And therefore my patch fixes real bug. > > In other hand, if sysdeps/unix/sysv/linux/ should work with 32-bit > off_t only, I suggest to describe it explicitly with code like this: > > #ifdef __OFF_T_MATCHES_OFF64_T > # error off_t is 32-bit only > #endif > > where needed. But then I'll still have to use sysdeps/unix/sysv/linux/ > for ILP32, and will redirect off_t-related syscalls in platform code. I agree your patch looks fine, and it fixes the problem for 32-bit RISC-V as well. Redirecting off_t based syscalls to architecture specific code sounds wrong: We should do the same thing for aarch64/ilp32 and 32-bit risc-v, whatever we end up doing. The alternative that I think Mike Frysinger was hinting at would be to leave off_t as 32-bit even on aarch64/ilp32, but then just not use it. If you build your compiler to always pass _FILE_OFFSET_BITS=64, you can leave this part of glibc completely untouched and will get the symbols for handling 32-bit off_t, but all applications built against the libc will use 64-bit off_t anyway. To me, that sounds like a bigger hack for the whole system, but it makes the glibc platform specific code a bit simpler because we avoid the special case. Arnd
On 27/06/2016 06:08, Arnd Bergmann wrote: > On Monday, June 27, 2016 9:26:46 AM CEST Yury Norov wrote: >> On Fri, Jun 24, 2016 at 03:15:50PM +0200, Arnd Bergmann wrote: >>> On Friday, June 24, 2016 5:57:26 AM CEST H.J. Lu wrote: >>>> On Fri, Jun 24, 2016 at 5:41 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: >>>>> On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote: >>>>>> On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote: >>>>>>> There are 3 syscall wrappers under sysdeps/unix/sysv/linux that >>>>>>> calculate register pair for off_t like this: >>>>>>> __LONG_LONG_PAIR (offset >> 31, offset) >>>>>>> >>>>>>> While it works for 32-bit off_t, new 32-bit APIs that use 64-bit >>>>>>> off_t will be broken with it. This patch redirects affected syscalls >>>>>>> to their 64-bit versions. It also saves few instructions and symbols >>>>>>> in glibc, as 32-bit syscall wrappers are not generated anymore. >>>>>> >>>>>> If you have 64-bit register, should you use wordsize-64, like >>>>>> >>>>>> sysdeps/unix/sysv/linux/wordsize-64 >>>>>> >>>>>> H.J. >>>>> >>>>> Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit >>>>> parameters as pair. (this is one of two options for ILP32 that is >>>>> under discussion) >>>> >>>> You should still use wordsize-64 and make special exceptions if needed. >>> >>> Can the syscall ABI be a single exception then? I think at this >>> point the syscall interface for aarch64 ILP32 is exactly the same >>> as for 32-bit RISC-V. >>> >>> I guess it makes sense to use sysdeps/wordsize-64/ and >>> sysdeps/ieee754/dbl-64/wordsize-64/, but the >>> sysdeps/unix/sysv/linux/wordsize-64/ directory seems to only >>> contain files for syscalls that differ between 32-bit and >>> 64-bit architectures, so each one of them would otherwise >>> need a separate override that redirects to the normal 32-bit >>> syscall. >>> >> Hi Arnd, H.J. Lu, others, >> >> I'm not so experienced in the glibc, and it seems I lost your point. > > I'm also not very experienced in glibc, it's possible that I'm the > one who's confused > >> The whole idea of ILP32 patchset is to be a counterpart for kernel code >> that clears top halves of registers unconditionally at now. It means we >> cannot pass any 64-bit value in a single register, and that's what >> the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't >> understand how we can use it. > > The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for > the 64-bit syscall API, which is not appropriate here as the kernel > port uses the 32-bit syscall API, now basically unchanged. > > This is the same as tile64/ilp32 does, but different from x86-64/ilp32 > (x32). I intend to send a patch upstream to consolidate all the fallocate implementation to help this very issue. The idea is to use the same pread consolidate idea: 1. Each architecture/ABI defines if its a ILP32 (__ASSUME_WORDSIZE64_ILP32) and if off64_t differs in size of off_t (__ASSUME_OFF_DIFF_OFF64). Currently, x32 defines __ASSUME_WORDSIZE64_ILP32 and only MIPS64-n32 defines both __ASSUME_WORDSIZE64_ILP32 and __ASSUME_OFF_DIFF_OFF64. 2. For the default function implementation (without the 64 suffix) the symbol will be built if is 32-bits (__WORDSIZE==64) or if off_t differs in size from off64_t (__ASSUME_OFF_DIFF_OFF64). It means that for architecture that only pass 64-bit off_t this symbol won't be build. 3. The 64 variant of the function implementation (with the 64 suffix) will be always build and a weak alias for the non-suffix variant will be created if __WORDSIZE == 64 and if size of off64_t differs from off_t. It means that for architecture that only pass 64-bits off_t function will be an alias to function64. I think with this patch there is no need to more arch-specific implementation. > >> Regarding this patch. As far as I understand, each ABI can define size >> of it's types with no relation to register size. And modern >> 32-bit ABIs should have off_t, ino_t etc 64-bit. So we have off_t >> 64-bit but pass it in a pair. That's what the code under >> sysdeps/unix/sysv/linux/ does, except that it does not do it right. >> >> I didn't find the limitation on sysdeps/unix/sysv/linux/ to have off_t >> exactly 32-bit, and it means it should work correct for both cases. >> And therefore my patch fixes real bug. >> >> In other hand, if sysdeps/unix/sysv/linux/ should work with 32-bit >> off_t only, I suggest to describe it explicitly with code like this: >> >> #ifdef __OFF_T_MATCHES_OFF64_T >> # error off_t is 32-bit only >> #endif >> >> where needed. But then I'll still have to use sysdeps/unix/sysv/linux/ >> for ILP32, and will redirect off_t-related syscalls in platform code. > > I agree your patch looks fine, and it fixes the problem for 32-bit > RISC-V as well. I think this patch is incomplete: it should use the SYSCALL_LL{64} macros for passing off_t{64} arguments which is being used in p{read,write}. I will send a consolidation patch today. > > Redirecting off_t based syscalls to architecture specific code sounds > wrong: We should do the same thing for aarch64/ilp32 and 32-bit > risc-v, whatever we end up doing. The alternative that I think > Mike Frysinger was hinting at would be to leave off_t as 32-bit even > on aarch64/ilp32, but then just not use it. If you build your compiler > to always pass _FILE_OFFSET_BITS=64, you can leave this part of > glibc completely untouched and will get the symbols for handling > 32-bit off_t, but all applications built against the libc will > use 64-bit off_t anyway. To me, that sounds like a bigger hack > for the whole system, but it makes the glibc platform specific code > a bit simpler because we avoid the special case. > > Arnd >
On Monday, June 27, 2016 9:11:51 AM CEST Adhemerval Zanella wrote: > > > >> The whole idea of ILP32 patchset is to be a counterpart for kernel code > >> that clears top halves of registers unconditionally at now. It means we > >> cannot pass any 64-bit value in a single register, and that's what > >> the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't > >> understand how we can use it. > > > > The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for > > the 64-bit syscall API, which is not appropriate here as the kernel > > port uses the 32-bit syscall API, now basically unchanged. > > > > This is the same as tile64/ilp32 does, but different from x86-64/ilp32 > > (x32). > > I intend to send a patch upstream to consolidate all the fallocate > implementation to help this very issue. The idea is to use the same > pread consolidate idea: > > 1. Each architecture/ABI defines if its a ILP32 (__ASSUME_WORDSIZE64_ILP32) > and if off64_t differs in size of off_t (__ASSUME_OFF_DIFF_OFF64). > Currently, x32 defines __ASSUME_WORDSIZE64_ILP32 and only MIPS64-n32 > defines both __ASSUME_WORDSIZE64_ILP32 and __ASSUME_OFF_DIFF_OFF64. > > 2. For the default function implementation (without the 64 suffix) > the symbol will be built if is 32-bits (__WORDSIZE==64) or > if off_t differs in size from off64_t (__ASSUME_OFF_DIFF_OFF64). > > It means that for architecture that only pass 64-bit off_t this > symbol won't be build. > > 3. The 64 variant of the function implementation (with the 64 suffix) > will be always build and a weak alias for the non-suffix variant > will be created if __WORDSIZE == 64 and if size of off64_t differs > from off_t. > > It means that for architecture that only pass 64-bits off_t > function will be an alias to function64. > > I think with this patch there is no need to more arch-specific implementation. Doesn't that assume that the kernel interface uses 64-bit registers to pass off_t? Yury's patch was specifically for the case where you use two 32-bit registers (or two lower halves of 64-bit registers in case of aarch64) but still want 64-bit off_t by default, i.e. (!defined(__ASSUME_WORDSIZE64_ILP32) && !defined(__ASSUME_OFF_DIFF_OFF64)). Arnd
On 27/06/2016 18:01, Arnd Bergmann wrote: > On Monday, June 27, 2016 9:11:51 AM CEST Adhemerval Zanella wrote: >>> >>>> The whole idea of ILP32 patchset is to be a counterpart for kernel code >>>> that clears top halves of registers unconditionally at now. It means we >>>> cannot pass any 64-bit value in a single register, and that's what >>>> the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't >>>> understand how we can use it. >>> >>> The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for >>> the 64-bit syscall API, which is not appropriate here as the kernel >>> port uses the 32-bit syscall API, now basically unchanged. >>> >>> This is the same as tile64/ilp32 does, but different from x86-64/ilp32 >>> (x32). >> >> I intend to send a patch upstream to consolidate all the fallocate >> implementation to help this very issue. The idea is to use the same >> pread consolidate idea: >> >> 1. Each architecture/ABI defines if its a ILP32 (__ASSUME_WORDSIZE64_ILP32) >> and if off64_t differs in size of off_t (__ASSUME_OFF_DIFF_OFF64). >> Currently, x32 defines __ASSUME_WORDSIZE64_ILP32 and only MIPS64-n32 >> defines both __ASSUME_WORDSIZE64_ILP32 and __ASSUME_OFF_DIFF_OFF64. >> >> 2. For the default function implementation (without the 64 suffix) >> the symbol will be built if is 32-bits (__WORDSIZE==64) or >> if off_t differs in size from off64_t (__ASSUME_OFF_DIFF_OFF64). >> >> It means that for architecture that only pass 64-bit off_t this >> symbol won't be build. >> >> 3. The 64 variant of the function implementation (with the 64 suffix) >> will be always build and a weak alias for the non-suffix variant >> will be created if __WORDSIZE == 64 and if size of off64_t differs >> from off_t. >> >> It means that for architecture that only pass 64-bits off_t >> function will be an alias to function64. >> >> I think with this patch there is no need to more arch-specific implementation. > > Doesn't that assume that the kernel interface uses 64-bit registers > to pass off_t? Yury's patch was specifically for the case where > you use two 32-bit registers (or two lower halves of 64-bit registers > in case of aarch64) but still want 64-bit off_t by default, > i.e. (!defined(__ASSUME_WORDSIZE64_ILP32) && !defined(__ASSUME_OFF_DIFF_OFF64)). > > Arnd > So if I understood correctly AArch64/ILP32 will another way to handle off_t/off64_t, which indeed the original patch make sense.
On Mon, Jun 27, 2016 at 08:00:41PM -0300, Adhemerval Zanella wrote: > > > On 27/06/2016 18:01, Arnd Bergmann wrote: > > On Monday, June 27, 2016 9:11:51 AM CEST Adhemerval Zanella wrote: > >>> > >>>> The whole idea of ILP32 patchset is to be a counterpart for kernel code > >>>> that clears top halves of registers unconditionally at now. It means we > >>>> cannot pass any 64-bit value in a single register, and that's what > >>>> the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't > >>>> understand how we can use it. > >>> > >>> The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for > >>> the 64-bit syscall API, which is not appropriate here as the kernel > >>> port uses the 32-bit syscall API, now basically unchanged. > >>> > >>> This is the same as tile64/ilp32 does, but different from x86-64/ilp32 > >>> (x32). > >> > >> I intend to send a patch upstream to consolidate all the fallocate > >> implementation to help this very issue. The idea is to use the same > >> pread consolidate idea: > >> > >> 1. Each architecture/ABI defines if its a ILP32 (__ASSUME_WORDSIZE64_ILP32) > >> and if off64_t differs in size of off_t (__ASSUME_OFF_DIFF_OFF64). > >> Currently, x32 defines __ASSUME_WORDSIZE64_ILP32 and only MIPS64-n32 > >> defines both __ASSUME_WORDSIZE64_ILP32 and __ASSUME_OFF_DIFF_OFF64. > >> > >> 2. For the default function implementation (without the 64 suffix) > >> the symbol will be built if is 32-bits (__WORDSIZE==64) or > >> if off_t differs in size from off64_t (__ASSUME_OFF_DIFF_OFF64). > >> > >> It means that for architecture that only pass 64-bit off_t this > >> symbol won't be build. > >> > >> 3. The 64 variant of the function implementation (with the 64 suffix) > >> will be always build and a weak alias for the non-suffix variant > >> will be created if __WORDSIZE == 64 and if size of off64_t differs > >> from off_t. > >> > >> It means that for architecture that only pass 64-bits off_t > >> function will be an alias to function64. > >> > >> I think with this patch there is no need to more arch-specific implementation. > > > > Doesn't that assume that the kernel interface uses 64-bit registers > > to pass off_t? Yury's patch was specifically for the case where > > you use two 32-bit registers (or two lower halves of 64-bit registers > > in case of aarch64) but still want 64-bit off_t by default, > > i.e. (!defined(__ASSUME_WORDSIZE64_ILP32) && !defined(__ASSUME_OFF_DIFF_OFF64)). > > > > Arnd > > > > So if I understood correctly AArch64/ILP32 will another way to handle > off_t/off64_t, which indeed the original patch make sense. Yes. You understood correctly. This is how RISC-V works, and this is default behavior for all new 32-bit ABIs, both native and ILP32. Is my understanding correct that for AARCH64/ILP32, in glibc terms we have solid understanding that __ASSUME_OFF_DIFF_OFF64 is disabled; and__ASSUME_WORDSIZE64_ILP32 depends on kernel wrappers, and under discussion. Could you (someone) explain me the difference between __ASSUME_OFF_DIFF_OFF64 and __OFF_T_MATCHES_OFF64_T? At first glance they are mutual exclusive, and so we can drop one of them. Yury
On 28/06/2016 00:44, Yury Norov wrote: > On Mon, Jun 27, 2016 at 08:00:41PM -0300, Adhemerval Zanella wrote: >> >> >> On 27/06/2016 18:01, Arnd Bergmann wrote: >>> On Monday, June 27, 2016 9:11:51 AM CEST Adhemerval Zanella wrote: >>>>> >>>>>> The whole idea of ILP32 patchset is to be a counterpart for kernel code >>>>>> that clears top halves of registers unconditionally at now. It means we >>>>>> cannot pass any 64-bit value in a single register, and that's what >>>>>> the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't >>>>>> understand how we can use it. >>>>> >>>>> The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for >>>>> the 64-bit syscall API, which is not appropriate here as the kernel >>>>> port uses the 32-bit syscall API, now basically unchanged. >>>>> >>>>> This is the same as tile64/ilp32 does, but different from x86-64/ilp32 >>>>> (x32). >>>> >>>> I intend to send a patch upstream to consolidate all the fallocate >>>> implementation to help this very issue. The idea is to use the same >>>> pread consolidate idea: >>>> >>>> 1. Each architecture/ABI defines if its a ILP32 (__ASSUME_WORDSIZE64_ILP32) >>>> and if off64_t differs in size of off_t (__ASSUME_OFF_DIFF_OFF64). >>>> Currently, x32 defines __ASSUME_WORDSIZE64_ILP32 and only MIPS64-n32 >>>> defines both __ASSUME_WORDSIZE64_ILP32 and __ASSUME_OFF_DIFF_OFF64. >>>> >>>> 2. For the default function implementation (without the 64 suffix) >>>> the symbol will be built if is 32-bits (__WORDSIZE==64) or >>>> if off_t differs in size from off64_t (__ASSUME_OFF_DIFF_OFF64). >>>> >>>> It means that for architecture that only pass 64-bit off_t this >>>> symbol won't be build. >>>> >>>> 3. The 64 variant of the function implementation (with the 64 suffix) >>>> will be always build and a weak alias for the non-suffix variant >>>> will be created if __WORDSIZE == 64 and if size of off64_t differs >>>> from off_t. >>>> >>>> It means that for architecture that only pass 64-bits off_t >>>> function will be an alias to function64. >>>> >>>> I think with this patch there is no need to more arch-specific implementation. >>> >>> Doesn't that assume that the kernel interface uses 64-bit registers >>> to pass off_t? Yury's patch was specifically for the case where >>> you use two 32-bit registers (or two lower halves of 64-bit registers >>> in case of aarch64) but still want 64-bit off_t by default, >>> i.e. (!defined(__ASSUME_WORDSIZE64_ILP32) && !defined(__ASSUME_OFF_DIFF_OFF64)). >>> >>> Arnd >>> >> >> So if I understood correctly AArch64/ILP32 will another way to handle >> off_t/off64_t, which indeed the original patch make sense. > > Yes. You understood correctly. This is how RISC-V works, and this is > default behavior for all new 32-bit ABIs, both native and ILP32. > > Is my understanding correct that for AARCH64/ILP32, in glibc terms > we have solid understanding that __ASSUME_OFF_DIFF_OFF64 is disabled; > and__ASSUME_WORDSIZE64_ILP32 depends on kernel wrappers, and under > discussion. > > Could you (someone) explain me the difference between > __ASSUME_OFF_DIFF_OFF64 and __OFF_T_MATCHES_OFF64_T? > At first glance they are mutual exclusive, and so we > can drop one of them. I see they are essentially the same and I added __ASSUME_OFF_DIFF_OFF64 on my pread consolidation by two main reasons: 1. It is defined only internally on GLIBC 2. By default on 32 bits it assumes off_t differs from off64_t and for 64 bits it is the contrary. On current GLIBC supported architectures it requires only MIPS64-n32 to define it. However since current kernel new port approach is make size of off_t the same as off64_t I see that all new ports will require to define it. I do not have a strong opinion which is the better approach, the only nit is I think it should not be on an installed header.
diff --git a/sysdeps/unix/sysv/linux/fallocate.c b/sysdeps/unix/sysv/linux/fallocate.c index 6a58a5f..4ec55a5 100644 --- a/sysdeps/unix/sysv/linux/fallocate.c +++ b/sysdeps/unix/sysv/linux/fallocate.c @@ -20,6 +20,8 @@ #include <sysdep-cancel.h> +#ifndef __OFF_T_MATCHES_OFF64_T + /* Reserve storage for the data of the file associated with FD. */ int fallocate (int fd, int mode, __off_t offset, __off_t len) @@ -33,3 +35,5 @@ fallocate (int fd, int mode, __off_t offset, __off_t len) return -1; #endif } + +#endif /* __OFF_T_MATCHES_OFF64_T */ diff --git a/sysdeps/unix/sysv/linux/fallocate64.c b/sysdeps/unix/sysv/linux/fallocate64.c index 8e76d6f..f4f73d5 100644 --- a/sysdeps/unix/sysv/linux/fallocate64.c +++ b/sysdeps/unix/sysv/linux/fallocate64.c @@ -35,3 +35,7 @@ fallocate64 (int fd, int mode, __off64_t offset, __off64_t len) return -1; #endif } + +#ifdef __OFF_T_MATCHES_OFF64_T +weak_alias(fallocate64, fallocate) +#endif diff --git a/sysdeps/unix/sysv/linux/posix_fadvise.c b/sysdeps/unix/sysv/linux/posix_fadvise.c index 093d707..8356bc7 100644 --- a/sysdeps/unix/sysv/linux/posix_fadvise.c +++ b/sysdeps/unix/sysv/linux/posix_fadvise.c @@ -19,6 +19,8 @@ #include <fcntl.h> #include <sysdep.h> +#ifndef __OFF_T_MATCHES_OFF64_T + /* Advice the system about the expected behaviour of the application with respect to the file associated with FD. */ @@ -46,3 +48,5 @@ posix_fadvise (int fd, off_t offset, off_t len, int advise) return ENOSYS; #endif } + +#endif /* __OFF_T_MATCHES_OFF64_T */ diff --git a/sysdeps/unix/sysv/linux/posix_fadvise64.c b/sysdeps/unix/sysv/linux/posix_fadvise64.c index 6d10558..c76d52f 100644 --- a/sysdeps/unix/sysv/linux/posix_fadvise64.c +++ b/sysdeps/unix/sysv/linux/posix_fadvise64.c @@ -56,3 +56,7 @@ compat_symbol (libc, __posix_fadvise64_l32, posix_fadvise64, GLIBC_2_2); #else strong_alias (__posix_fadvise64_l64, posix_fadvise64); #endif + +#ifdef __OFF_T_MATCHES_OFF64_T +weak_alias(__posix_fadvise64_l64, __posix_fadvise) +#endif /* __OFF_T_MATCHES_OFF64_T */ diff --git a/sysdeps/unix/sysv/linux/posix_fallocate.c b/sysdeps/unix/sysv/linux/posix_fallocate.c index fc9ac37..f9ca34b 100644 --- a/sysdeps/unix/sysv/linux/posix_fallocate.c +++ b/sysdeps/unix/sysv/linux/posix_fallocate.c @@ -18,6 +18,8 @@ #include <fcntl.h> #include <sysdep.h> +#ifndef __OFF_T_MATCHES_OFF64_T + #define posix_fallocate static internal_fallocate #include <sysdeps/posix/posix_fallocate.c> #undef posix_fallocate @@ -37,3 +39,5 @@ posix_fallocate (int fd, __off_t offset, __off_t len) return INTERNAL_SYSCALL_ERRNO (res, err); return internal_fallocate (fd, offset, len); } + +#endif /* __OFF_T_MATCHES_OFF64_T */ diff --git a/sysdeps/unix/sysv/linux/posix_fallocate64.c b/sysdeps/unix/sysv/linux/posix_fallocate64.c index 4a0a722..3a65d35 100644 --- a/sysdeps/unix/sysv/linux/posix_fallocate64.c +++ b/sysdeps/unix/sysv/linux/posix_fallocate64.c @@ -40,3 +40,7 @@ __posix_fallocate64_l64 (int fd, __off64_t offset, __off64_t len) return INTERNAL_SYSCALL_ERRNO (res, err); return internal_fallocate64 (fd, offset, len); } + +#ifdef __OFF_T_MATCHES_OFF64_T +weak_alias(__posix_fallocate64_l64, posix_fallocate) +#endif
There are 3 syscall wrappers under sysdeps/unix/sysv/linux that calculate register pair for off_t like this: __LONG_LONG_PAIR (offset >> 31, offset) While it works for 32-bit off_t, new 32-bit APIs that use 64-bit off_t will be broken with it. This patch redirects affected syscalls to their 64-bit versions. It also saves few instructions and symbols in glibc, as 32-bit syscall wrappers are not generated anymore. Tested on AARCH64/ILP32. Signed-off-by: Yury Norov <ynorov@caviumnetworks.com> --- sysdeps/unix/sysv/linux/fallocate.c | 4 ++++ sysdeps/unix/sysv/linux/fallocate64.c | 4 ++++ sysdeps/unix/sysv/linux/posix_fadvise.c | 4 ++++ sysdeps/unix/sysv/linux/posix_fadvise64.c | 4 ++++ sysdeps/unix/sysv/linux/posix_fallocate.c | 4 ++++ sysdeps/unix/sysv/linux/posix_fallocate64.c | 4 ++++ 6 files changed, 24 insertions(+)