Re: [PATCH] Consolidate Linux readahead() implementations

[Florian Weimer <fw at deneb dot enyo dot de>]

* Yury Norov:

> On Thu, Sep 22, 2016 at 06:36:22PM -0300, Adhemerval Zanella wrote:
>> Hi Yury, some comments below.
>> 
>> On 22/09/2016 17:44, Yury Norov wrote:
>> I think we can use SYSCALL_LL64 plus __ALIGNMENT_ARG here.  The tricky is to pass
>> the correct argument size, since it used by the macro to select the correct 
>> arch-dependent one.  This is exactly why I proposed the patch to add
>> {INTERNAL,INLINE}_SYSCALL_CALL [1], readahead will be simplified to just:
>> 
>> ssize_t
>> __readahead (int fd, off64_t offset, size_t count)
>> {
>>   return INLINE_SYSCALL_CALL (readahead, fd,
>> 			      __ALIGNMENT_ARG SYSCALL_LL64 (offset));
>> }
>> 
>> I suggest you to wait the {INTERNAL,INLINE}_SYSCALL_CALL patch to land and
>> based this one on it.  I think I addressed most of Florian comments, I just
>> need to check if assembly generated using the new macros is the same as
>> before (which I expect to).
>> 
>> [1] https://sourceware.org/ml/libc-alpha/2016-09/msg00452.html
>
> __ALIGNMENT_ARG is controlled by __ASSUME_ALIGNED_REGISTER_PAIRS,
> as well as __ALIGNMENT_COUNT(). Currently we have 4 ports that
> enable it: mips, arm, powerpc and tile. Mips and arm pass 5 arguments,
> so __ASSUME_ALIGNED_REGISTER_PAIRS is what they need. Powerpc uses
> syscalls.list (thanks for pointing it), and so is not affected by this
> change. But tile is still in case. Is my understanding correct that it
> uses linux/readahead.c as implementation? If so, this patch will break
> tile ABI. That's why I introduced new option.
>
> So, as for me we cannot use __ASSUME_ALIGNED_REGISTER_PAIRS because of
> tile. Is it correct?

Does readahead even work on tile today?  Maybe it's already broken and
this change fixes it. :)

Chris?