diff mbox

[v2] linux: posix_fadvise: restore implementation for xtensa

Message ID ada573f94588305f3bcf05ced59d1dbb3d3fb2a2.1388915042.git.baruch@tkos.co.il
State Accepted, archived
Headers show

Commit Message

Baruch Siach Jan. 5, 2014, 9:44 a.m. UTC
Commit ee84b8b400 (linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT)
removed posix_fadvise implementation for xtensa, since xtensa does not
define __NR_fadvise64. Reuse the ARM support code to restore xtensa support.

This commit is based Mike Frysinger's suggested patch.

Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Use Mike's approach, v1 was broken
---
 libc/sysdeps/linux/common/posix_fadvise.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

Bernhard Reutner-Fischer Jan. 8, 2014, 11:47 a.m. UTC | #1
On Sun, Jan 05, 2014 at 11:44:02AM +0200, Baruch Siach wrote:
> Commit ee84b8b400 (linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT)
> removed posix_fadvise implementation for xtensa, since xtensa does not
> define __NR_fadvise64. Reuse the ARM support code to restore xtensa support.
> 
> This commit is based Mike Frysinger's suggested patch.
> 
> Cc: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: Use Mike's approach, v1 was broken

Applied, thanks!
Vineet Gupta Jan. 9, 2014, 5:58 a.m. UTC | #2
Hi Baruch,

On Sunday 05 January 2014 03:14 PM, Baruch Siach wrote:
> Commit ee84b8b400 (linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT)
> removed posix_fadvise implementation for xtensa, since xtensa does not
> define __NR_fadvise64. Reuse the ARM support code to restore xtensa support.
> 
> This commit is based Mike Frysinger's suggested patch.
> 
> Cc: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: Use Mike's approach, v1 was broken
> ---
>  libc/sysdeps/linux/common/posix_fadvise.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
> index e102ce7afe31..25c294178e5e 100644
> --- a/libc/sysdeps/linux/common/posix_fadvise.c
> +++ b/libc/sysdeps/linux/common/posix_fadvise.c
> @@ -10,18 +10,25 @@
>  
>  #include <sys/syscall.h>
>  
> -#if defined(__NR_fadvise64) || defined(__NR_arm_fadvise64_64)
> +#ifdef __NR_arm_fadvise64_64
> +/* We handle the 64bit alignment issue which is why the arm guys renamed their
> + * syscall in the first place.  So rename it back.
> + */
> +# define __NR_fadvise64_64 __NR_arm_fadvise64_64
> +#endif
> +
> +#if defined(__NR_fadvise64) || defined(__NR_fadvise64_64)
>  # include <fcntl.h>
>  # include <endian.h>
>  # include <bits/wordsize.h>
>  
> -# ifdef __NR_arm_fadvise64_64
> +# ifdef __NR_fadvise64_64
>  int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>  # endif
>  
>  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>  {
> -# ifdef __NR_arm_fadvise64_64
> +# ifdef __NR_fadvise64_64
>  	return posix_fadvise64(fd, offset, len, advice);
>  # else
>  	int ret;
> @@ -41,7 +48,7 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>  	return 0;
>  #  endif
>  }
> -# if defined __UCLIBC_HAS_LFS__ && ((!defined __NR_fadvise64_64 && !defined __NR_arm_fadvise64_64) || __WORDSIZE == 64)
> +# if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
>  strong_alias(posix_fadvise,posix_fadvise64)
>  # endif
>  #endif
> 

Thx for this fix, it unborks e2fsprogs build for ARC. However this doesn't build
for !LFS. It seems !LFS breakage was introduced with 347567313 "add
posix_fadvise() for arm" and confined only to ARM. Now that everyone uses same
code, the damage is noticeable elsewhere too.

The twist now is that, arches such as ARC/METAG are "no-legacy-syscalls-kernel ABI
- so we only have __NR_fadvise64_64 (even for !LFS)

I'll try to come up with a patch later today but just wanted to bring this to
attention anyways.

-Vineet
Baruch Siach Jan. 9, 2014, 6:25 a.m. UTC | #3
Hi Vineet,

