From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030351AbcBZRrP (ORCPT ); Fri, 26 Feb 2016 12:47:15 -0500 Received: from mail.efficios.com ([78.47.125.74]:41063 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933602AbcBZRrM convert rfc822-to-8bit (ORCPT ); Fri, 26 Feb 2016 12:47:12 -0500 Date: Fri, 26 Feb 2016 17:47:05 +0000 (UTC) From: Mathieu Desnoyers To: Rasmus Villemoes Cc: Thomas Gleixner , Andrew Morton , Russell King , Ingo Molnar , "H. Peter Anvin" , linux-kernel@vger.kernel.org, linux-api , Paul Turner , Andrew Hunter , Peter Zijlstra , Andy Lutomirski , Andi Kleen , Dave Watson , Chris Lameter , Ben Maurer , rostedt , "Paul E. McKenney" , Josh Triplett , Linus Torvalds , Catalin Marinas , Will Deacon , Michael Kerrisk Message-ID: <668290565.8970.1456508825358.JavaMail.zimbra@efficios.com> In-Reply-To: <87egc0l62k.fsf@rasmusvillemoes.dk> References: <1456270120-7560-1-git-send-email-mathieu.desnoyers@efficios.com> <1456270120-7560-2-git-send-email-mathieu.desnoyers@efficios.com> <87egc0l62k.fsf@rasmusvillemoes.dk> Subject: Re: [PATCH v4 1/5] getcpu_cache system call: cache CPU number of running thread MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Originating-IP: [78.47.125.74] X-Mailer: Zimbra 8.6.0_GA_1178 (ZimbraWebClient - FF44 (Linux)/8.6.0_GA_1178) Thread-Topic: getcpu_cache system call: cache CPU number of running thread Thread-Index: zQLqTm+cr0dnDK+l7+jYi67RKUCmZw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- On Feb 25, 2016, at 6:32 PM, Rasmus Villemoes linux@rasmusvillemoes.dk wrote: > On Wed, Feb 24 2016, Mathieu Desnoyers wrote: > >> >> Typically, a library or application will keep the cpu number >> cache in a thread-local storage variable, or other memory >> areas belonging to each thread. It is recommended to perform >> a volatile read of the cpu number cache to prevent the com‐ >> piler from doing load tearing. An alternative approach is to >> read the cpu number cache from inline assembly in a single >> instruction. >> >> Each thread is responsible for registering its own cpu number >> cache. Only one cpu cache address can be registered per >> thread. >> >> The symbol __getcpu_cache_tls is recommended to be used >> across libraries and applications wishing to register a >> thread-local getcpu_cache. The attribute "weak" is recom‐ >> mended when declaring this variable in libraries. Applica‐ >> tions can choose to define their own version of this symbol >> without the weak attribute as a performance improvement. >> >> In a typical usage scenario, the thread registering the cpu >> number cache will be performing reads from that cache. It is >> however also allowed to read the cpu number cache from other >> threads. The cpu number cache updates performed by the kernel >> provide single-copy atomicity semantics, which guarantee that >> other threads performing single-copy atomic reads of the cpu >> number cache will always observe a consistent value. >> >> Memory registered as cpu number cache should never be deallo‐ >> cated before the thread which registered it exits: specifi‐ >> cally, it should not be freed, and the library containing the >> registered thread-local storage should not be dlclose'd. > > Maybe spell out the consequence if this is violated - since the SIGSEGV > only happens on migration, it may take a while to strike. Good point. > > Random thoughts: The current implementation ensures that getcpu_cache is > "idempotent" from within a single thread - once set, it can never get > unset nor set to some other pointer. I think that can be useful, since > it means a library can reliably use the TLS variable itself (initialized > with some negative number) as an indicator of whether > getcpu_cache(GETCPU_CACHE_SET) has been called. So if a single test on a > fast path where the library would need to load __getcpu_cache_tls anyway > is acceptable, it can avoid requiring some library init function to be > called in each thread - which can sometimes be hard to arrange. Is this > something we want to guarantee - that is, will we never implement > GETCPU_CACHE_UNSET or a "force" flag to _SET? Either way, I think we > should spend a few words on it to avoid the current behaviour becoming > accidental ABI. Yes, I would be tempted to state that once set, the address is idempotent for a thread. > > In another thread: > >> However, there are other use-cases for having a fast mechanism for >> reading the current CPU number, besides restartable sequences. For >> instance, it can be used by glibc to implement a faster sched_getcpu. > > Will glibc do that? It may be a little contentious for glibc to claim a > unique resource such as task_struct::cpu_cache for itself, even if > everybody is supposed to use the same symbol. Hm, maybe one could say > that if an application does define the symbol __getcpu_cache_tls (which > is techically in the implementation namespace), that gives glibc (and > any other library) license to do getcpu_cache(SET, &&__getcpu_cache_tls) > (pseudo-code, of course). If a library initializes its own weak version > with -2 it can check whether the application defined > __getcpu_cache_tls. Ok, I'm probably overthinking this... I've had the exact same thoughts a few days ago then thinking about how lttng-ust could do a "lazy binding" of the getcpu_cache without requiring an explicit initialization at thread start. We're reaching very similar conclusions. We could recommend/require that userspace does this whenever it defines a __getcpu_cache_tls: Declare as __thread __attribute__((weak)) volatile int32_t __getcpu_cache_tls = -1; Then whenever it loads it, "-1" would mean "uninitialized", and "-2" could mean "this thread tried to initialize it, but fail, so you should directly go to a fallback". ">= 0" would mean initialized and working. static inline int32_t getcpu_cache_read(void) { int32_t cachev = __getcpu_cache_tls; if (likely(cachev >= 0)) return cachev; if (cachev == -1) { volatile int32_t *cpu_cache = &__getcpu_cache_tls; if (!getcpu_cache(GETCPU_CACHE_SET, &cpu_cache, 0)) return __getcpu_cache_tls; __getcpu_cache_tls = -2; } /* Fallback on sched_getcpu(). */ return sched_getcpu(); } This could be documented in the getcpu_cache system call man page. Thoughts ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com