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 523111F619 for ; Wed, 26 Feb 2020 19:12:58 +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=jLRS 6iasly+QfMJ1ZbuOGT3af61N/t1XDexKWuut28hPqqFC7IudLtCiIBi5RlFuuV9X hGANUOF/w4MX3WCcdH48MBcWMTl67ILlIKEvpmrjsvUygGFv5lqJiPsKxxxNqS87 PZLnGqA26ghC5iFnYOFYwdQC8IhPJUYRea2t2pY= 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=NByeLY5hOg mupSZfqtOqq+viMpc=; b=hpz4aLGWwilvE3yZGYV7F6ZZ/eStMas9dcYD5SXt8O Qus7SUNFOFD8+G/frWUTiRfJQtKv5VkdBmFIa4D5ealJ2gQ0SnFNDwkwuWyQw28O QmuI2FpTJaT29bI6ips3+DX4FjyJVymtbEPabjAdbWQ78Uj4gDdQVjMAk/2x84q1 g= Received: (qmail 32422 invoked by alias); 26 Feb 2020 19:12:56 -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 32408 invoked by uid 89); 26 Feb 2020 19:12:55 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-wm1-f49.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=c0D5zQRFMYYKGJDzA+8Wd1v+wvzeFmhIB9Cp0LJFiSM=; b=dh8IYfg6/tgPHa59ohPah4F0J933V0LOdiqkQTFJuTZ3O6Ak3KVZgmM7wec2tVUIXX jh3VD4l5u9LVcUpVCQkbMPr0dIYNpzpFJKRgmjapZOdvbM3d2OtJS+BVRWGZ3rCS6mDO 0ny0MzkPxTCN4F3RJW/hkLa7Fcx6eg0BmxXqaGKzoF2lkL21WBJWj4XUjB2Ji/27hr9g NLXgVOCBlBcbfj8bJ4PHubPMW1MLiszHdBn/SmHmqvtZ1lY8AKQkHUKymDokRp2hHKF2 LTFoZHySKIP6Gg4wc0sw4kqVIq/Zno6sXMRvVLhrwEjOYtXJYf2xtCCktqyNtSEe1Xsh oryQ== 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> <982202794.8791.1582743392060.JavaMail.zimbra@efficios.com> In-Reply-To: <982202794.8791.1582743392060.JavaMail.zimbra@efficios.com> From: Chris Kennelly Date: Wed, 26 Feb 2020 14:12:37 -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 1:56 PM Mathieu Desnoyers wrote: > > ----- On Feb 26, 2020, at 12:27 PM, Chris Kennelly ckennelly@google.com wrote: > > > 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. > > OK, explicitly documenting this would be a good thing. In my own projects, > I prefix those symbols with double-underscores (__) to indicate that those > are not meant to be called by other means than the static inlines in the API. > > There may be use-cases which justify exposing InitFastPerCpu as a public API for > applications though, especially for those which require some level of > real-time guarantees from the malloc/free APIs. I've run into this situation > with liburcu which I maintain. > > > > >> 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. > > Is there an explicit tcmalloc API call that each thread need to do before starting > to use tcmalloc to allocate and free memory ? If not, you'll probably need to add > at least a load of __rseq_refcount (or some other TLS variable), test and conditional > branch on the fast-path, which is an additional cost I would ideally prefer to avoid. > Or do you have something else in mind ? No explicit call is necessary. This is something that can be done in the slow path, since we can recognize the transition from slow -> fast path for that thread