Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Borislav Petkov" <bp@alien8.de>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	x86@kernel.org, "Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>, "Paul Walmsley" <pjw@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"André Almeida" <andrealmeid@igalia.com>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Samuel Holland" <samuel.holland@sifive.com>,
	"Charlie Jenkins" <thecharlesjenkins@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Sean Christopherson" <seanjc@google.com>,
	"Jisheng Zhang" <jszhang@kernel.org>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Sven Schnelle" <svens@linux.ibm.com>
Subject: Re: [PATCH v5 8/8] futex: Use runtime constants for __futex_hash() hot path
Date: Wed, 1 Jul 2026 21:58:42 +0200	[thread overview]
Message-ID: <20260701195842.GKzb73-i@linutronix.de> (raw)
In-Reply-To: <20260630045531.3939-9-kprateek.nayak@amd.com>

On 2026-06-30 04:55:31 [+0000], K Prateek Nayak wrote:
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -395,13 +391,13 @@ __futex_hash(union futex_key *key, struct futex_private_hash *fph, struct futex_
>  		 * NOTE: this isn't perfectly uniform, but it is fast and
>  		 * handles sparse node masks.
>  		 */
> -		node = (hash >> futex_hashshift) % nr_node_ids;
> +		node = runtime_const_shift_right_32(hash, __futex_shift) % nr_node_ids;
>  		if (!node_possible(node)) {
>  			node = find_next_bit_wrap(node_possible_map.bits, nr_node_ids, node);
>  		}

I replaced this with:

diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 79e770d4d166..30d8622958d2 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -382,6 +382,7 @@ __futex_hash(union futex_key *key, struct futex_private_hash *fph, struct futex_
 		      key->both.offset);
 
 	if (node == FUTEX_NO_NODE) {
+		u32 node_limit = nr_node_ids;
 		/*
 		 * In case of !FLAGS_NUMA, use some unused hash bits to pick a
 		 * node -- this ensures regular futexes are interleaved across
@@ -391,9 +392,9 @@ __futex_hash(union futex_key *key, struct futex_private_hash *fph, struct futex_
 		 * NOTE: this isn't perfectly uniform, but it is fast and
 		 * handles sparse node masks.
 		 */
-		node = runtime_const_shift_right_32(hash, __futex_shift) % nr_node_ids;
-		if (!node_possible(node)) {
-			node = find_next_bit_wrap(node_possible_map.bits, nr_node_ids, node);
+		node = reciprocal_scale(hash, node_limit);
+		if (!node_possible(node)) {
+			node = find_next_bit_wrap(node_possible_map.bits, node_limit, node);
 		}
 	}
 
I don't think it is worse, I hardly see a change perf wise. Sometimes
op/s is reported almost unchanged, sometimes it improves a bit.

What it does it reads nr_node_ids only once (which has no effect here
because I have no sparse node) and it replaces the shift + divl with
imulq + shift.

perf was pointing me to the divl but now it points to the imulq.
¯\_(ツ)_/¯

But having that div gone, can't be bad, can it?

Sebastian


      parent reply	other threads:[~2026-07-01 19:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30  4:55 [PATCH v5 0/8] futex: Use runtime constants for futex_hash computation K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 1/8] x86/runtime-const: Introduce runtime_const_mask_32() K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 2/8] arm64/runtime-const: Use aarch64_insn_patch_text_nosync() for patching K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 3/8] arm64/runtime-const: Introduce runtime_const_mask_32() K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 4/8] riscv/runtime-const: Replace open-coded placeholder with RUNTIME_MAGIC K Prateek Nayak
2026-06-30  6:47   ` Guo Ren
2026-06-30  4:55 ` [PATCH v5 5/8] riscv/runtime-const: Introduce runtime_const_mask_32() K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 6/8] s390/runtime-const: " K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 7/8] asm-generic/runtime-const: Add dummy runtime_const_mask_32() K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 8/8] futex: Use runtime constants for __futex_hash() hot path K Prateek Nayak
2026-07-01  7:57   ` Peter Zijlstra
2026-07-01  8:41     ` Sebastian Andrzej Siewior
2026-07-01  9:07       ` K Prateek Nayak
2026-07-01 16:17         ` [PATCH] futex: Optimise the size check get_futex_key() Sebastian Andrzej Siewior
2026-07-01 11:01       ` [PATCH v5 8/8] futex: Use runtime constants for __futex_hash() hot path Sebastian Andrzej Siewior
2026-07-01 19:58   ` Sebastian Andrzej Siewior [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260701195842.GKzb73-i@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=agordeev@linux.ibm.com \
    --cc=alex@ghiti.fr \
    --cc=andrealmeid@igalia.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=arnd@arndb.de \
    --cc=borntraeger@linux.ibm.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jszhang@kernel.org \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=peterz@infradead.org \
    --cc=pjw@kernel.org \
    --cc=samuel.holland@sifive.com \
    --cc=seanjc@google.com \
    --cc=svens@linux.ibm.com \
    --cc=tglx@kernel.org \
    --cc=thecharlesjenkins@gmail.com \
    --cc=thuth@redhat.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox