From patchwork Wed Aug 24 14:39:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 14908 Received: (qmail 68398 invoked by alias); 24 Aug 2016 14:40:11 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 68386 invoked by uid 89); 24 Aug 2016 14:40:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=act, organization X-HELO: mail-yb0-f178.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=PkwgaQUMCPWBsop43G5sbQXa+350se8h/iF2201yStk=; b=iaytI9de2LSYqxTuOrorUXMZocCxMsP898npYbprFQ+iKTOHOZcihnj4JppcGvoC3u oTZHd5WZd2VRBmH1IWV1vgCoO4pWXHxePU0mlzEOhWXhF09upkLvyP4ALNvb/IE4DdgZ 8K/2cg1MKK9PbUvkZABfgjr8xQCwHZmNxoixCsXm1TYe4cU7l1MlRUAafLO/Nl/zVqVY rgZSBYmg2EkXikN9q3ffmo8f7WiXmmIwNA/G0I8wLBlU6SWkq2IulojlewpnWASAkMou KdQM5wKLr3IaBPwDhATsHUQcvkW9lgTFB6vvwaRzlvgsPqTLjaSvSE7perE8c9lv4Zbf qweQ== X-Gm-Message-State: AEkoouuWcLEh58IXFzr/uO7iL2501ZNEWsCVxcDEVwcMEiMPZOhl8pAswLdHzdY2yM11pNoA X-Received: by 10.37.206.138 with SMTP id x132mr2680173ybe.34.1472049598498; Wed, 24 Aug 2016 07:39:58 -0700 (PDT) Subject: Re: [PATCH 4/4] Consolidate posix_fadvise implementations To: Yury Norov References: <1471617709-16267-1-git-send-email-adhemerval.zanella@linaro.org> <1471617709-16267-5-git-send-email-adhemerval.zanella@linaro.org> <20160824035253.GA6175@yury-N73SV> Cc: libc-alpha@sourceware.org From: Adhemerval Zanella Message-ID: <0381de41-fe7c-ad7f-4859-250c64bed863@linaro.org> Date: Wed, 24 Aug 2016 11:39:55 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160824035253.GA6175@yury-N73SV> 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 --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