This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Consolidate fallocate/fallocate64 implementations



On 27/06/2016 19:53, Adhemerval Zanella wrote:
> 
> 
> On 27/06/2016 18:43, Arnd Bergmann wrote:
>> On Monday, June 27, 2016 4:34:33 PM CEST Adhemerval Zanella wrote:
>>> diff --git a/sysdeps/unix/sysv/linux/fallocate.c b/sysdeps/unix/sysv/linux/fallocate.c
>>> index 6a58a5f..e648a73 100644
>>> --- a/sysdeps/unix/sysv/linux/fallocate.c
>>> +++ b/sysdeps/unix/sysv/linux/fallocate.c
>>> @@ -19,17 +19,12 @@
>>>  #include <fcntl.h>
>>>  #include <sysdep-cancel.h>
>>>  
>>> +#if __WORDSIZE != 64 || defined (__ASSUME_OFF_DIFF_OFF64)
>>>  /* Reserve storage for the data of the file associated with FD.  */
>>>  int
>>>  fallocate (int fd, int mode, __off_t offset, __off_t len)
>>>  {
>>>    return SYSCALL_CANCEL (fallocate, fd, mode,
>>> +                        SYSCALL_LL (offset), SYSCALL_LL (len));
>>>  }
>>> +#endif
>>> diff --git a/sysdeps/unix/sysv/linux/fallocate64.c b/sysdeps/unix/sysv/linux/fallocate64.c
>>> index 8e76d6f..176caa8 100644
>>> --- a/sysdeps/unix/sysv/linux/fallocate64.c
>>> +++ b/sysdeps/unix/sysv/linux/fallocate64.c
>>> @@ -24,14 +24,10 @@
>>>  int
>>>  fallocate64 (int fd, int mode, __off64_t offset, __off64_t len)
>>>  {
>>>    return SYSCALL_CANCEL (fallocate, fd, mode,
>>> +                        SYSCALL_LL64 (offset), SYSCALL_LL64 (len));
>>>  }
>>> +
>>> +#if __WORDSIZE == 64 && !defined (__ASSUME_OFF_DIFF_OFF64)
>>> +weak_alias (fallocate64, fallocate)
>>> +#endif
>>>
>>
>> How should this work for new 32-bit architectures when they do
>> not set __ASSUME_OFF_DIFF_OFF64?
>>
>> It looks like you have two identical implementations then, though
>> you would want to have just one of them plus an alias.
>>
>> Maybe the "__WORDSIZE == 64" check can simply be dropped and all
>> existing 32-bit architectures set __ASSUME_OFF_DIFF_OFF64 instead?
>>
>> 	Arnd
>>
> 
> It was something that crossed my mind just after I sent the patch.
> And it is not wrong itself, although suboptimal as you noted
> (glibc will provide fallocate and fallocate64 which will be same the
> implementation).
> 
> I think for fallocate we can just use
> 
> fallocate.c
> #if __WORDSIZE != 64 && !defined __ASSUME_WORDSIZE64_ILP32
> /* build fallocate */
> #endif
> 
> and
> 
> fallocate64.c
> 
> /* build fallocate64 */
> #if __WORDSIZE == 64 || defined __ASSUME_WORDSIZE64_ILP32
> weak_alias (fallocate64, fallocate)
> #endif
> 
> It will avoid building fallocate for ILP32 architectures with
> off and off64_t with the same size.
> 

My mistake here, I was with the wrong assumption aarch64/ilp32 was
following the already supported ilp32 architectures for passing
off_t/off64_t (I still reading all the discussion for ilp32).

I see now correct way is indeed what Yuri Norov has suggested in
the previous [1] thread:

fallocate.c
#ifndef __OFF_T_MATCHES_OFF64_T
/* build fallocate */
#endif

and

fallocate64.c

/* build fallocate64 */
#ifdef __OFF_T_MATCHES_OFF64_T
weak_alias (fallocate64, fallocate)
#endif


I am assuming here aarch64/ilp32 is not defining __ASSUME_WORDSIZE64_ILP32
(which I plan to check in the patchset).

[1] https://sourceware.org/ml/libc-alpha/2016-06/msg00958.html


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]