This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Consolidate Linux readahead() implementations
- From: Yury Norov <ynorov at caviumnetworks dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: <libc-alpha at sourceware dot org>
- Date: Fri, 23 Sep 2016 02:21:19 +0300
- Subject: Re: [PATCH] Consolidate Linux readahead() implementations
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Yuri dot Norov at caviumnetworks dot com;
- References: <1474577068-1781-1-git-send-email-ynorov@caviumnetworks.com> <827f758c-b744-22f2-5dbb-4471208cd6b2@linaro.org>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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?
Is my understanding correct that powerpc layout is identical to arm
and mips? If so we can remove readahead from powerpc syscalls.abilst
and switch to linux/readahead.c with this patch (or with one proposed
by you).
Yury.