From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 66B881F462 for ; Fri, 24 May 2019 15:41:19 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id:references :mime-version:content-type:in-reply-to; q=dns; s=default; b=b6R+ AV4fOjChxiUU7wKIPZi2zQplxwofg17smEmHYUp6pP5FHJ9ySC/xjVfD5Hw8QnQM 9QYB5fohz9WR0txc072pRfhkaBCjPe6ItnUHS087mpp7YzILALbDl5n32JXxRDQ/ D01UpT2ZfmLnbZQY0pM6w4EUOJokeGylAUBj3KM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id:references :mime-version:content-type:in-reply-to; s=default; bh=49SU9RWtcf gwrgSh3p6GW/BHG44=; b=ihIT9xU4Q3oXYAU15B/6ncdpSfIVzMZTDIU988wJ3z CvykxMzR3vCu/azGLskWCLD6oQb5eWu3RHrqxlbHGeiUmGkbuO8A9WD4pbo7rYPV DsYY/1tSgshyu++zkCXUpYtz/M6GWdq33aC3FHMMnOmmcljXMN/nJHC2C680KzJN c= Received: (qmail 124460 invoked by alias); 24 May 2019 15:41:17 -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 Received: (qmail 123409 invoked by uid 89); 24 May 2019 15:41:15 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: sym2.noone.org Date: Fri, 24 May 2019 17:41:11 +0200 From: Tobias Klauser To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN Message-ID: <20190524154111.l5ofva7drfld7olg@distanz.ch> References: <20190516115924.29027-1-tklauser@distanz.ch> <95538879-7ff2-275b-ad26-fd1e68ebbd1a@linaro.org> <20190524072356.it7dqmlkmt4vf3p2@distanz.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) On 2019-05-24 at 16:42:32 +0200, Adhemerval Zanella wrote: > > > On 24/05/2019 04:23, Tobias Klauser wrote: > > Thank you for the review. > > > > On 2019-05-23 at 21:36:01 +0200, Adhemerval Zanella wrote: > >> On 16/05/2019 08:59, Tobias Klauser wrote: > >>> Define all currently used Linux versions used for > >>> PREPARE_VERSION{,_KNOWN} in sysdeps/unix/sysv/linux/dl-vdso.h and use > >>> them instead of duplicating the versions and precomputed hashes across > >>> architecture specific files. > >>> > >>> 2019-05-16 Tobias Klauser > >>> > >>> * sysdeps/unix/sysv/linux/aarch64/gettimeofday.c (INIT_ARCH): Use > >>> PREPARE_VERSION_KNOWN. > >>> * sysdeps/unix/sysv/linux/aarch64/init-first.c: Likewise. > >>> * sysdeps/unix/sysv/linux/dl-vdso.h (VDSO_NAME_LINUX_2_6_39): New > >>> define. > >>> (VDSO_HASH_LINUX_2_6_39): Likewise. > >>> (VDSO_NAME_LINUX_4_9): Likewise. > >>> (VDSO_HASH_LINUX_4_9): Likewise. > >>> * sysdeps/unix/sysv/linux/m68k/init-first.c (_libc_vdso_platform_setup): > >>> Use PREPARE_VERSION_KNOWN. > >>> * sysdeps/unix/sysv/linux/powerpc/gettimeofday.c (INIT_ARCH): Likewise. > >>> * sysdeps/unix/sysv/linux/powerpc/init-first.c > >>> (_libc_vdso_platform_setup): Likewise. > >>> * sysdeps/unix/sysv/linux/powerpc/time.c (INIT_ARCH): Likewise. > >>> * sysdeps/unix/sysv/linux/s390/init-first.c (_libc_vdso_platform_setup): > >>> Likewise. > >>> * sysdeps/unix/sysv/linux/x86_64/init-first.c (__vdso_platform_setup): > >>> Likewise. > >> > >> > >> LGTM, I assume you checked a build against affected architectures. As a side > >> note I think also should just remove PREPARE_VERSION macro and replace it with > >> a more sane interface: > > > > I (cross-)compile tested on all architectures and ran the test suite on > > x86_64. > > > >> -- > >> static inline struct r_found_version > >> prepare_version_base (const char *name, ElfW(Word) hash) > >> { > >> assert (hash == _dl_elf_hash (name)); > >> return (struct r_found_version) { name, hash, 1, NULL }; > >> } > >> #define prepare_version(vname) \ > >> prepare_version_base (VDSO_NAME_##vname, VDSO_HASH_##vname) > >> -- > > > > Should I add this to a v2 of this patch? Or should I send a follow-up > > patch with the change? > > I don't have a preference here. Refactor the vDSO code is something > I have in mind for some time. Sent it as a follow-up patch with Message-Id: <20190524153941.10128-1-tklauser@distanz.ch> And will send another follow-up for sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > > > > >>> --- > >>> sysdeps/unix/sysv/linux/aarch64/gettimeofday.c | 4 ++-- > >>> sysdeps/unix/sysv/linux/aarch64/init-first.c | 4 ++-- > >>> sysdeps/unix/sysv/linux/dl-vdso.h | 4 ++++ > >>> sysdeps/unix/sysv/linux/m68k/init-first.c | 2 +- > >>> sysdeps/unix/sysv/linux/powerpc/gettimeofday.c | 4 ++-- > >>> sysdeps/unix/sysv/linux/powerpc/init-first.c | 2 +- > >>> sysdeps/unix/sysv/linux/powerpc/time.c | 4 ++-- > >>> sysdeps/unix/sysv/linux/s390/init-first.c | 2 +- > >>> sysdeps/unix/sysv/linux/x86_64/init-first.c | 2 +- > >>> 9 files changed, 16 insertions(+), 12 deletions(-) > >>> > >>> diff --git a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > >>> index 6c008ed9357f..9180b50bf7c3 100644 > >>> --- a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > >>> +++ b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > >>> @@ -38,13 +38,13 @@ __gettimeofday_vsyscall (struct timeval *tv, struct timezone *tz) > >>> return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > >>> } > >>> > >>> -/* PREPARE_VERSION will need an __LP64__ ifdef when ILP32 support > >>> +/* PREPARE_VERSION_KNOWN will need an __LP64__ ifdef when ILP32 support > >>> goes in. See _libc_vdso_platform_setup in > >>> sysdeps/unix/sysv/linux/aarch64/init-first.c. */ > >>> > >>> # undef INIT_ARCH > >>> # define INIT_ARCH() \ > >>> - PREPARE_VERSION (linux_version, "LINUX_2.6.39", 123718537); \ > >>> + PREPARE_VERSION_KNOWN (linux_version, LINUX_2_6_39); \ > >>> void *vdso_gettimeofday = \ > >>> _dl_vdso_vsym ("__kernel_gettimeofday", &linux_version); > >> > >> Also a side note, this vsdo setup is not required. The init-first.c > >> already setups the VDSO_SYMBOL(gettimeofday). So a possible followup > >> cleanup would be: > >> > >> -- > >> # undef INIT_ARCH > >> # define INIT_ARCH() > >> > >> static inline void * > >> gettimeofday_vdso (void) > >> { > >> __typeof (VDSO_SYMBOL(gettimeofday)) vdsop = VDSO_SYMBOL(gettimeofday); > >> PTR_DEMANGLE (vdsop); > >> return vdsop != NULL ? vdsop : (void *) __gettimeofday_vsyscall; > >> } > >> > >> libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday, > >> gettimeofday_vdso ()) > >> -- > > > > Thanks, will send a follow-up patch. > > >