linux-kernel - Re: [PATCH 1/7] cleanup: Introduce DEFINE_ACQUIRE() a CLASS() for conditional locking

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250512185817.GA1808@noisy.programming.kicks-ass.net>
Date: Mon, 12 May 2025 20:58:17 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: dan.j.williams@...el.com
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
	David Lechner <dlechner@...libre.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	"Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	Jonathan Cameron <jonathan.cameron@...wei.com>,
	Dave Jiang <dave.jiang@...el.com>,
	Alison Schofield <alison.schofield@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH 1/7] cleanup: Introduce DEFINE_ACQUIRE() a CLASS() for
 conditional locking

On Mon, May 12, 2025 at 08:25:59PM +0200, Peter Zijlstra wrote:
> On Mon, May 12, 2025 at 12:50:26PM +0200, Peter Zijlstra wrote:
> 
> > +#define __GUARD_ERR(_ptr) \
> > +	({ long _rc = (__force unsigned long)(_ptr); \
> > +	   if (!_rc) { _rc = -EBUSY; } if (!IS_ERR_VALUE(_rc)) { _rc = 0; } \
> > +	   _rc; })
> > +
> 
> >  #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
> > -	DEFINE_CLASS(_name, _type, if (_T) { _unlock; }, ({ _lock; _T; }), _type _T); \
> > +	DEFINE_CLASS(_name, _type, if (!__GUARD_ERR(_T)) { _unlock; }, ({ _lock; _T; }), _type _T); \
> >  	DEFINE_CLASS_IS_GUARD(_name)
> 
> GCC is 'stupid' and this generates atrocious code. I'll play with it.

PRE:
    bf9e:       48 85 db                test   %rbx,%rbx
    bfa1:       74 1a                   je     bfbd <foo+0x5d>
    bfa3:       48 81 fb 00 f0 ff ff    cmp    $0xfffffffffffff000,%rbx
    bfaa:       77 11                   ja     bfbd <foo+0x5d>

POST:
    bf9e:       48 8d 43 ff             lea    -0x1(%rbx),%rax
    bfa2:       48 3d ff ef ff ff       cmp    $0xffffffffffffefff,%rax
    bfa8:       77 11                   ja     bfbb <foo+0x5b>

---
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -293,17 +293,18 @@ static inline class_##_name##_t class_##
 #define __DEFINE_CLASS_IS_CONDITIONAL(_name, _is_cond)	\
 static __maybe_unused const bool class_##_name##_is_conditional = _is_cond
 
-#define __GUARD_ERR(_ptr) \
-	({ long _rc = (__force unsigned long)(_ptr); \
-	   if (!_rc) { _rc = -EBUSY; } if (!IS_ERR_VALUE(_rc)) { _rc = 0; } \
-	   _rc; })
+#define __GUARD_IS_ERR(_ptr) \
+	({ unsigned long _rc = (__force unsigned long)(_ptr); \
+	   unlikely((_rc-1) >= -MAX_ERRNO-1); })
 
 #define __DEFINE_GUARD_LOCK_PTR(_name, _exp) \
 	static inline void * class_##_name##_lock_ptr(class_##_name##_t *_T) \
 	{ void *_ptr = (void *)(__force unsigned long)*(_exp); \
 	  if (IS_ERR(_ptr)) { _ptr = NULL; } return _ptr; } \
 	static inline int class_##_name##_lock_err(class_##_name##_t *_T) \
-	{ return __GUARD_ERR(*(_exp)); }
+	{ long _rc = (__force unsigned long)*(_exp); \
+	  if (!_rc) { _rc = -EBUSY; } if (!IS_ERR_VALUE(_rc)) { _rc = 0; } \
+	  return _rc; }
 
 #define DEFINE_CLASS_IS_GUARD(_name) \
 	__DEFINE_CLASS_IS_CONDITIONAL(_name, false); \
@@ -314,7 +315,7 @@ static __maybe_unused const bool class_#
 	__DEFINE_GUARD_LOCK_PTR(_name, _T)
 
 #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
-	DEFINE_CLASS(_name, _type, if (!__GUARD_ERR(_T)) { _unlock; }, ({ _lock; _T; }), _type _T); \
+	DEFINE_CLASS(_name, _type, if (!__GUARD_IS_ERR(_T)) { _unlock; }, ({ _lock; _T; }), _type _T); \
 	DEFINE_CLASS_IS_GUARD(_name)
 
 #define DEFINE_GUARD_COND_4(_name, _ext, _lock, _cond) \
@@ -406,7 +407,7 @@ typedef struct {							\
 									\
 static inline void class_##_name##_destructor(class_##_name##_t *_T)	\
 {									\
-	if (!__GUARD_ERR(_T->lock)) { _unlock; }			\
+	if (!__GUARD_IS_ERR(_T->lock)) { _unlock; }			\
 }									\
 									\
 __DEFINE_GUARD_LOCK_PTR(_name, &_T->lock)