Linux s390 Architecture development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "K Prateek Nayak" <kprateek.nayak@amd.com>
Cc: linux-s390@vger.kernel.org, "Heiko Carstens" <hca@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>
Subject: Re: [PATCH v5 8/8] futex: Use runtime constants for __futex_hash() hot path
Date: Tue, 30 Jun 2026 05:20:13 +0000	[thread overview]
Message-ID: <20260630052014.C7D6A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260630045531.3939-9-kprateek.nayak@amd.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Accessing the runtime constant accessor `futex_queues()` immediately after `runtime_const_init()` in `futex_init()` causes a deterministic kernel crash during boot due to compiler hoisting and pipeline hazards.
--

commit e05e661cc5e69b283700fa99f0da313e4673aa43
Author: Peter Zijlstra <peterz@infradead.org>

futex: Use runtime constants for __futex_hash() hot path

This commit converts the read-only after init data __futex_shift,
__futex_mask, and __futex_queues used in the __futex_hash() hot path to
runtime constants. It also dynamically allocates __futex_queues based on
nr_node_ids instead of a static maximum. Runtime constants are initialized
in futex_init() using runtime_const_init().

Link: https://patch.msgid.link/20260227161841.GH606826@noisy.programming.kicks-ass.net

> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c

[ ... ]

> @@ -2020,10 +2016,19 @@ static int __init futex_init(void)
>  	hashsize = max(4, hashsize);
>  	hashsize = roundup_pow_of_two(hashsize);
>  #endif
> -	futex_hashshift = ilog2(hashsize);
> +	__futex_mask = hashsize - 1;
> +	__futex_shift = ilog2(hashsize);
>  	size = sizeof(struct futex_hash_bucket) * hashsize;
>  	order = get_order(size);
>  
> +	__futex_queues = kcalloc(nr_node_ids, sizeof(*__futex_queues), GFP_KERNEL);
> +
> +	runtime_const_init(shift, __futex_shift);
> +	runtime_const_init(mask,  __futex_mask);
> +	runtime_const_init(ptr,   __futex_queues);
> +
> +	BUG_ON(!futex_queues());
> +
>  	for_each_node(n) {
>  		struct futex_hash_bucket *table;
>  
> @@ -2037,10 +2042,9 @@ static int __init futex_init(void)
>  		for (i = 0; i < hashsize; i++)
>  			futex_hash_bucket_init(&table[i]);
>  
> -		futex_queues[n] = table;
> +		futex_queues()[n] = table;

[Severity: Critical]
Does using the futex_queues() accessor here in futex_init() right after
runtime_const_init() cause a boot crash?

Since runtime_const_ptr() expands to an asm_inline block with no memory
dependencies or volatile qualifiers, the compiler might treat it as a pure
computation. This means the compiler is free to hoist the evaluation of
futex_queues() before the runtime_const_init() patching loop executes, or
it could evaluate it once and reuse the cached dummy register value.

If the compiler hoists this evaluation, won't this BUG_ON() and subsequent
assignment dereference the unpatched dummy pointer and cause a deterministic
kernel panic during early boot?

Would it be safer to directly use the raw global variable __futex_queues
instead of the futex_queues() accessor inside this initialization function?

>  	}
>  
> -	futex_hashmask = hashsize - 1;
>  	pr_info("futex hash table entries: %lu (%lu bytes on %d NUMA nodes, total %lu KiB, %s).\n",
>  		hashsize, size, num_possible_nodes(), size * num_possible_nodes() / 1024,
>  		order > MAX_PAGE_ORDER ? "vmalloc" : "linear");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630045531.3939-1-kprateek.nayak@amd.com?part=8

      reply	other threads:[~2026-06-30  5:20 UTC|newest]

Thread overview: 13+ 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  5:07   ` sashiko-bot
2026-06-30  8:16     ` 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-06-30  5:20   ` sashiko-bot [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=20260630052014.C7D6A1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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