Message ID | 550B95D4.6020405@huawei.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 28420 invoked by alias); 20 Mar 2015 03:40:29 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 28398 invoked by uid 89); 20 Mar 2015 03:40:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: szxga02-in.huawei.com Message-ID: <550B95D4.6020405@huawei.com> Date: Fri, 20 Mar 2015 11:36:52 +0800 From: Bamvor Jian Zhang <bamvor.zhangjian@huawei.com> User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6 MIME-Version: 1.0 To: Chris Metcalf <cmetcalf@ezchip.com> CC: "Pinski, Andrew" <Andrew.Pinski@caviumnetworks.com>, "libc-alpha@sourceware.org" <libc-alpha@sourceware.org>, "apinski@cavium.com" <apinski@cavium.com>, "yangyingliang@huawei.com" <yangyingliang@huawei.com>, "bintian.wang@huawei.com" <bintian.wang@huawei.com>, "dingtianhong@huawei.com" <dingtianhong@huawei.com>, <hjl.tools@gmail.com> Subject: Re: [RFC PATCH 2/3] ARM64: ILP32: use __fsword_t in generic/bits/statfs.h References: <1426674611-26427-1-git-send-email-bamvor.zhangjian@huawei.com> <1426674611-26427-3-git-send-email-bamvor.zhangjian@huawei.com>, <550AFBE6.1000402@ezchip.com> <405BEB99-764E-474B-A3E0-7225FDE6596C@caviumnetworks.com> <550B007A.9090407@ezchip.com> In-Reply-To: <550B007A.9090407@ezchip.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-CFilter-Loop: Reflected |
Commit Message
Zhang Jian(Bamvor)
March 20, 2015, 3:36 a.m. UTC
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
Comments
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) \