Message ID | 550B95D4.6020405@huawei.com |
---|---|
State | New |
Headers | show |
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. >
--- a/sysdeps/unix/sysv/linux/generic/bits/stat.h +++ b/sysdeps/unix/sysv/linux/generic/bits/stat.h @@ -42,7 +42,10 @@ #if defined __USE_FILE_OFFSET64 # define __field64(type, type64, name) type64 name -#elif __WORDSIZE == 64 +#elif __WORDSIZE == 64 \ + || (defined(__OFF_T_MATCHES_OFF64_T) \ + && defined(__INO_T_MATCHES_INO64_T) \ + && defined (__BLKCNT_T_TYPE_MATCHES_BLKCNT64_T_TYPE)) # define __field64(type, type64, name) type name #elif __BYTE_ORDER == __LITTLE_ENDIAN # define __field64(type, type64, name) \