[RFC,2/3] ARM64: ILP32: use __fsword_t in generic/bits/statfs.h

  On 2015/3/20 0:59, Chris Metcalf wrote:
> On 03/19/2015 12:49 PM, Pinski, Andrew wrote:
>>> On Mar 19, 2015, at 9:40 AM, Chris Metcalf<cmetcalf@ezchip.com>  wrote:
>>> >
>>>> >>On 03/18/2015 06:30 AM, Zhang Jian(Bamvor) wrote:
>>>> >>From: Yang Yingliang<yangyingliang@huawei.com>
>>>> >>
>>>> >>Use __fsword_t to make size of struct statfs equal in userspace and kernel.
>>>> >>
>>>> >>Signed-off-by: Yang Yingliang<yangyingliang@huawei.com>
>>>> >>---
>>>> >>  sysdeps/unix/sysv/linux/generic/bits/statfs.h | 24 ++++++++++++------------
>>>> >>  1 file changed, 12 insertions(+), 12 deletions(-)
>>> >
>>> >I don't object to this patch as a cleanup (to match the types used in the base Linux statfs.h), but can you tell me why this makes a difference to you?  On what asm-generic platform does __SWORD_TYPE != __fsword_t ?  My earlier analysis a few minutes ago suggested that was true only for alpha and x32, neither of which use the linux/generic code in glibc.
>> Aarch64:ILP32 will use linux/generic and will have SWORD_TYPE != __fsword_t just like x32. Basically this patch set goes on top of my already submitted patch set.
> 
> OK, thanks.  But presumably this won't work right, because if you don't
> set __USE_FILE_OFFSET64, and __WORDSIZE == 32, you'll end up injecting
> padding fields that don't belong, via the __field64 macro?
In current patch from Andrew, ilp32 share the same type with lp64 for
off_t, ino_t, blkcnt_t. And it share the same "#elif" with "__WORDSIZE == 64":

> This is basically the issue I just raised in the parallel thread with HJ.
It seems that aarch64:ilp32 is different from x32 at this point. In that
thread H.J. Lu said that "Since x32 uses a combination of x86-64 and ia32
syscall interfaces, x32 can't use linux/generic.". Whereas aarch64:ilp32
include "unix/sysv/linux/generic" in its Imples.

regards

bamvor
  

On 03/19/2015 11:36 PM, Bamvor Jian Zhang wrote:
> On 2015/3/20 0:59, Chris Metcalf wrote:
>> >On 03/19/2015 12:49 PM, Pinski, Andrew wrote:
>>>> >>>On Mar 19, 2015, at 9:40 AM, Chris Metcalf<cmetcalf@ezchip.com>   wrote:
>>>>> >>> >
>>>>>>> >>>> >>On 03/18/2015 06:30 AM, Zhang Jian(Bamvor) wrote:
>>>>>>> >>>> >>From: Yang Yingliang<yangyingliang@huawei.com>
>>>>>>> >>>> >>
>>>>>>> >>>> >>Use __fsword_t to make size of struct statfs equal in userspace and kernel.
>>>>>>> >>>> >>
>>>>>>> >>>> >>Signed-off-by: Yang Yingliang<yangyingliang@huawei.com>
>>>>>>> >>>> >>---
>>>>>>> >>>> >>  sysdeps/unix/sysv/linux/generic/bits/statfs.h | 24 ++++++++++++------------
>>>>>>> >>>> >>  1 file changed, 12 insertions(+), 12 deletions(-)
>>>>> >>> >
>>>>> >>> >I don't object to this patch as a cleanup (to match the types used in the base Linux statfs.h), but can you tell me why this makes a difference to you?  On what asm-generic platform does __SWORD_TYPE != __fsword_t ?  My earlier analysis a few minutes ago suggested that was true only for alpha and x32, neither of which use the linux/generic code in glibc.
>>> >>Aarch64:ILP32 will use linux/generic and will have SWORD_TYPE != __fsword_t just like x32. Basically this patch set goes on top of my already submitted patch set.
>> >
>> >OK, thanks.  But presumably this won't work right, because if you don't
>> >set __USE_FILE_OFFSET64, and __WORDSIZE == 32, you'll end up injecting
>> >padding fields that don't belong, via the __field64 macro?
> In current patch from Andrew, ilp32 share the same type with lp64 for
> off_t, ino_t, blkcnt_t. And it share the same "#elif" with "__WORDSIZE == 64"

