This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Consolidate fallocate/fallocate64 implementations
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Arnd Bergmann <arnd at arndb dot de>, libc-alpha at sourceware dot org
- Date: Mon, 27 Jun 2016 20:12:29 -0300
- Subject: Re: [PATCH] Consolidate fallocate/fallocate64 implementations
- Authentication-results: sourceware.org; auth=none
- References: <1467056073-8061-1-git-send-email-adhemerval dot zanella at linaro dot org> <4587082 dot yzeSXCKaO7 at wuerfel> <5771AE52 dot 6090406 at linaro dot org>
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