diff mbox

[4/4] Consolidate posix_fadvise implementations

Message ID 0381de41-fe7c-ad7f-4859-250c64bed863@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella Netto Aug. 24, 2016, 2:39 p.m. UTC 
On 24/08/2016 00:52, Yury Norov wrote:
> On Fri, Aug 19, 2016 at 11:41:49AM -0300, Adhemerval Zanella wrote:

> Ah, forgot to notice...

> 

> [...]

> 

>> diff --git a/sysdeps/unix/sysv/linux/arm/kernel-features.h b/sysdeps/unix/sysv/linux/arm/kernel-features.h

>> index 6ca607e..628d27f 100644

>> --- a/sysdeps/unix/sysv/linux/arm/kernel-features.h

>> +++ b/sysdeps/unix/sysv/linux/arm/kernel-features.h

>> @@ -27,6 +27,13 @@

>>  # undef __ASSUME_SET_ROBUST_LIST

>>  #endif

>>  

>> +/* ARM fadvise64_64 reorganize the syscall arguments.  */

>> +#define __ASSUME_FADVISE64_64_6ARG	1

> 

> [...]

> 

>> diff --git a/sysdeps/unix/sysv/linux/posix_fadvise.c b/sysdeps/unix/sysv/linux/posix_fadvise.c

>> index 093d707..869a642 100644

>> --- a/sysdeps/unix/sysv/linux/posix_fadvise.c

>> +++ b/sysdeps/unix/sysv/linux/posix_fadvise.c

>> @@ -22,27 +22,46 @@

>>  /* Advice the system about the expected behaviour of the application with

>>     respect to the file associated with FD.  */

>>  

>> +#ifndef __OFF_T_MATCHES_OFF64_T

>> +

>> +/* Both arm and powerpc implements fadvise64_64 with last 'advise' argument

>> +   just after 'fd' to avoid the requirement of implementing 7-arg syscalls.

>> +   ARM also defines __NR_fadvise64_64 as __NR_arm_fadvise64_64.

>> +

>> +   tile requires __ASSUME_ALIGNED_REGISTER_PAIRS but implements the 32-bit

>> +   fadvise64_64 without the padding 0 after fd.

>> +

>> +   s390 implements fadvice64_64 using a specific struct with arguments

>> +   packed inside.  This is the only implementation handled in arch-specific

>> +   code.  */

>> +

>>  int

>>  posix_fadvise (int fd, off_t offset, off_t len, int advise)

>>  {

>> -#if defined(__NR_fadvise64) || defined(__NR_fadvise64_64)

>>    INTERNAL_SYSCALL_DECL (err);

>>  # ifdef __NR_fadvise64

>> -  int ret = INTERNAL_SYSCALL (fadvise64, err, 5, fd,

>> -			      __LONG_LONG_PAIR (offset >> 31, offset), len,

>> -			      advise);

>> +  int ret = INTERNAL_SYSCALL_CALL (fadvise64, err, fd,

>> +				   __ALIGNMENT_ARG SYSCALL_LL (offset),

>> +				   len, advise);

>>  # else

>> -  int ret = INTERNAL_SYSCALL (fadvise64_64, err, 6, fd,

>> -			      __LONG_LONG_PAIR ((long) (offset >> 31),

>> -						(long) offset),

>> -			      __LONG_LONG_PAIR ((long) (len >> 31),

>> -						(long) len),

>> -			      advise);

>> +#  ifdef __ASSUME_FADVISE64_64_6ARG

> 

> You should define __ASSUME_FADVISE64_64_6ARG for all ports,

> and use #if instead of #ifdef.

> 

> For all new options you introduce. This is coding style rule. :(

> 


My understanding is this is not the general idea of kernel-features.h
current organization.  It is included several times in object build and the
idea is to have linux default behaviour in default Linux file with
ports undefining non-supported features.  There were some cleanup recently
in this directions by increasing the minimum kernel version supported.

So I the way to follow kernel-features conventions is to invert current
logic and add:


And set fallocate.c to act as:

#  ifdef __ASSUME_FADVISE64_64_DEFAULT_ARG
#   ifndef __ASSUME_FADVISE64_64_ALIGN
#     undef __ALIGNMENT_ARG
#     define __ALIGNMENT_ARG
#   endif 
  int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd,
                                   __ALIGNMENT_ARG SYSCALL_LL (offset),
                                   SYSCALL_LL (len), advise);
#  else
  int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd, advise,
                                   __ALIGNMENT_ARG SYSCALL_LL (offset),
                                   SYSCALL_LL (len));
#  endif


I would strongly prefer to *not* have to define two new define on every 
kernel-features.h and even create extra files for ports that do not 
require such header (as for aarch64).
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
index 1d3b554..362c2ef 100644
--- a/sysdeps/unix/sysv/linux/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/kernel-features.h
@@ -147,3 +147,6 @@ 
    separate syscalls were only added later.  */
 #define __ASSUME_SENDMSG_SYSCALL       1
 #define __ASSUME_RECVMSG_SYSCALL       1
+
+#define __ASSUME_FADVISE64_64_DEFAULT_ARG      1
+#define __ASSUME_FADVISE64_64_ALIGN            1