On Thu, Jan 09, 2014 at 11:28:19AM +0530, Vineet Gupta wrote:
> On Sunday 05 January 2014 03:14 PM, Baruch Siach wrote:
> > Commit ee84b8b400 (linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT)
> > removed posix_fadvise implementation for xtensa, since xtensa does not
> > define __NR_fadvise64. Reuse the ARM support code to restore xtensa support.
> > 
> > This commit is based Mike Frysinger's suggested patch.
> > 
> > Cc: Mike Frysinger <vapier@gentoo.org>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > v2: Use Mike's approach, v1 was broken
> > ---
> >  libc/sysdeps/linux/common/posix_fadvise.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
> > index e102ce7afe31..25c294178e5e 100644
> > --- a/libc/sysdeps/linux/common/posix_fadvise.c
> > +++ b/libc/sysdeps/linux/common/posix_fadvise.c
> > @@ -10,18 +10,25 @@
> >  
> >  #include <sys/syscall.h>
> >  
> > -#if defined(__NR_fadvise64) || defined(__NR_arm_fadvise64_64)
> > +#ifdef __NR_arm_fadvise64_64
> > +/* We handle the 64bit alignment issue which is why the arm guys renamed their
> > + * syscall in the first place.  So rename it back.
> > + */
> > +# define __NR_fadvise64_64 __NR_arm_fadvise64_64
> > +#endif
> > +
> > +#if defined(__NR_fadvise64) || defined(__NR_fadvise64_64)
> >  # include <fcntl.h>
> >  # include <endian.h>
> >  # include <bits/wordsize.h>
> >  
> > -# ifdef __NR_arm_fadvise64_64
> > +# ifdef __NR_fadvise64_64
> >  int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
> >  # endif
> >  
> >  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
> >  {
> > -# ifdef __NR_arm_fadvise64_64
> > +# ifdef __NR_fadvise64_64
> >  	return posix_fadvise64(fd, offset, len, advice);
> >  # else
> >  	int ret;
> > @@ -41,7 +48,7 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
> >  	return 0;
> >  #  endif
> >  }
> > -# if defined __UCLIBC_HAS_LFS__ && ((!defined __NR_fadvise64_64 && !defined __NR_arm_fadvise64_64) || __WORDSIZE == 64)
> > +# if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
> >  strong_alias(posix_fadvise,posix_fadvise64)
> >  # endif
> >  #endif
> > 
> 
> Thx for this fix, it unborks e2fsprogs build for ARC. However this doesn't build
> for !LFS. It seems !LFS breakage was introduced with 347567313 "add
> posix_fadvise() for arm" and confined only to ARM. Now that everyone uses same
> code, the damage is noticeable elsewhere too.
> 
> The twist now is that, arches such as ARC/METAG are "no-legacy-syscalls-kernel ABI
> - so we only have __NR_fadvise64_64 (even for !LFS)
> 
> I'll try to come up with a patch later today but just wanted to bring this to
> attention anyways.

Thanks for testing. Looking forward to your patch. It's needed for xtensa as 
well, I guess. I really wonder whether this !LFS is worth the trouble.  How 
much do you actually save when disabling LFS?

