Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
On 11/21/2013 06:41 PM, Peter Korsgaard wrote: "Khem" == Khem Raj writes: Hi, The code nicely checks for it, but then ends up hard coding the syscall to use __NR_pread64 afterwards, rendering the check useless. Fix it by using the result of the test instead. >> Mike> i think you should look at all the pread/pwrite changes in Mike> master. afaik, all issues are addressed there. >> >> Yes, possible. I'm trying to test the 0.9.33 branch to hopefully speed >> up the 0.9.33.3 release as there's quite some fixes pending, but it >> looks like some more stuff should get backported. >> >> Anybody else testing the branch? > I would be interested if you try out latest master. Sorry for the slow response - I only now found time to do so. I'm happy to say that the pread issue ISN'T present on todays snapshot of master. So these pread/pwrite changes imho should get backported to to the 0.9.33 branch if we ever plan on doing a bugfix release from it. I hit a nasty race condition in git 1.8 because pread/pwrite are not atomic in 0.9.33.2 [1]. I've been meaning to backport Mike's fixes for Gentoo, but really we should push out another release. The linux kernel has provided pread/pwrite for a long time now, no need to simulate them with lseek(). Ref [1] https://bugs.gentoo.org/show_bug.cgi?id=475920 -- Anthony G. Basile, Ph. D. Chair of Information Technology D'Youville College Buffalo, NY 14201 (716) 829-8197 ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Khem" == Khem Raj writes: Hi, The code nicely checks for it, but then ends up hard coding the syscall to use __NR_pread64 afterwards, rendering the check useless. Fix it by using the result of the test instead. >> Mike> i think you should look at all the pread/pwrite changes in Mike> master. afaik, all issues are addressed there. >> >> Yes, possible. I'm trying to test the 0.9.33 branch to hopefully speed >> up the 0.9.33.3 release as there's quite some fixes pending, but it >> looks like some more stuff should get backported. >> >> Anybody else testing the branch? > I would be interested if you try out latest master. Sorry for the slow response - I only now found time to do so. I'm happy to say that the pread issue ISN'T present on todays snapshot of master. So these pread/pwrite changes imho should get backported to to the 0.9.33 branch if we ever plan on doing a bugfix release from it. -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Mike" == Mike Frysinger writes: Hi, > On Tuesday 15 October 2013 16:37:32 Peter Korsgaard wrote: Mike> your e-mail client still sucks btw >> >> Sorry, in what way? > the quoting style unreasonably mangles things The supercite 'name>' thing? I kind of like it, but I've turned it off for @uclibc.org. -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
On Tuesday 15 October 2013 16:37:32 Peter Korsgaard wrote: > Mike> your e-mail client still sucks btw > > Sorry, in what way? the quoting style unreasonably mangles things -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
On Tuesday 15 October 2013 19:04:12 Khem Raj wrote: > On Oct 15, 2013, at 1:41 PM, Peter Korsgaard wrote: > >> "Mike" == Mike Frysinger writes: > > Hi, > > > >>> The code nicely checks for it, but then ends up hard coding the syscall > >>> to use __NR_pread64 afterwards, rendering the check useless. Fix it by > >>> using the result of the test instead. > > > > Mike> i think you should look at all the pread/pwrite changes in > > Mike> master. afaik, all issues are addressed there. > > > > Yes, possible. I'm trying to test the 0.9.33 branch to hopefully speed > > up the 0.9.33.3 release as there's quite some fixes pending, but it > > looks like some more stuff should get backported. > > > > Anybody else testing the branch? > > I would be interested if you try out latest master. if master works, then we can cherry pick back the patches. but i'd like to know before we start that work :). -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
On Oct 15, 2013, at 1:41 PM, Peter Korsgaard wrote: >> "Mike" == Mike Frysinger writes: > > Hi, > >>> The code nicely checks for it, but then ends up hard coding the syscall >>> to use __NR_pread64 afterwards, rendering the check useless. Fix it by >>> using the result of the test instead. > > Mike> i think you should look at all the pread/pwrite changes in > Mike> master. afaik, all issues are addressed there. > > Yes, possible. I'm trying to test the 0.9.33 branch to hopefully speed > up the 0.9.33.3 release as there's quite some fixes pending, but it > looks like some more stuff should get backported. > > Anybody else testing the branch? I would be interested if you try out latest master. > > -- > Bye, Peter Korsgaard > ___ > uClibc mailing list > [email protected] > http://lists.busybox.net/mailman/listinfo/uclibc ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Mike" == Mike Frysinger writes: Hi, >> The code nicely checks for it, but then ends up hard coding the syscall >> to use __NR_pread64 afterwards, rendering the check useless. Fix it by >> using the result of the test instead. Mike> i think you should look at all the pread/pwrite changes in Mike> master. afaik, all issues are addressed there. Yes, possible. I'm trying to test the 0.9.33 branch to hopefully speed up the 0.9.33.3 release as there's quite some fixes pending, but it looks like some more stuff should get backported. Anybody else testing the branch? -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Mike" == Mike Frysinger writes: Hi, >> I noticed another critical issue on ARM EABI. The use of >> __LONG_LONG_PAIR for the offset doesn't take alignment requirement of >> 64bit parameters on EABI into consideration, so the offset is off by one >> register :/ >> >> https://lkml.org/lkml/2006/1/12/175 >> >> How should that be handled? Mike> i introduced __UCLIBC_SYSCALL_ALIGN_64BIT__ to handle this case. Mike> and the pread/pwrite logic takes that into account. do you have Mike> information to indicate it isn't working ? Well, as the subject indicates this is for the 0.9.33 branch, where there isn't any ALIGN_64BIT. I haven't tried master yet (will do now), but it looks like this should get backported before 0.9.33.3. Mike> your e-mail client still sucks btw Sorry, in what way? -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
On Friday 04 October 2013 15:41:20 Peter Korsgaard wrote: > Some archs (avr32 in particular) still doesn't define __NR_pread64, so > we should fall back to __NR_pread if it isn't available. > > The code nicely checks for it, but then ends up hard coding the syscall > to use __NR_pread64 afterwards, rendering the check useless. Fix it by > using the result of the test instead. i think you should look at all the pread/pwrite changes in master. afaik, all issues are addressed there. -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
On Friday 04 October 2013 17:45:20 Peter Korsgaard wrote: > > "Peter" == Peter Korsgaard writes: > Peter> Some archs (avr32 in particular) still doesn't define __NR_pread64, > so Peter> we should fall back to __NR_pread if it isn't available. > > Peter> The code nicely checks for it, but then ends up hard coding the > syscall Peter> to use __NR_pread64 afterwards, rendering the check > useless. Fix it by Peter> using the result of the test instead. > > I noticed another critical issue on ARM EABI. The use of > __LONG_LONG_PAIR for the offset doesn't take alignment requirement of > 64bit parameters on EABI into consideration, so the offset is off by one > register :/ > > https://lkml.org/lkml/2006/1/12/175 > > How should that be handled? i introduced __UCLIBC_SYSCALL_ALIGN_64BIT__ to handle this case. and the pread/pwrite logic takes that into account. do you have information to indicate it isn't working ? your e-mail client still sucks btw -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Peter" == Peter Korsgaard writes: Hi, Peter> Some archs (avr32 in particular) still doesn't define __NR_pread64, so Peter> we should fall back to __NR_pread if it isn't available. Peter> The code nicely checks for it, but then ends up hard coding the syscall Peter> to use __NR_pread64 afterwards, rendering the check useless. Fix it by Peter> using the result of the test instead. Peter> I noticed another critical issue on ARM EABI. The use of Peter> __LONG_LONG_PAIR for the offset doesn't take alignment requirement of Peter> 64bit parameters on EABI into consideration, so the offset is off by one Peter> register :/ Peter> https://lkml.org/lkml/2006/1/12/175 Peter> How should that be handled? Anybody? -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Peter" == Peter Korsgaard writes: Peter> Some archs (avr32 in particular) still doesn't define __NR_pread64, so Peter> we should fall back to __NR_pread if it isn't available. Peter> The code nicely checks for it, but then ends up hard coding the syscall Peter> to use __NR_pread64 afterwards, rendering the check useless. Fix it by Peter> using the result of the test instead. Peter> Signed-off-by: Peter Korsgaard Peter> --- Peter> Noticed when adding the pending patches for 0.9.33.3 to Buildroot: Peter> http://jenkins.free-electrons.com/job/buildroot/config=atstk100x_defconfig/116/console Ping? Peter> libc/sysdeps/linux/common/pread_write.c |2 +- Peter> 1 file changed, 1 insertion(+), 1 deletion(-) Peter> diff --git a/libc/sysdeps/linux/common/pread_write.c b/libc/sysdeps/linux/common/pread_write.c Peter> index b13de66..8562ab4 100644 Peter> --- a/libc/sysdeps/linux/common/pread_write.c Peter> +++ b/libc/sysdeps/linux/common/pread_write.c Peter> @@ -42,7 +42,7 @@ extern __typeof(pwrite64) __libc_pwrite64; Peter> #include Peter> -# define __NR___syscall_pread __NR_pread64 Peter> +# define __NR___syscall_pread __NR_pread Peter> static __inline__ _syscall5(ssize_t, __syscall_pread, int, fd, void *, buf, Peter> size_t, count, off_t, offset_hi, off_t, offset_lo) Peter> -- Peter> 1.7.10.4 -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
> "Peter" == Peter Korsgaard writes: Hi, Peter> Some archs (avr32 in particular) still doesn't define __NR_pread64, so Peter> we should fall back to __NR_pread if it isn't available. Peter> The code nicely checks for it, but then ends up hard coding the syscall Peter> to use __NR_pread64 afterwards, rendering the check useless. Fix it by Peter> using the result of the test instead. I noticed another critical issue on ARM EABI. The use of __LONG_LONG_PAIR for the offset doesn't take alignment requirement of 64bit parameters on EABI into consideration, so the offset is off by one register :/ https://lkml.org/lkml/2006/1/12/175 How should that be handled? -- Bye, Peter Korsgaard ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
[PATCH-0.9.33] common/pread_write.c: unbreak on archs without __NR_pread64
Some archs (avr32 in particular) still doesn't define __NR_pread64, so we should fall back to __NR_pread if it isn't available. The code nicely checks for it, but then ends up hard coding the syscall to use __NR_pread64 afterwards, rendering the check useless. Fix it by using the result of the test instead. Signed-off-by: Peter Korsgaard --- Noticed when adding the pending patches for 0.9.33.3 to Buildroot: http://jenkins.free-electrons.com/job/buildroot/config=atstk100x_defconfig/116/console libc/sysdeps/linux/common/pread_write.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libc/sysdeps/linux/common/pread_write.c b/libc/sysdeps/linux/common/pread_write.c index b13de66..8562ab4 100644 --- a/libc/sysdeps/linux/common/pread_write.c +++ b/libc/sysdeps/linux/common/pread_write.c @@ -42,7 +42,7 @@ extern __typeof(pwrite64) __libc_pwrite64; #include -# define __NR___syscall_pread __NR_pread64 +# define __NR___syscall_pread __NR_pread static __inline__ _syscall5(ssize_t, __syscall_pread, int, fd, void *, buf, size_t, count, off_t, offset_hi, off_t, offset_lo) -- 1.7.10.4 ___ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
