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

Message-ID: <20250517101742.4c38830b@pumpkin>
Date: Sat, 17 May 2025 10:17:42 +0100
From: David Laight <david.laight.linux@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Al Viro <viro@...iv.linux.org.uk>, Peter Zijlstra
 <peterz@...radead.org>, dan.j.williams@...el.com,
 linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org, David Lechner
 <dlechner@...libre.com>, 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 Tue, 13 May 2025 14:28:37 -0700
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Tue, 13 May 2025 at 13:31, Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > Nevermind - should've read back through the thread for context.  
> 
> Well, your comment did make me test what I can make gcc generate..
> 
> I still can't get gcc to do
> 
>        cmpq $-4095,%rdi
>         jns     .L189
> 
> for IS_ERR_OR_NULL() however hard I try.
> 
> The best I *can* get both gcc and clang to at least do
> 
>         movq    %rdi, %rcx
>         addq    $4095, %rcx
>         jns     .L189
> 
> which I suspect it much better than the "lea+cmpq", because a pure
> register move is handled by the renaming and has no cost aside from
> the front end (ie decoding).
> 
> So
> 
>   #define IS_ERR_OR_NULL(ptr) (MAX_ERRNO + (long)(ptr) >= 0)
> 
> does seem to be potentially something we could use, and maybe we could
> push the compiler people to realize that their current code generation
> is bad.
> 
> Of course, it doesn't actually *really* work for IS_ERR_OR_NULL(),
> because it gets the wrong results for user pointers, and while the
> *common* case for the kernel is to test various kernel pointers, the
> user pointer case does happen (ie mmap() and friends).
> 
> IOW, it's not actually correct in that form, I just wanted to see what
> we could do for some limited form of this common pattern.
> 
> Anyway, I am surprised that neither gcc nor clang seem to have
> realized that you can turn an "add" that just checks the condition
> codes for sign or equality into a "cmp" of the negative value.
> 
> It seems such a trivial and obvious thing to do. But maybe I'm
> confused and am missing something.

Doing the signed compare (long)(ptr) >= -MAX_ERRNO generates cmp + jl
(sign != overflow) which is a better test.

To let user pointers through it might be possible to generate:
	leaq	-1(%reg), %reg
	cmpq	$-4097, %reg
	leaq	1(%reg), %reg
	ja	label
which trades a register for an instruction.
It wouldn't be too bad if the second 'leaq' is moved to the branch
target - especially for any cpu that don't have inc/dec that doesn't
affect the flags.

	David