| Message ID | 1389260145-8810-1-git-send-email-vgupta@synopsys.com |
|---|---|
| State | New |
| Headers | show |
Ping ! On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote: > commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa" > enabled posix_fadvise() for all arches (it was just not generated > before). > > However this also unearthed an issue introduced by ee84b8b400 > "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to > referencing LFS'ish code (off64_t) w/o proper checks which causes build > to break for !LFS. > > Fix this by calling posix_fadvise64() only for LFS case and open-code > it's equivalent for !LFS. > > Signed-off-by: Vineet Gupta <vgupta@synopsys.com> > Cc: Mike Frysinger <vapier@gentoo.org> > Cc: Baruch Siach <baruch@tkos.co.il> > Cc: Khem Raj <raj.khem@gmail.com> > Cc: markos Chandras <markos.chandras@gmail.com> > Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> > --- > libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c > index 25c294178e5e..14bbeeea13bc 100644 > --- a/libc/sysdeps/linux/common/posix_fadvise.c > +++ b/libc/sysdeps/linux/common/posix_fadvise.c > @@ -22,17 +22,34 @@ > # include <endian.h> > # include <bits/wordsize.h> > > -# ifdef __NR_fadvise64_64 > -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); > -# endif > +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__) > +#include <_lfs_64.h> > > +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); > int posix_fadvise(int fd, off_t offset, off_t len, int advice) > { > -# ifdef __NR_fadvise64_64 > return posix_fadvise64(fd, offset, len, advice); > -# else > +} > +#else > + > +int posix_fadvise(int fd, off_t offset, off_t len, int advice) > +{ > int ret; > INTERNAL_SYSCALL_DECL(err); > + > +# ifdef __NR_fadvise64_64 > +# if __WORDSIZE == 64 > + ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice); > +# else > +# if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__) > + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice, > + OFF_HI_LO (offset), OFF_HI_LO (len)); > +# else > + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, > + OFF_HI_LO (offset), OFF_HI_LO (len), advice); > +# endif > +# endif > +# else /* __NR_fadvise64 */ > # if __WORDSIZE == 64 > ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice); > # else > @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) > # endif > OFF_HI_LO (offset), len, advice); > # endif > +# endif > if (INTERNAL_SYSCALL_ERROR_P (ret, err)) > return INTERNAL_SYSCALL_ERRNO (ret, err); > return 0; > -# endif > } > # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64) > strong_alias(posix_fadvise,posix_fadvise64) > # endif > #endif > +#endif >
Ping ^ 2 On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote: > Ping ! > > On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote: >> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa" >> enabled posix_fadvise() for all arches (it was just not generated >> before). >> >> However this also unearthed an issue introduced by ee84b8b400 >> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to >> referencing LFS'ish code (off64_t) w/o proper checks which causes build >> to break for !LFS. >> >> Fix this by calling posix_fadvise64() only for LFS case and open-code >> it's equivalent for !LFS. >> >> Signed-off-by: Vineet Gupta <vgupta@synopsys.com> >> Cc: Mike Frysinger <vapier@gentoo.org> >> Cc: Baruch Siach <baruch@tkos.co.il> >> Cc: Khem Raj <raj.khem@gmail.com> >> Cc: markos Chandras <markos.chandras@gmail.com> >> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> >> --- >> libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------ >> 1 file changed, 24 insertions(+), 6 deletions(-) >> >> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c >> index 25c294178e5e..14bbeeea13bc 100644 >> --- a/libc/sysdeps/linux/common/posix_fadvise.c >> +++ b/libc/sysdeps/linux/common/posix_fadvise.c >> @@ -22,17 +22,34 @@ >> # include <endian.h> >> # include <bits/wordsize.h> >> >> -# ifdef __NR_fadvise64_64 >> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); >> -# endif >> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__) >> +#include <_lfs_64.h> >> >> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); >> int posix_fadvise(int fd, off_t offset, off_t len, int advice) >> { >> -# ifdef __NR_fadvise64_64 >> return posix_fadvise64(fd, offset, len, advice); >> -# else >> +} >> +#else >> + >> +int posix_fadvise(int fd, off_t offset, off_t len, int advice) >> +{ >> int ret; >> INTERNAL_SYSCALL_DECL(err); >> + >> +# ifdef __NR_fadvise64_64 >> +# if __WORDSIZE == 64 >> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice); >> +# else >> +# if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__) >> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice, >> + OFF_HI_LO (offset), OFF_HI_LO (len)); >> +# else >> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, >> + OFF_HI_LO (offset), OFF_HI_LO (len), advice); >> +# endif >> +# endif >> +# else /* __NR_fadvise64 */ >> # if __WORDSIZE == 64 >> ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice); >> # else >> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) >> # endif >> OFF_HI_LO (offset), len, advice); >> # endif >> +# endif >> if (INTERNAL_SYSCALL_ERROR_P (ret, err)) >> return INTERNAL_SYSCALL_ERRNO (ret, err); >> return 0; >> -# endif >> } >> # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64) >> strong_alias(posix_fadvise,posix_fadvise64) >> # endif >> #endif >> +#endif >>
On Tue, Apr 01, 2014 at 10:53:29AM +0530, Vineet Gupta wrote: > Ping ^ 2 > > On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote: > > Ping ! > > > > On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote: > >> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa" > >> enabled posix_fadvise() for all arches (it was just not generated > >> before). > >> > >> However this also unearthed an issue introduced by ee84b8b400 > >> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to > >> referencing LFS'ish code (off64_t) w/o proper checks which causes build > >> to break for !LFS. > >> > >> Fix this by calling posix_fadvise64() only for LFS case and open-code > >> it's equivalent for !LFS. I do not like the open-coding, can we avoid it? I.e. let the __NR_fadvise64_64 based impl live in posix_fadvise64.c, __WORDSIZE == 64 ? thanks, > >> > >> Signed-off-by: Vineet Gupta <vgupta@synopsys.com> > >> Cc: Mike Frysinger <vapier@gentoo.org> > >> Cc: Baruch Siach <baruch@tkos.co.il> > >> Cc: Khem Raj <raj.khem@gmail.com> > >> Cc: markos Chandras <markos.chandras@gmail.com> > >> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> > >> --- > >> libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------ > >> 1 file changed, 24 insertions(+), 6 deletions(-) > >> > >> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c > >> index 25c294178e5e..14bbeeea13bc 100644 > >> --- a/libc/sysdeps/linux/common/posix_fadvise.c > >> +++ b/libc/sysdeps/linux/common/posix_fadvise.c > >> @@ -22,17 +22,34 @@ > >> # include <endian.h> > >> # include <bits/wordsize.h> > >> > >> -# ifdef __NR_fadvise64_64 > >> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); > >> -# endif > >> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__) > >> +#include <_lfs_64.h> > >> > >> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); > >> int posix_fadvise(int fd, off_t offset, off_t len, int advice) > >> { > >> -# ifdef __NR_fadvise64_64 > >> return posix_fadvise64(fd, offset, len, advice); > >> -# else > >> +} > >> +#else > >> + > >> +int posix_fadvise(int fd, off_t offset, off_t len, int advice) > >> +{ > >> int ret; > >> INTERNAL_SYSCALL_DECL(err); > >> + > >> +# ifdef __NR_fadvise64_64 > >> +# if __WORDSIZE == 64 > >> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice); > >> +# else > >> +# if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__) > >> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice, > >> + OFF_HI_LO (offset), OFF_HI_LO (len)); > >> +# else > >> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, > >> + OFF_HI_LO (offset), OFF_HI_LO (len), advice); > >> +# endif > >> +# endif > >> +# else /* __NR_fadvise64 */ > >> # if __WORDSIZE == 64 > >> ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice); > >> # else > >> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) > >> # endif > >> OFF_HI_LO (offset), len, advice); > >> # endif > >> +# endif > >> if (INTERNAL_SYSCALL_ERROR_P (ret, err)) > >> return INTERNAL_SYSCALL_ERRNO (ret, err); > >> return 0; > >> -# endif > >> } > >> # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64) > >> strong_alias(posix_fadvise,posix_fadvise64) > >> # endif > >> #endif > >> +#endif > >> > > _______________________________________________ > uClibc mailing list > uClibc@uclibc.org > http://lists.busybox.net/mailman/listinfo/uclibc
On Thursday 12 June 2014 09:40 PM, Bernhard Reutner-Fischer wrote: > On Tue, Apr 01, 2014 at 10:53:29AM +0530, Vineet Gupta wrote: >> Ping ^ 2 >> >> On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote: >>> Ping ! >>> >>> On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote: >>>> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa" >>>> enabled posix_fadvise() for all arches (it was just not generated >>>> before). >>>> >>>> However this also unearthed an issue introduced by ee84b8b400 >>>> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to >>>> referencing LFS'ish code (off64_t) w/o proper checks which causes build >>>> to break for !LFS. >>>> >>>> Fix this by calling posix_fadvise64() only for LFS case and open-code >>>> it's equivalent for !LFS. > > I do not like the open-coding, can we avoid it? I don't like it too however I don't see how it will work. W/o LFS we can't call any *64 stuff. > I.e. let the __NR_fadvise64_64 based impl live in posix_fadvise64.c, > __WORDSIZE == 64 ? I don't think I understand. 1. My patch doesn't touch wordsize == 64 case, it handles the size 32 case. 2. I agree that current code is open coding posix_fadvise64 inside posix_fadvise, but I don't know a better way of doing it - ideas / suggestions welcome ! Thx, -Vineet > > thanks, >>>> >>>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com> >>>> Cc: Mike Frysinger <vapier@gentoo.org> >>>> Cc: Baruch Siach <baruch@tkos.co.il> >>>> Cc: Khem Raj <raj.khem@gmail.com> >>>> Cc: markos Chandras <markos.chandras@gmail.com> >>>> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> >>>> --- >>>> libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------ >>>> 1 file changed, 24 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c >>>> index 25c294178e5e..14bbeeea13bc 100644 >>>> --- a/libc/sysdeps/linux/common/posix_fadvise.c >>>> +++ b/libc/sysdeps/linux/common/posix_fadvise.c >>>> @@ -22,17 +22,34 @@ >>>> # include <endian.h> >>>> # include <bits/wordsize.h> >>>> >>>> -# ifdef __NR_fadvise64_64 >>>> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); >>>> -# endif >>>> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__) >>>> +#include <_lfs_64.h> >>>> >>>> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); >>>> int posix_fadvise(int fd, off_t offset, off_t len, int advice) >>>> { >>>> -# ifdef __NR_fadvise64_64 >>>> return posix_fadvise64(fd, offset, len, advice); >>>> -# else >>>> +} >>>> +#else >>>> + >>>> +int posix_fadvise(int fd, off_t offset, off_t len, int advice) >>>> +{ >>>> int ret; >>>> INTERNAL_SYSCALL_DECL(err); >>>> + >>>> +# ifdef __NR_fadvise64_64 >>>> +# if __WORDSIZE == 64 >>>> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice); >>>> +# else >>>> +# if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__) >>>> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice, >>>> + OFF_HI_LO (offset), OFF_HI_LO (len)); >>>> +# else >>>> + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, >>>> + OFF_HI_LO (offset), OFF_HI_LO (len), advice); >>>> +# endif >>>> +# endif >>>> +# else /* __NR_fadvise64 */ >>>> # if __WORDSIZE == 64 >>>> ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice); >>>> # else >>>> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) >>>> # endif >>>> OFF_HI_LO (offset), len, advice); >>>> # endif >>>> +# endif >>>> if (INTERNAL_SYSCALL_ERROR_P (ret, err)) >>>> return INTERNAL_SYSCALL_ERRNO (ret, err); >>>> return 0; >>>> -# endif >>>> } >>>> # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64) >>>> strong_alias(posix_fadvise,posix_fadvise64) >>>> # endif >>>> #endif >>>> +#endif >>>> >> >> _______________________________________________ >> uClibc mailing list >> uClibc@uclibc.org >> http://lists.busybox.net/mailman/listinfo/uclibc
On Tue, Jul 22, 2014 at 08:03:58PM +0530, Vineet Gupta wrote: > On Thursday 12 June 2014 09:40 PM, Bernhard Reutner-Fischer wrote: > > On Tue, Apr 01, 2014 at 10:53:29AM +0530, Vineet Gupta wrote: > >> Ping ^ 2 > >> > >> On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote: > >>> Ping ! > >>> > >>> On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote: > >>>> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa" > >>>> enabled posix_fadvise() for all arches (it was just not generated > >>>> before). > >>>> > >>>> However this also unearthed an issue introduced by ee84b8b400 > >>>> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to > >>>> referencing LFS'ish code (off64_t) w/o proper checks which causes build > >>>> to break for !LFS. > >>>> > >>>> Fix this by calling posix_fadvise64() only for LFS case and open-code > >>>> it's equivalent for !LFS. > > > > I do not like the open-coding, can we avoid it? > > I don't like it too however I don't see how it will work. W/o LFS we can't call > any *64 stuff. > > > I.e. let the __NR_fadvise64_64 based impl live in posix_fadvise64.c, > > __WORDSIZE == 64 ? > > I don't think I understand. > > 1. My patch doesn't touch wordsize == 64 case, it handles the size 32 case. > 2. I agree that current code is open coding posix_fadvise64 inside posix_fadvise, > but I don't know a better way of doing it - ideas / suggestions welcome ! We can revisit this later. Applied this one for now, thanks!
diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c index 25c294178e5e..14bbeeea13bc 100644 --- a/libc/sysdeps/linux/common/posix_fadvise.c +++ b/libc/sysdeps/linux/common/posix_fadvise.c @@ -22,17 +22,34 @@ # include <endian.h> # include <bits/wordsize.h> -# ifdef __NR_fadvise64_64 -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); -# endif +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__) +#include <_lfs_64.h> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice); int posix_fadvise(int fd, off_t offset, off_t len, int advice) { -# ifdef __NR_fadvise64_64 return posix_fadvise64(fd, offset, len, advice); -# else +} +#else + +int posix_fadvise(int fd, off_t offset, off_t len, int advice) +{ int ret; INTERNAL_SYSCALL_DECL(err); + +# ifdef __NR_fadvise64_64 +# if __WORDSIZE == 64 + ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice); +# else +# if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__) + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice, + OFF_HI_LO (offset), OFF_HI_LO (len)); +# else + ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, + OFF_HI_LO (offset), OFF_HI_LO (len), advice); +# endif +# endif +# else /* __NR_fadvise64 */ # if __WORDSIZE == 64 ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice); # else @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) # endif OFF_HI_LO (offset), len, advice); # endif +# endif if (INTERNAL_SYSCALL_ERROR_P (ret, err)) return INTERNAL_SYSCALL_ERRNO (ret, err); return 0; -# endif } # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64) strong_alias(posix_fadvise,posix_fadvise64) # endif #endif +#endif
commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa" enabled posix_fadvise() for all arches (it was just not generated before). However this also unearthed an issue introduced by ee84b8b400 "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to referencing LFS'ish code (off64_t) w/o proper checks which causes build to break for !LFS. Fix this by calling posix_fadvise64() only for LFS case and open-code it's equivalent for !LFS. Signed-off-by: Vineet Gupta <vgupta@synopsys.com> Cc: Mike Frysinger <vapier@gentoo.org> Cc: Baruch Siach <baruch@tkos.co.il> Cc: Khem Raj <raj.khem@gmail.com> Cc: markos Chandras <markos.chandras@gmail.com> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> --- libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-)