Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/27/2014 12:11 PM, Andy Lutomirski wrote: > > Hmm. This sort of goes against existing x86_32 practice where, > AFAICT, things that need a particular calling convention specify > asmlinkage and everything else uses regparm(3) if config/kbuild thinks > it's appropriate. > That is not really true for things that aren't part of the kernel image proper (e.g. the real mode code and so on.) This is a very special case. > But I'm happy to resubmit the patch if you prefer the CFLAGS approach > for the 32-bit vdso. I don't think anything will break, since I don't > think that the 32-bit vdso has any other exported C code. I think it is the better way to go. >> It isn't any faster if the C library has to provide a wrapper just to >> marshal parameters. > > Probably true, given that the glibc wrapper could, in principle, use > an optimized tail call. Also, I see no reason why vdso functions, > alone of all userspace code, should be special. Yes, let's stick to the standard ABI. The syscall entry point was very special. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On Wed, Feb 26, 2014 at 9:22 PM, H. Peter Anvin wrote: > On 02/26/2014 09:19 PM, Andy Lutomirski wrote: >>> >>> The normal ABI almost certainly makes more sense; as such -mregparm=3 is >>> probably not what we want, and I suspect it makes more sense to just >>> drop that from the CFLAGS line? >> >> Hmm. What happens on a native 32-bit build? IIRC the whole kernel is >> build with regparm(3). >> > > Well, the vdso is still built separately, so we can use different CFLAGS > if we want to. > >> If we want to save a cycle or two, then regparm(3) is probably faster. >> But I think that these functions should either be asmlinkage or (on >> 32 bit builds) explicitly regparm(3) to avoid confusion. > > I suggest using the standard ABI, but I suggest doing it via CFLAGS. Hmm. This sort of goes against existing x86_32 practice where, AFAICT, things that need a particular calling convention specify asmlinkage and everything else uses regparm(3) if config/kbuild thinks it's appropriate. But I'm happy to resubmit the patch if you prefer the CFLAGS approach for the 32-bit vdso. I don't think anything will break, since I don't think that the 32-bit vdso has any other exported C code. > > It isn't any faster if the C library has to provide a wrapper just to > marshal parameters. Probably true, given that the glibc wrapper could, in principle, use an optimized tail call. Also, I see no reason why vdso functions, alone of all userspace code, should be special. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On Wed, Feb 26, 2014 at 9:22 PM, H. Peter Anvin h...@zytor.com wrote: On 02/26/2014 09:19 PM, Andy Lutomirski wrote: The normal ABI almost certainly makes more sense; as such -mregparm=3 is probably not what we want, and I suspect it makes more sense to just drop that from the CFLAGS line? Hmm. What happens on a native 32-bit build? IIRC the whole kernel is build with regparm(3). Well, the vdso is still built separately, so we can use different CFLAGS if we want to. If we want to save a cycle or two, then regparm(3) is probably faster. But I think that these functions should either be asmlinkage or (on 32 bit builds) explicitly regparm(3) to avoid confusion. I suggest using the standard ABI, but I suggest doing it via CFLAGS. Hmm. This sort of goes against existing x86_32 practice where, AFAICT, things that need a particular calling convention specify asmlinkage and everything else uses regparm(3) if config/kbuild thinks it's appropriate. But I'm happy to resubmit the patch if you prefer the CFLAGS approach for the 32-bit vdso. I don't think anything will break, since I don't think that the 32-bit vdso has any other exported C code. It isn't any faster if the C library has to provide a wrapper just to marshal parameters. Probably true, given that the glibc wrapper could, in principle, use an optimized tail call. Also, I see no reason why vdso functions, alone of all userspace code, should be special. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/27/2014 12:11 PM, Andy Lutomirski wrote: Hmm. This sort of goes against existing x86_32 practice where, AFAICT, things that need a particular calling convention specify asmlinkage and everything else uses regparm(3) if config/kbuild thinks it's appropriate. That is not really true for things that aren't part of the kernel image proper (e.g. the real mode code and so on.) This is a very special case. But I'm happy to resubmit the patch if you prefer the CFLAGS approach for the 32-bit vdso. I don't think anything will break, since I don't think that the 32-bit vdso has any other exported C code. I think it is the better way to go. It isn't any faster if the C library has to provide a wrapper just to marshal parameters. Probably true, given that the glibc wrapper could, in principle, use an optimized tail call. Also, I see no reason why vdso functions, alone of all userspace code, should be special. Yes, let's stick to the standard ABI. The syscall entry point was very special. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 09:19 PM, Andy Lutomirski wrote: >> >> The normal ABI almost certainly makes more sense; as such -mregparm=3 is >> probably not what we want, and I suspect it makes more sense to just >> drop that from the CFLAGS line? > > Hmm. What happens on a native 32-bit build? IIRC the whole kernel is > build with regparm(3). > Well, the vdso is still built separately, so we can use different CFLAGS if we want to. > If we want to save a cycle or two, then regparm(3) is probably faster. > But I think that these functions should either be asmlinkage or (on > 32 bit builds) explicitly regparm(3) to avoid confusion. I suggest using the standard ABI, but I suggest doing it via CFLAGS. It isn't any faster if the C library has to provide a wrapper just to marshal parameters. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On Wed, Feb 26, 2014 at 10:06 PM, H. Peter Anvin wrote: > On 02/26/2014 07:39 PM, Andi Kleen wrote: >> On Wed, Feb 26, 2014 at 05:02:13PM -0800, Andy Lutomirski wrote: >>> This makes no difference for 64-bit, bit it's critical for 32-bit code: >>> these functions are called from outside the kernel, so they need to comply >>> with the ABI. >> >> That's an odd patch. If that was wrong things couldn't have worked at all. >> Probably hidden by inlining? If yes just make it static >> >> Also you would rather need notrace more often. >> > > It has to support *an* ABI... the syscall vdso entry point uses the old > int $0x80 calling convention rather than the normal ABI. It would > depend on the test program and eventual glibc implementation. And sure > enough, the test program has: > > int (*vdso_gettimeofday)(struct timeval *tv, struct timezone *tz) > __attribute__ ((regparm (3))); > int (*vdso_clock_gettime)(clockid_t clk_id, struct timespec *tp) > __attribute__ ((regparm (3))); > time_t (*vdso_time)(time_t *t) __attribute__ ((regparm (3))); > > That being said, since this code is compiled separately, the compiler > flags there determine what actually matters. However, there we have: > > KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic > > The normal ABI almost certainly makes more sense; as such -mregparm=3 is > probably not what we want, and I suspect it makes more sense to just > drop that from the CFLAGS line? Hmm. What happens on a native 32-bit build? IIRC the whole kernel is build with regparm(3). If we want to save a cycle or two, then regparm(3) is probably faster. But I think that these functions should either be asmlinkage or (on 32 bit builds) explicitly regparm(3) to avoid confusion. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 07:39 PM, Andi Kleen wrote: > On Wed, Feb 26, 2014 at 05:02:13PM -0800, Andy Lutomirski wrote: >> This makes no difference for 64-bit, bit it's critical for 32-bit code: >> these functions are called from outside the kernel, so they need to comply >> with the ABI. > > That's an odd patch. If that was wrong things couldn't have worked at all. > Probably hidden by inlining? If yes just make it static > > Also you would rather need notrace more often. > It has to support *an* ABI... the syscall vdso entry point uses the old int $0x80 calling convention rather than the normal ABI. It would depend on the test program and eventual glibc implementation. And sure enough, the test program has: int (*vdso_gettimeofday)(struct timeval *tv, struct timezone *tz) __attribute__ ((regparm (3))); int (*vdso_clock_gettime)(clockid_t clk_id, struct timespec *tp) __attribute__ ((regparm (3))); time_t (*vdso_time)(time_t *t) __attribute__ ((regparm (3))); That being said, since this code is compiled separately, the compiler flags there determine what actually matters. However, there we have: KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic The normal ABI almost certainly makes more sense; as such -mregparm=3 is probably not what we want, and I suspect it makes more sense to just drop that from the CFLAGS line? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 07:39 PM, Andi Kleen wrote: > > Also you would rather need notrace more often. > Again, can be dealt with in CFLAGS, no? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On Wed, Feb 26, 2014 at 05:02:13PM -0800, Andy Lutomirski wrote: > This makes no difference for 64-bit, bit it's critical for 32-bit code: > these functions are called from outside the kernel, so they need to comply > with the ABI. That's an odd patch. If that was wrong things couldn't have worked at all. Probably hidden by inlining? If yes just make it static Also you would rather need notrace more often. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 05:02 PM, Andy Lutomirski wrote: > This makes no difference for 64-bit, bit it's critical for 32-bit code: > these functions are called from outside the kernel, so they need to comply > with the ABI. Or at least with *an* ABI (the i386 syscall vdso uses the syscall convention, not the i386 ABI convention, for example.) However, it probably does makes most sense to just stick with the standard i386 ABI. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 05:02 PM, Andy Lutomirski wrote: This makes no difference for 64-bit, bit it's critical for 32-bit code: these functions are called from outside the kernel, so they need to comply with the ABI. Or at least with *an* ABI (the i386 syscall vdso uses the syscall convention, not the i386 ABI convention, for example.) However, it probably does makes most sense to just stick with the standard i386 ABI. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On Wed, Feb 26, 2014 at 05:02:13PM -0800, Andy Lutomirski wrote: This makes no difference for 64-bit, bit it's critical for 32-bit code: these functions are called from outside the kernel, so they need to comply with the ABI. That's an odd patch. If that was wrong things couldn't have worked at all. Probably hidden by inlining? If yes just make it static Also you would rather need notrace more often. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 07:39 PM, Andi Kleen wrote: Also you would rather need notrace more often. Again, can be dealt with in CFLAGS, no? -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 07:39 PM, Andi Kleen wrote: On Wed, Feb 26, 2014 at 05:02:13PM -0800, Andy Lutomirski wrote: This makes no difference for 64-bit, bit it's critical for 32-bit code: these functions are called from outside the kernel, so they need to comply with the ABI. That's an odd patch. If that was wrong things couldn't have worked at all. Probably hidden by inlining? If yes just make it static Also you would rather need notrace more often. It has to support *an* ABI... the syscall vdso entry point uses the old int $0x80 calling convention rather than the normal ABI. It would depend on the test program and eventual glibc implementation. And sure enough, the test program has: int (*vdso_gettimeofday)(struct timeval *tv, struct timezone *tz) __attribute__ ((regparm (3))); int (*vdso_clock_gettime)(clockid_t clk_id, struct timespec *tp) __attribute__ ((regparm (3))); time_t (*vdso_time)(time_t *t) __attribute__ ((regparm (3))); That being said, since this code is compiled separately, the compiler flags there determine what actually matters. However, there we have: KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic The normal ABI almost certainly makes more sense; as such -mregparm=3 is probably not what we want, and I suspect it makes more sense to just drop that from the CFLAGS line? -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On Wed, Feb 26, 2014 at 10:06 PM, H. Peter Anvin h...@zytor.com wrote: On 02/26/2014 07:39 PM, Andi Kleen wrote: On Wed, Feb 26, 2014 at 05:02:13PM -0800, Andy Lutomirski wrote: This makes no difference for 64-bit, bit it's critical for 32-bit code: these functions are called from outside the kernel, so they need to comply with the ABI. That's an odd patch. If that was wrong things couldn't have worked at all. Probably hidden by inlining? If yes just make it static Also you would rather need notrace more often. It has to support *an* ABI... the syscall vdso entry point uses the old int $0x80 calling convention rather than the normal ABI. It would depend on the test program and eventual glibc implementation. And sure enough, the test program has: int (*vdso_gettimeofday)(struct timeval *tv, struct timezone *tz) __attribute__ ((regparm (3))); int (*vdso_clock_gettime)(clockid_t clk_id, struct timespec *tp) __attribute__ ((regparm (3))); time_t (*vdso_time)(time_t *t) __attribute__ ((regparm (3))); That being said, since this code is compiled separately, the compiler flags there determine what actually matters. However, there we have: KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic The normal ABI almost certainly makes more sense; as such -mregparm=3 is probably not what we want, and I suspect it makes more sense to just drop that from the CFLAGS line? Hmm. What happens on a native 32-bit build? IIRC the whole kernel is build with regparm(3). If we want to save a cycle or two, then regparm(3) is probably faster. But I think that these functions should either be asmlinkage or (on 32 bit builds) explicitly regparm(3) to avoid confusion. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] x86: Mark __vdso entries as asmlinkage
On 02/26/2014 09:19 PM, Andy Lutomirski wrote: The normal ABI almost certainly makes more sense; as such -mregparm=3 is probably not what we want, and I suspect it makes more sense to just drop that from the CFLAGS line? Hmm. What happens on a native 32-bit build? IIRC the whole kernel is build with regparm(3). Well, the vdso is still built separately, so we can use different CFLAGS if we want to. If we want to save a cycle or two, then regparm(3) is probably faster. But I think that these functions should either be asmlinkage or (on 32 bit builds) explicitly regparm(3) to avoid confusion. I suggest using the standard ABI, but I suggest doing it via CFLAGS. It isn't any faster if the C library has to provide a wrapper just to marshal parameters. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
