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=-11.6 required=3.0 tests=AWL,BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS, USER_IN_DEF_DKIM_WL 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 D14B31F619 for ; Wed, 26 Feb 2020 17:27:55 +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:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-type; q=dns; s=default; b=pbcM Sd93sC3mdnNBl77nnnnFZW+k/BxeiXNVnLEZ1ZY0ESWN79MnL8TDt7hcTqZJ0O4S DYmuBCC6Pni3lLag8cH0IJ8Co7JorhE0IEyis7HrYtZSyA9VkmmJS7GriUXltNKq mpSu4hpLHSZEI/DiCrafAJkc4jtTVszHNRCuL7k= 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:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-type; s=default; bh=VgfWQh1Kh5 4/qW3AoQ6+TrHoNJY=; b=y3Q5d1iNUYvthtHuKSG72/eGzYni6FSb7c2mkoITJ9 Ykv6SWIpzLfHfLAYqOyk9MakrfzP4yELIayHAzqrnQtFG3QgphQ6QVzMub862X3l 3M5xaNyuUmWBl6hBbRWqTMwY93aVai+Z3GEbS+KRZwhib4o57VJgmIa2DcYw+Hxo 8= Received: (qmail 116439 invoked by alias); 26 Feb 2020 17:27:53 -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 116431 invoked by uid 89); 26 Feb 2020 17:27:52 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-wr1-f48.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Guhz1TvLa+j1kLsI8L+19deSsQysL9993WVY47CsKR0=; b=oCEmacRFo+nQYpacfKMzpwTIVY0CzTR7rB6sAB57a3/DesOjLeh4CRwMIfbSeR8FYx F1uipdbUYpVCoTJCTXeQ15cNiRG9R4BnexbXkB2FPCLIyrc7ey8cgBnZQ69ivDQM1b4T O/vnc3tzeOI+oSnEArvlIU972xBCmVhn/ONkrK+oUrfqV1lXHgGpz0EISocApJHMMHAR /FZt9VuPJv6fEwCEVLG2rdwSiIqJ6BLWNo6jn8hyBtOZgkXyov3ntp3uHxZd9nfQxYDA fga64Rq72jEGJyzSImRuPiUiAHAxjseokf8Vg6lPOLh6uEl41jbti5UTGMrZOE0SMyUk 3OSw== MIME-Version: 1.0 References: <1503467992.2999.1582234410317.JavaMail.zimbra@efficios.com> <20200221154923.GC194360@google.com> <1683022606.3452.1582301632640.JavaMail.zimbra@efficios.com> <1089333712.8657.1582736509318.JavaMail.zimbra@efficios.com> In-Reply-To: <1089333712.8657.1582736509318.JavaMail.zimbra@efficios.com> From: Chris Kennelly Date: Wed, 26 Feb 2020 12:27:36 -0500 Message-ID: Subject: Re: Rseq registration: Google tcmalloc vs glibc To: Mathieu Desnoyers Cc: "Joel Fernandes, Google" , Paul Turner , Florian Weimer , "Carlos O'Donell" , libc-alpha , linux-kernel , Peter Zijlstra , paulmck , Boqun Feng , Brian Geffon Content-Type: text/plain; charset="UTF-8" On Wed, Feb 26, 2020 at 12:01 PM Mathieu Desnoyers wrote: > > ----- On Feb 25, 2020, at 10:38 PM, Chris Kennelly ckennelly@google.com wrote: > > > On Tue, Feb 25, 2020 at 10:25 PM Joel Fernandes wrote: > >> > >> On Fri, Feb 21, 2020 at 11:13 AM Mathieu Desnoyers > >> wrote: > >> > > >> > ----- On Feb 21, 2020, at 10:49 AM, Joel Fernandes, Google > >> > joel@joelfernandes.org wrote: > >> > > >> > [...] > >> > >> > >> > >> 3) Use the __rseq_abi TLS cpu_id field to know whether Rseq has been > >> > >> registered. > >> > >> > >> > >> - Current protocol in the most recent glibc integration patch set. > >> > >> - Not supported yet by Linux kernel rseq selftests, > >> > >> - Not supported yet by tcmalloc, > >> > >> > >> > >> Use the per-thread state to figure out whether each thread need to register > >> > >> Rseq individually. > >> > >> > >> > >> Works for integration between a library which exists for the entire lifetime > >> > >> of the executable (e.g. glibc) and other libraries. However, it does not > >> > >> allow a set of libraries which are dlopen'd/dlclose'd to co-exist without > >> > >> having a library like glibc handling the registration present. > >> > > > >> > > Mathieu, could you share more details about why during dlopen/close > >> > > libraries we cannot use the same __rseq_abi TLS to detect that rseq was > >> > > registered? > >> > > >> > Sure, > >> > > >> > A library which is only loaded and never closed during the execution of the > >> > program can let the kernel implicitly unregister rseq at thread exit. For > >> > the dlopen/dlclose use-case, we need to be able to explicitly unregister > >> > each thread's __rseq_abi which sit in a library which is going to be > >> > dlclose'd. > >> > >> Mathieu, Thanks a lot for the explanation, it makes complete sense. It > >> sounds from Chris's reply that tcmalloc already checks > >> __rseq_abi.cpu_id and is not dlopened/closed. Considering these, it > >> seems to already handle things properly - CMIIW. > > > > I'll make a note about this, since we can probably benefit from some > > more comments about the assumptions/invariants the fastpath uses. > > I suspect the integration with glibc and with dlopen'd/dlclose'd libraries will not > behave correctly with the current tcmalloc implementation. > > Based on the tcmalloc code-base, InitFastPerCpu is only called from IsFast. As long > as this is the only expected caller, having IsFast comparing the RseqCpuId detects > whether glibc (or some other library) has already registered rseq for the current > thread. > > However, if the application chooses to invoke InitFastPerCpu() directly, things become > expected, because it invokes: > > absl::base_internal::LowLevelCallOnce(&init_per_cpu_once, InitPerCpu); > > which AFAIU invokes InitPerCpu once after execution of the current program. Which > does: > > static bool InitThreadPerCpu() { > if (__rseq_refcount++ > 0) { > return true; > } > > auto ret = syscall(__NR_rseq, &__rseq_abi, sizeof(__rseq_abi), 0, > PERCPU_RSEQ_SIGNATURE); > if (ret == 0) { > return true; > } else { > __rseq_refcount--; > } > > return false; > } > > static void InitPerCpu() { > // Based on the results of successfully initializing the first thread, mark > // init_status to initialize all subsequent threads. > if (InitThreadPerCpu()) { > init_status = kFastMode; > } > } > > In a scenario where glibc has already registered Rseq, the __rseq_refcount will > be incremented, the __NR_rseq syscall will fail with -1, errno=EBUSY, so the refcount > will be immediately decremented and it will return false. Therefore, "init_status" will > never be set fo kFastMode, leaving it in kSlowMode for the entire lifetime of this > program. That being said, even though this state can come as a surprise, it seems to > be entirely bypassed by the fast-paths IsFast() and IsFastNoInit(), so maybe it won't > have any observable side-effects other than leaving init_status in a state that does not > match reality. I agree that this could potentially violate inviarants, but InitFastPerCpu is not intended to be called by the application. > In the other use-case where tcmalloc co-exist with a dlopened/dlclosed library, but glibc > does not provide Rseq registration, we run into issues as well if the dlopened library > registers rseq first for a given thread. The IsFastNoInit() expects that if Rseq has been > observed as registered in the past for a thread, it stays registered. However, if a > dlclosed library unregisters Rseq, we need to be prepared to re-register it. So either > tcmalloc needs to express its use of Rseq by incrementing __rseq_refcount even when Rseq > is registered (this would hurt the fast-path however, and I would hate to have to do this), > or tcmalloc needs to be able to handle the fact that Rseq may be unregistered by a dlclosed > library which was the actual owner of the Rseq registration. We have a bit of an opportunity to figure out whether this is the first time--from TCMalloc's perspective--a thread is doing per-CPU and bump the __rseq_count accordingly. I think this could be done off of the fast path. Chris