baruch
Vineet Gupta Jan. 9, 2014, 6:32 a.m. UTC | #4
On Thursday 09 January 2014 11:55 AM, Baruch Siach wrote:
> Hi Vineet,
> 
> On Thu, Jan 09, 2014 at 11:28:19AM +0530, Vineet Gupta wrote:
>> On Sunday 05 January 2014 03:14 PM, Baruch Siach wrote:
>>> Commit ee84b8b400 (linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT)
>>> removed posix_fadvise implementation for xtensa, since xtensa does not
>>> define __NR_fadvise64. Reuse the ARM support code to restore xtensa support.
>>>
>>> This commit is based Mike Frysinger's suggested patch.
>>>
>>> Cc: Mike Frysinger <vapier@gentoo.org>
>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>>> ---
>>> v2: Use Mike's approach, v1 was broken
>>> ---
>>>  libc/sysdeps/linux/common/posix_fadvise.c | 15 +++++++++++----
>>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
>>> index e102ce7afe31..25c294178e5e 100644
>>> --- a/libc/sysdeps/linux/common/posix_fadvise.c
>>> +++ b/libc/sysdeps/linux/common/posix_fadvise.c
>>> @@ -10,18 +10,25 @@
>>>  
>>>  #include <sys/syscall.h>
>>>  
>>> -#if defined(__NR_fadvise64) || defined(__NR_arm_fadvise64_64)
>>> +#ifdef __NR_arm_fadvise64_64
>>> +/* We handle the 64bit alignment issue which is why the arm guys renamed their
>>> + * syscall in the first place.  So rename it back.
>>> + */
>>> +# define __NR_fadvise64_64 __NR_arm_fadvise64_64
>>> +#endif
>>> +
>>> +#if defined(__NR_fadvise64) || defined(__NR_fadvise64_64)
>>>  # include <fcntl.h>
>>>  # include <endian.h>
>>>  # include <bits/wordsize.h>
>>>  
>>> -# ifdef __NR_arm_fadvise64_64
>>> +# ifdef __NR_fadvise64_64
>>>  int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>>>  # endif
>>>  
>>>  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>>  {
>>> -# ifdef __NR_arm_fadvise64_64
>>> +# ifdef __NR_fadvise64_64
>>>  	return posix_fadvise64(fd, offset, len, advice);
>>>  # else
>>>  	int ret;
>>> @@ -41,7 +48,7 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>>  	return 0;
>>>  #  endif
>>>  }
>>> -# if defined __UCLIBC_HAS_LFS__ && ((!defined __NR_fadvise64_64 && !defined __NR_arm_fadvise64_64) || __WORDSIZE == 64)
>>> +# if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
>>>  strong_alias(posix_fadvise,posix_fadvise64)
>>>  # endif
>>>  #endif
>>>
>>
>> Thx for this fix, it unborks e2fsprogs build for ARC. However this doesn't build
>> for !LFS. It seems !LFS breakage was introduced with 347567313 "add
>> posix_fadvise() for arm" and confined only to ARM. Now that everyone uses same
>> code, the damage is noticeable elsewhere too.
>>
>> The twist now is that, arches such as ARC/METAG are "no-legacy-syscalls-kernel ABI
>> - so we only have __NR_fadvise64_64 (even for !LFS)
>>
>> I'll try to come up with a patch later today but just wanted to bring this to
>> attention anyways.
> 
> Thanks for testing. Looking forward to your patch. It's needed for xtensa as 
> well, I guess. I really wonder whether this !LFS is worth the trouble.  How 
> much do you actually save when disabling LFS?

Personally I don't care much, however customers sometimes run into it and if the
feature is there we might as well keep it working.

-Vineet
diff mbox

Patch

diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
index e102ce7afe31..25c294178e5e 100644
--- a/libc/sysdeps/linux/common/posix_fadvise.c
+++ b/libc/sysdeps/linux/common/posix_fadvise.c
@@ -10,18 +10,25 @@ 
 
 #include <sys/syscall.h>
 
-#if defined(__NR_fadvise64) || defined(__NR_arm_fadvise64_64)
+#ifdef __NR_arm_fadvise64_64
+/* We handle the 64bit alignment issue which is why the arm guys renamed their
+ * syscall in the first place.  So rename it back.
+ */
+# define __NR_fadvise64_64 __NR_arm_fadvise64_64
+#endif
+
+#if defined(__NR_fadvise64) || defined(__NR_fadvise64_64)
 # include <fcntl.h>
 # include <endian.h>
 # include <bits/wordsize.h>
 
-# ifdef __NR_arm_fadvise64_64
+# ifdef __NR_fadvise64_64
 int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
 # endif
 
 int posix_fadvise(int fd, off_t offset, off_t len, int advice)
 {
-# ifdef __NR_arm_fadvise64_64
+# ifdef __NR_fadvise64_64
 	return posix_fadvise64(fd, offset, len, advice);
 # else
 	int ret;
@@ -41,7 +48,7 @@  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
 	return 0;
 #  endif
 }
-# if defined __UCLIBC_HAS_LFS__ && ((!defined __NR_fadvise64_64 && !defined __NR_arm_fadvise64_64) || __WORDSIZE == 64)
+# if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
 strong_alias(posix_fadvise,posix_fadvise64)
 # endif
 #endif