From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42223 invoked by alias); 10 Jan 2019 20:32:03 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 42212 invoked by uid 89); 10 Jan 2019 20:32:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=HDKIM-Filter:Filter, HDKIM-Filter:OpenDKIM, H*UA:Zimbra, H*M:zimbra X-HELO: mail.efficios.com DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com A6D70B1EF3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficios.com; s=default; t=1547152315; bh=Qylj13W67+MLvuB9r/ny2zzNL99I1zZw9ZHaJ/LA8/w=; h=Date:From:To:Message-ID:MIME-Version; b=jk8ZUzzk6AnBqm5LBQtMub5tISpNmVF5/TqNb6iIkmlMX0+TT1DneWb9vS5oSnfK7 kiVy0Df5bpRYampLV1EVbZFyZjzMNEiisIKwfvGnK3WigGcgevx4QrUKvlq2rfhDIv QHBHl6/K2kko2xrk8nM7qSW79S8MqR88MqIVPxU3I1rY1+bZ2ZUUO2E/SdlpI7iMtb SGAq4MLaodnpWySo9wUAxrnlX4VSu2MLe11HPHKXM7a5nzK3L0c6FG0XqYlvAlTBTO mQCrAIxwZeuA/SwQjbAcg+dv2zqIFunUJsjIg9ghicOWTW+SvCJ9r2PeOmJv/8Ac01 kmX09ghRsC1og== Date: Thu, 10 Jan 2019 20:32:00 -0000 From: Mathieu Desnoyers To: Florian Weimer Cc: carlos , Joseph Myers , Szabolcs Nagy , libc-alpha , Thomas Gleixner , Ben Maurer , Peter Zijlstra , "Paul E. McKenney" , Boqun Feng , Will Deacon , Dave Watson , Paul Turner , Rich Felker , linux-kernel , linux-api Message-ID: <1681283664.1380.1547152315426.JavaMail.zimbra@efficios.com> In-Reply-To: <87h8fkz6qx.fsf@oldenburg2.str.redhat.com> References: <20181204192141.4684-1-mathieu.desnoyers@efficios.com> <87h8fkz6qx.fsf@oldenburg2.str.redhat.com> Subject: Re: [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at nptl init and thread creation (v4) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-01/txt/msg00236.txt.bz2 ----- On Dec 11, 2018, at 2:40 AM, Florian Weimer fweimer@redhat.com wrote: > * Mathieu Desnoyers: > >> I want to keep the __rseq_refcount symbol so out-of-libc users can >> register rseq if they are linked against a pre-2.29 libc. > > Sorry, I was confused. Hi Florian, Thanks for your questions below. Sorry for my delayed answer, I've been preempted by vacation time. See more below, > >> diff --git a/csu/Makefile b/csu/Makefile >> index 88fc77662e..81d471587f 100644 >> --- a/csu/Makefile >> +++ b/csu/Makefile >> @@ -28,7 +28,7 @@ include ../Makeconfig >> >> routines = init-first libc-start $(libc-init) sysdep version check_fds \ >> libc-tls elf-init dso_handle >> -aux = errno >> +aux = errno rseq >> elide-routines.os = libc-tls >> static-only-routines = elf-init >> csu-dummies = $(filter-out $(start-installed-name),crt1.o Mcrt1.o) > > Do we plan to add Hurd support for this? No. A logical path where we could move rseq.c is under sysdeps/unix/sysv/linux/rseq.c. This would allow the __rseq_abi symbol to be used from anywhere in glibc. > >> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h >> b/sysdeps/unix/sysv/linux/rseq-internal.h >> new file mode 100644 >> index 0000000000..2367926def >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h > >> +#define RSEQ_SIG 0x53053053 > > What's this? This needs a comment. I will move it to an installed header (sysdeps/unix/sysv/linux/sys/rseq.h) with the following comment: /* Signature required before each abort handler code. */ #define RSEQ_SIG 0x53053053 > >> +extern __thread volatile struct rseq __rseq_abi >> +__attribute__ ((tls_model ("initial-exec"))); >> + >> +extern __thread volatile uint32_t __rseq_refcount >> +__attribute__ ((tls_model ("initial-exec"))); > > The volatile qualifier needs justification in a comment. (Usually, > volatile is wrong. and it is difficult to get rid of it.) > > We need to document these public symbols somewhere. There should be an > installed header file. Moving to sysdeps/unix/sysv/linux/sys/rseq.h with the following comments: /* volatile because fields can be read/updated by the kernel. */ extern __thread volatile struct rseq __rseq_abi __attribute__ ((tls_model ("initial-exec"))); /* volatile because refcount can be read/updated by signal handlers. */ extern __thread volatile uint32_t __rseq_refcount __attribute__ ((tls_model ("initial-exec"))); > >> diff --git a/nptl/Versions b/nptl/Versions >> index e7f691da7a..f7890f73fc 100644 >> --- a/nptl/Versions >> +++ b/nptl/Versions >> @@ -277,6 +277,10 @@ libpthread { >> cnd_timedwait; cnd_wait; tss_create; tss_delete; tss_get; tss_set; >> } >> >> + GLIBC_2.29 { >> + __rseq_refcount; >> + } > > Why put this into libpthread, and __rseq_abi into libc? The __rseq_abi symbol should be available to the glibc memory allocator. I plan to move the __rseq_abi to sysdeps/unix/sysv/linux/Versions instead so it becomes Linux-specific. The __rseq_refcount symbol only needs to be made available to applications and libraries linking against libpthread, because only libpthread actually handles the rseq registration/unregistration at thread start/exit and library initialization. However, considering that we want this to be Linux-specific as well, I could move it to sysdeps/unix/sysv/linux/Versions too. Then it would make sense to move the __rseq_refcount symbol defined in nptl/rseq.c to sysdeps/unix/sysv/linux/rseq.c as well and group everything together. Therefore, both symbols will end up in sysdeps/unix/sysv/linux/Versions. > > What, exactly, is the benefit of having __rseq_refcount defined by > glibc? Have you actually got this working? If an rseq library is > linked against glibc 2.29, it will reference the GLIBC_2.29 symbol > version, so it cannot be loaded by older glibcs. In this case, > __rseq_refcount is not needed. > > If you build against pre-2.29, then the __rseq_refcount symbol will be > unversioned. But then you don't need it glibc, either. > > So it seems to me that the addition to glibc is useless in both > scenarios. Am I missing something? Here is the scenario where it becomes useful: librseq is built against a pre-2.29 glibc. So the __rseq_refcount symbol it emits is unversioned. Application is build against 2.29 glibc. Application links both against librseq (itself built against pre-2.29 glibc) and glibc (2.29). In that scenario, librseq and glibc rely on a unique __rseq_refcount TLS variable per process ensure that they don't register rseq twice for each thread. > > By the way, you could avoid the need for unregistration if you allocated > the rseq areas persistently, index by TID. They are quite small, so > with the typical PID range, maybe the wasted memory due to changing TIDs > would be acceptable? Would we be able to access those __rseq_abi as normal TLS IE model variables ? The overhead of indexing an array matters for a fast-path. > > I guess things would be so much easier if the kernel simply provided a > means to obtain the address of a previously registered rseq area. Even if the kernel did provide this (which is not part of the syscall ABI anyway), I suspect we would need extra code on the fast-path to access the __rseq_abi TLS, which I would very much like to avoid. But perhaps there are ways to do this without extra overhead that are beyond my understanding of glibc handling of TLS models. I will soon post an updated patch set taking care of your comments. Thanks! Mathieu > > Thanks, > Florian -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com