OK, that makes sense, and in that case the patch looks good to me. I 
assume you are planning to push all this stuff to glibc via Marcus or 
some other ARM maintainer, so I don't need to push this for you. Let me 
know if that's not the case.
  

Hi, Chris

On 2015/3/20 21:36, Chris Metcalf wrote:
> On 03/19/2015 11:36 PM, Bamvor Jian Zhang wrote:
>> On 2015/3/20 0:59, Chris Metcalf wrote:
>>> >On 03/19/2015 12:49 PM, Pinski, Andrew wrote:
>>>>> >>>On Mar 19, 2015, at 9:40 AM, Chris Metcalf<cmetcalf@ezchip.com>   wrote:
>>>>>> >>> >
>>>>>>>> >>>> >>On 03/18/2015 06:30 AM, Zhang Jian(Bamvor) wrote:
>>>>>>>> >>>> >>From: Yang Yingliang<yangyingliang@huawei.com>
>>>>>>>> >>>> >>
>>>>>>>> >>>> >>Use __fsword_t to make size of struct statfs equal in userspace and kernel.
>>>>>>>> >>>> >>
>>>>>>>> >>>> >>Signed-off-by: Yang Yingliang<yangyingliang@huawei.com>
>>>>>>>> >>>> >>---
>>>>>>>> >>>> >>  sysdeps/unix/sysv/linux/generic/bits/statfs.h | 24 ++++++++++++------------
>>>>>>>> >>>> >>  1 file changed, 12 insertions(+), 12 deletions(-)
>>>>>> >>> >
>>>>>> >>> >I don't object to this patch as a cleanup (to match the types used in the base Linux statfs.h), but can you tell me why this makes a difference to you?  On what asm-generic platform does __SWORD_TYPE != __fsword_t ?  My earlier analysis a few minutes ago suggested that was true only for alpha and x32, neither of which use the linux/generic code in glibc.
>>>> >>Aarch64:ILP32 will use linux/generic and will have SWORD_TYPE != __fsword_t just like x32. Basically this patch set goes on top of my already submitted patch set.
>>> >
>>> >OK, thanks.  But presumably this won't work right, because if you don't
>>> >set __USE_FILE_OFFSET64, and __WORDSIZE == 32, you'll end up injecting
>>> >padding fields that don't belong, via the __field64 macro?
> > In current patch from Andrew, ilp32 share the same type with lp64 for
> > off_t, ino_t, blkcnt_t. And it share the same "#elif" with "__WORDSIZE == 64"
> OK, that makes sense, and in that case the patch looks good to me.
Thanks.
> I assume you are planning to push all this stuff to glibc via Marcus or
> some other ARM maintainer, so I don't need to push this for you. Let me
> know if that's not the case.
Well, it sounds that it is not this case. This patch does not depend on the
aarch64 ilp32 patch from Andrew and does not depend on any aarch64 specific
code either. While the other two patches of mine depend on Andrew's ILP32
patches.
It would be great this single patch could be applied first.

regards

bamvor
  

On 03/22/2015 10:38 PM, Bamvor Jian Zhang wrote:
>> I assume you are planning to push all this stuff to glibc via Marcus or
>> >some other ARM maintainer, so I don't need to push this for you. Let me
>> >know if that's not the case.
> Well, it sounds that it is not this case. This patch does not depend on the
> aarch64 ilp32 patch from Andrew and does not depend on any aarch64 specific
> code either. While the other two patches of mine depend on Andrew's ILP32
> patches.
> It would be great this single patch could be applied first.

OK.  Please repost with an appropriate ChangeLog entry and I can
commit for you.  I don't know if you've done the commit paperwork
but in any case I would say this patch counts as legally trivial.

Thanks.
  

On 2015/3/23 23:39, Chris Metcalf wrote:
> On 03/22/2015 10:38 PM, Bamvor Jian Zhang wrote:
>>> I assume you are planning to push all this stuff to glibc via Marcus or
>>> >some other ARM maintainer, so I don't need to push this for you. Let me
>>> >know if that's not the case.
>> Well, it sounds that it is not this case. This patch does not depend on the
>> aarch64 ilp32 patch from Andrew and does not depend on any aarch64 specific
>> code either. While the other two patches of mine depend on Andrew's ILP32
>> patches.
>> It would be great this single patch could be applied first.
> 
> OK.  Please repost with an appropriate ChangeLog entry and I can
> commit for you.  I don't know if you've done the commit paperwork
> but in any case I would say this patch counts as legally trivial.
Thanks. I will add the ChangeLog in the next version.
> Thanks.
>