From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D081C0218A for ; Tue, 28 Jan 2025 17:21:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8B1F1280240; Tue, 28 Jan 2025 12:21:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 88C88280222; Tue, 28 Jan 2025 12:21:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 735D6280240; Tue, 28 Jan 2025 12:21:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 533A1280222 for ; Tue, 28 Jan 2025 12:21:45 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 0034D1C69C3 for ; Tue, 28 Jan 2025 17:21:43 +0000 (UTC) X-FDA: 83057527686.24.81044A0 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf10.hostedemail.com (Postfix) with ESMTP id BC101C001B for ; Tue, 28 Jan 2025 17:21:41 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=RS36gzQL; dkim=pass header.d=linutronix.de header.s=2020e header.b=Ka5PDexh; spf=pass (imf10.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738084902; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=MOJUlAo7lz1CwdMKpv46rlAJOwnNrK33eU6Dw3GvpRY=; b=FYX3hFpc3FYRWjq3SQdUupwhOGAhO8VfASZNq/pkWziJqOUZ5jn6aF9CQYGX6OC6Wcnpkl woX/O0inrqUIG1YWHvJ11tixsqf5xHfUKTrcWo7P6AMk7f0YsyQINGmXRdsJLJyCMGKP80 HjjuzUg6wOs2EMjdfbBgnUFPj/u2/9M= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=RS36gzQL; dkim=pass header.d=linutronix.de header.s=2020e header.b=Ka5PDexh; spf=pass (imf10.hostedemail.com: domain of bigeasy@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=bigeasy@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738084902; a=rsa-sha256; cv=none; b=u+BvmMs5Mq12ECC0Y//PiOND9Q6GQJCmIyRkqqSERbVyLUmHIKkySUXi5mx41NaxXf8hO2 oTjXd8Y1zZgVnEFxw8+dfAwZXCURlNwE1d+ba0bZhl1iRlTbKSGyRvQm7O/pr1AsCROrOa Cda79S+/HF4fo7jOT505JQNS5JE40eI= Date: Tue, 28 Jan 2025 18:21:37 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1738084899; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MOJUlAo7lz1CwdMKpv46rlAJOwnNrK33eU6Dw3GvpRY=; b=RS36gzQLNYLvo/gP8Pu3Qw+zafkf6Vyzb3rQz+36Z4CDthljPupCFikNp0MNMl1AXuWZ+/ 7/a5KjvD28hNar6vx4iMVRRGQYN5IC0AwDuMARcCxXjdlzfc2Ao1R6hw0Di7TXrW5bbEYK Yh4Ye2v9lheN+5fJMdaOTpRXxnhici02bM7H+1bgxKlxL5nKycx+/+E4dztIAVpcCgRcYf vhrupS1PipeRLed5lz29oo+RJBNjh1GklFQ/nJXBHwRmj0+YHpiBxj4xM1XtfI03pH5ux+ pv0jIw8/7ffpViTNzV7aXhSWWt5Cfkwi89J9XeUMV9tzEMIAtJehdaRKz3BUkw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1738084899; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MOJUlAo7lz1CwdMKpv46rlAJOwnNrK33eU6Dw3GvpRY=; b=Ka5PDexhtF5JhuaPwa5lr6lrqXmXclnV44mzF0L9JhPN2+u5ZT5ueMiRFKfP0WgONYda9G qIi5tstpIZoOTuCA== From: Sebastian Andrzej Siewior To: Alexei Starovoitov Cc: bpf@vger.kernel.org, andrii@kernel.org, memxor@gmail.com, akpm@linux-foundation.org, peterz@infradead.org, vbabka@suse.cz, rostedt@goodmis.org, houtao1@huawei.com, hannes@cmpxchg.org, shakeel.butt@linux.dev, mhocko@suse.com, willy@infradead.org, tglx@linutronix.de, jannh@google.com, tj@kernel.org, linux-mm@kvack.org, kernel-team@fb.com Subject: Re: [PATCH bpf-next v6 3/6] locking/local_lock: Introduce local_trylock_t and local_trylock_irqsave() Message-ID: <20250128172137.bLPGqHth@linutronix.de> References: <20250124035655.78899-1-alexei.starovoitov@gmail.com> <20250124035655.78899-4-alexei.starovoitov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20250124035655.78899-4-alexei.starovoitov@gmail.com> X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: BC101C001B X-Stat-Signature: 7upwse9ua7yi8tsdedecco7bmfetxnjs X-Rspam-User: X-HE-Tag: 1738084901-515294 X-HE-Meta: U2FsdGVkX19irx3Vb+yyL1phsYa5dt84+blgMZSdKe3ezg1+inuz3alD/xym+CP3aXAcS69FCNov7Ai0oInXShTZKMN68qyoXsobIdmkSeTWpUSf81Hb8kbOvMuqdd3b9DsyZDqELswhI8jsDTwtquM/oBbGUZ3xfbW37P4rNHzkHBoTKBTrTtdo9iRJUj0YY4ISZoqsKTawVloBzvKLMX4xJ+Sfxyh6OnBi256s0AnMhZJiq2Uzbu0Vq4mNMWIpkwFJipWQ0SQMTEhqrJS5mvtXvgOwvB+4hyEKBKPoUb+lixDU6lvaxyzId//U1+nNOBqYJ/2EDx53TmzT6HTOO7W9pJFD/dD5DTbwK9FBdyn7HfRMpVdHlHuPVfcutoPGBaqEUlfOuLlw+LOx0PLOhnJ1hJLtdY4iZN6V5HzvFN3/cTVNk3Jkg5tl6c7G6nOsGmyUAZAXycNChHtRdF2ajjQmpkY/48TUvfKFTIIU519ke/DUYpG8OMHc5lsV6WEldyWuR/zRgWfaPljrouwucqC7Q9C17obpZiXHZqTMP7oVFrzpjqdLgOqZLoryDVkDsbqAFl0/SQgPN05YfKApHqBL9je56ICbheiHccrYgGuApLAhzuWOVa0U5rXG0Xy9QYl4IX2VI7fFVcVsrVyR/28zlZIQko1zFb9jcORTZc3OTtt33ko8Yh9l7OwyXYkwtMMmyMIdlsZ4EsfPLHpZ0qLjDYHw8IH9JBUTHT6dXCh3Wjy9VNK/6ILa1QOUPKraRqskMdoGyKDyutoVqPc5CWltwCSzR8hRcE9bWyOLO1O3zRdgOq7gPXhab0zSeM0GcpMkxLfDYozjRaAGrJZVBmDMln6R8C2SAZV+HBSMZThF3+NEGcynZpXzpiuL9DmiEUSumbj083Vd155MiLq/3yIpCDxttp0rm8vkluPeaRwyWMs1W5v8hWt59iJHryYmOUum/oHm4JQM/GtWTQC qmqpkEfb W6nUnXhncWXWm9kGWJ8m1J3lf7+8GK3mckVzpnZeBXTtx0LuJyQAe56cmypEMGFS9ly7/dhUnkTNUQG6W1/LfHymyQ/Edx+r46G5WgPIabRKyOIRNBifRlj5nrciSABf+sc15dLdzhiaHzbFp5m640w7xvYsTiufnBvy7glUC+Y1PDllNZ7mbtsPHtw6FCdcXLrixIiS5QNBmP3Z74uaXo2IFiSoFqV94oX0DNCn1m4fVyFaRPiNcFl2424eQl5/Fa5rrDaPV/lSkiCsddowdDKmdKJv7PuFR+Fy5gx8/TvSNTmh9IF3oOrgirw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2025-01-23 19:56:52 [-0800], Alexei Starovoitov wrote: > Usage: > > local_lock_t lock; // sizeof(lock) == 0 in !RT > local_lock_irqsave(&lock, ...); // irqsave as before > if (local_trylock_irqsave(&lock, ...)) // compilation error > > local_trylock_t lock; // sizeof(lock) == 4 in !RT > local_lock_irqsave(&lock, ...); // irqsave and active = 1 > if (local_trylock_irqsave(&lock, ...)) // if (!active) irqsave so I've been looking at this for a while and I don't like the part where the type is hidden away. It is then casted back. So I tried something with _Generics but then the existing guard implementation complained. Then I asked myself why do we want to hide much of the implementation and not make it obvious. So I made a few macros to hide most of the implementation for !RT. Later I figured if the variable is saved locally then I save one this_cpu_ptr invocation. So I wrote it out and the snippet below is what I have. is this anywhere near possible to accept? diff --git a/include/linux/local_lock.h b/include/linux/local_lock.h index 091dc0b6bdfb9..05c254a5d7d3e 100644 --- a/include/linux/local_lock.h +++ b/include/linux/local_lock.h @@ -51,6 +51,65 @@ #define local_unlock_irqrestore(lock, flags) \ __local_unlock_irqrestore(lock, flags) +/** + * localtry_lock_init - Runtime initialize a lock instance + */ +#define localtry_lock_init(lock) __localtry_lock_init(lock) + +/** + * localtry_lock - Acquire a per CPU local lock + * @lock: The lock variable + */ +#define localtry_lock(lock) __localtry_lock(lock) + +/** + * localtry_lock_irq - Acquire a per CPU local lock and disable interrupts + * @lock: The lock variable + */ +#define localtry_lock_irq(lock) __localtry_lock_irq(lock) + +/** + * localtry_lock_irqsave - Acquire a per CPU local lock, save and disable + * interrupts + * @lock: The lock variable + * @flags: Storage for interrupt flags + */ +#define localtry_lock_irqsave(lock, flags) \ + __localtry_lock_irqsave(lock, flags) + +/** + * localtry_trylock_irqsave - Try to acquire a per CPU local lock, save and disable + * interrupts if acquired + * @lock: The lock variable + * @flags: Storage for interrupt flags + * + * The function can be used in any context such as NMI or HARDIRQ. Due to + * locking constrains it will _always_ fail to acquire the lock on PREEMPT_RT. + */ +#define localtry_trylock_irqsave(lock, flags) \ + __localtry_trylock_irqsave(lock, flags) + +/** + * local_unlock - Release a per CPU local lock + * @lock: The lock variable + */ +#define localtry_unlock(lock) __localtry_unlock(lock) + +/** + * local_unlock_irq - Release a per CPU local lock and enable interrupts + * @lock: The lock variable + */ +#define localtry_unlock_irq(lock) __localtry_unlock_irq(lock) + +/** + * localtry_unlock_irqrestore - Release a per CPU local lock and restore + * interrupt flags + * @lock: The lock variable + * @flags: Interrupt flags to restore + */ +#define localtry_unlock_irqrestore(lock, flags) \ + __localtry_unlock_irqrestore(lock, flags) + DEFINE_GUARD(local_lock, local_lock_t __percpu*, local_lock(_T), local_unlock(_T)) diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h index 8dd71fbbb6d2b..789b0d878e6c5 100644 --- a/include/linux/local_lock_internal.h +++ b/include/linux/local_lock_internal.h @@ -15,6 +15,11 @@ typedef struct { #endif } local_lock_t; +typedef struct { + local_lock_t llock; + unsigned int acquired; +} localtry_lock_t; + #ifdef CONFIG_DEBUG_LOCK_ALLOC # define LOCAL_LOCK_DEBUG_INIT(lockname) \ .dep_map = { \ @@ -50,6 +55,7 @@ static inline void local_lock_debug_init(local_lock_t *l) { } #endif /* !CONFIG_DEBUG_LOCK_ALLOC */ #define INIT_LOCAL_LOCK(lockname) { LOCAL_LOCK_DEBUG_INIT(lockname) } +#define INIT_LOCALTRY_LOCK(lockname) { .llock = { LOCAL_LOCK_DEBUG_INIT(lockname.llock) }} #define __local_lock_init(lock) \ do { \ @@ -118,6 +124,86 @@ do { \ #define __local_unlock_nested_bh(lock) \ local_lock_release(this_cpu_ptr(lock)) +/* localtry_lock_t variants */ + +#define __localtry_lock_init(lock) \ +do { \ + __local_lock_init(&(lock)->llock); \ + WRITE_ONCE(&(lock)->acquired, 0); \ +} while (0) + +#define __localtry_lock(lock) \ + do { \ + localtry_lock_t *lt; \ + preempt_disable(); \ + lt = this_cpu_ptr(lock); \ + local_lock_acquire(<->llock); \ + WRITE_ONCE(lt->acquired, 1); \ + } while (0) + +#define __localtry_lock_irq(lock) \ + do { \ + localtry_lock_t *lt; \ + local_irq_disable(); \ + lt = this_cpu_ptr(lock); \ + local_lock_acquire(<->llock); \ + WRITE_ONCE(lt->acquired, 1); \ + } while (0) + +#define __localtry_lock_irqsave(lock, flags) \ + do { \ + localtry_lock_t *lt; \ + local_irq_save(flags); \ + lt = this_cpu_ptr(lock); \ + local_lock_acquire(<->llock); \ + WRITE_ONCE(lt->acquired, 1); \ + } while (0) + +#define __localtry_trylock_irqsave(lock, flags) \ + ({ \ + localtry_lock_t *lt; \ + bool _ret; \ + \ + local_irq_save(flags); \ + lt = this_cpu_ptr(lock); \ + if (!READ_ONCE(lt->acquired)) { \ + local_lock_acquire(<->llock); \ + WRITE_ONCE(lt->acquired, 1); \ + _ret = true; \ + } else { \ + _ret = false; \ + local_irq_restore(flags); \ + } \ + _ret; \ + }) + +#define __localtry_unlock(lock) \ + do { \ + localtry_lock_t *lt; \ + lt = this_cpu_ptr(lock); \ + WRITE_ONCE(lt->acquired, 0); \ + local_lock_release(<->llock); \ + preempt_enable(); \ + } while (0) + +#define __localtry_unlock_irq(lock) \ + do { \ + localtry_lock_t *lt; \ + lt = this_cpu_ptr(lock); \ + WRITE_ONCE(lt->acquired, 0); \ + local_lock_release(<->llock); \ + local_irq_enable(); \ + } while (0) + +#define __localtry_unlock_irqrestore(lock, flags) \ + do { \ + localtry_lock_t *lt; \ + lt = this_cpu_ptr(lock); \ + WRITE_ONCE(lt->acquired, 0); \ + local_lock_release(<->llock); \ + local_irq_restore(flags); \ + } while (0) + #else /* !CONFIG_PREEMPT_RT */ /* @@ -125,8 +211,10 @@ do { \ * critical section while staying preemptible. */ typedef spinlock_t local_lock_t; +typedef spinlock_t localtry_lock_t; #define INIT_LOCAL_LOCK(lockname) __LOCAL_SPIN_LOCK_UNLOCKED((lockname)) +#define INIT_LOCALTRY_LOCK(lockname) INIT_LOCAL_LOCK(lockname) #define __local_lock_init(l) \ do { \ @@ -169,4 +257,31 @@ do { \ spin_unlock(this_cpu_ptr((lock))); \ } while (0) +/* localtry_lock_t variants */ + +#define __localtry_lock_init(lock) __local_lock_init(lock) +#define __localtry_lock(lock) __local_lock(lock) +#define __localtry_lock_irq(lock) __local_lock(lock) +#define __localtry_lock_irqsave(lock, flags) __local_lock_irqsave(lock, flags) +#define __localtry_unlock(lock) __local_unlock(lock) +#define __localtry_unlock_irq(lock) __local_unlock(lock) +#define __localtry_unlock_irqrestore(lock, flags) __local_unlock_irqrestore(lock, flags) + +#define __localtry_trylock_irqsave(lock, flags) \ + ({ \ + int __locked; \ + \ + typecheck(unsigned long, flags); \ + flags = 0; \ + if (in_nmi() | in_hardirq()) { \ + __locked = 0; \ + } else { \ + migrate_disable(); \ + __locked = spin_trylock(this_cpu_ptr((lock))); \ + if (!__locked) \ + migrate_enable(); \ + } \ + __locked; \ + }) + #endif /* CONFIG_PREEMPT_RT */