linux-kernel - Re: [RFC PATCH] local_lock: Add local_lock access for a CPU-local pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250425091443.2HMvQfPv@linutronix.de>
Date: Fri, 25 Apr 2025 11:14:43 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Waiman Long <llong@...hat.com>
Cc: linux-kernel@...r.kernel.org, Boqun Feng <boqun.feng@...il.com>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>, Will Deacon <will@...nel.org>
Subject: Re: [RFC PATCH] local_lock: Add local_lock access for a CPU-local
 pointer

On 2025-04-18 22:52:29 [-0400], Waiman Long wrote:
> > Adding a _local to the function name would be a bit too local. I added
> > _this instead but don't like it very much. Anyone with a better naming?
> 
> The "this" suffix looks a bit weird. Since you had introduced localtry_lock
> before, maybe you can follow a similar scheme like localcpu_lock.
> 
> My 2 cents.

Okay. Better.
We usually have function() and __function() which is the internal
implementation with some changes/ presets. Now if we apply this here
and shift the this_cpu_ptr() from __ to main one, like:

diff --git a/include/linux/local_lock.h b/include/linux/local_lock.h
index 1a0bc35839e36..d5e8c7a298055 100644
--- a/include/linux/local_lock.h
+++ b/include/linux/local_lock.h
@@ -133,10 +133,10 @@ DEFINE_LOCK_GUARD_1(local_lock_irqsave, local_lock_t __percpu,
 		    unsigned long flags)
 
 #define local_lock_nested_bh(_lock)				\
-	__local_lock_nested_bh(_lock)
+	__local_lock_nested_bh(this_cpu_ptr(_lock))
 
 #define local_unlock_nested_bh(_lock)				\
-	__local_unlock_nested_bh(_lock)
+	__local_unlock_nested_bh(this_cpu_ptr(_lock))
 
 DEFINE_GUARD(local_lock_nested_bh, local_lock_t __percpu*,
 	     local_lock_nested_bh(_T),
diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
index 67bd13d142fac..bc6e6cc5dca99 100644
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -126,11 +126,11 @@ do {								\
 #define __local_lock_nested_bh(lock)				\
 	do {							\
 		lockdep_assert_in_softirq();			\
-		local_lock_acquire(this_cpu_ptr(lock));	\
+		local_lock_acquire(lock);	\
 	} while (0)
 
 #define __local_unlock_nested_bh(lock)				\
-	local_lock_release(this_cpu_ptr(lock))
+	local_lock_release(lock)
 
 /* localtry_lock_t variants */
 
@@ -275,12 +275,12 @@ typedef spinlock_t localtry_lock_t;
 #define __local_lock_nested_bh(lock)				\
 do {								\
 	lockdep_assert_in_softirq_func();			\
-	spin_lock(this_cpu_ptr(lock));				\
+	spin_lock(lock);				\
 } while (0)
 
 #define __local_unlock_nested_bh(lock)				\
 do {								\
-	spin_unlock(this_cpu_ptr((lock)));			\
+	spin_unlock((lock));			\
 } while (0)
 
 /* localtry_lock_t variants */


Then I could use __local_lock_nested_bh(lock) where "lock" is already
the actual lock pointer.

> Cheers,
> Longman

Sebastian