All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "K Prateek Nayak" <kprateek.nayak@amd.com>
Cc: "Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	linux-s390@vger.kernel.org
Subject: Re: [PATCH v5 3/8] arm64/runtime-const: Introduce runtime_const_mask_32()
Date: Tue, 30 Jun 2026 05:07:41 +0000	[thread overview]
Message-ID: <20260630050742.0BBB21F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260630045531.3939-4-kprateek.nayak@amd.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Calling `__fls(val)` before `BUG_ON(!val)` invokes undefined behavior for `val == 0`, potentially allowing the compiler to optimize away the safety check.
- [Low] The commit message incorrectly references `amd64` instead of `arm64` and misspells `density`.
--

commit e9b79c78d336baf757f7578713831df9b1c4c1b9
Author: K Prateek Nayak <kprateek.nayak@amd.com>

arm64/runtime-const: Introduce runtime_const_mask_32()

This patch introduces runtime_const_mask_32 to optimize the mask operation
in the futex hash computation hot path using the UBFX instruction. It also
adds __runtime_fixup_mask to patch the 32-bit mask in the asm block once
the mask is known.

Link: https://developer.arm.com/documentation/ddi0602/2026-03/Base-Instructions/ [1]
Link: https://lore.kernel.org/lkml/20260430094730.31624-4-kprateek.nayak@amd.com/ [2]

> instruction is used for amd64 to improve instruction dinsity and
> performance.

[Severity: Low]
This isn't a bug, but there are a couple of typos here. Should "amd64" be
"arm64", and "dinsity" be "density"?

> diff --git a/arch/arm64/include/asm/runtime-const.h b/arch/arm64/include/asm/runtime-const.h
> index 838145bc289d2..371c9a4bc2d4b 100644
> --- a/arch/arm64/include/asm/runtime-const.h
> +++ b/arch/arm64/include/asm/runtime-const.h
[ ... ]
> +static inline void __runtime_fixup_mask(void *where, unsigned long val)
> +{
> +	unsigned int width = __fls(val) + 1;
> +	__le32 *p = where;
> +	u32 insn;
> +
> +	/*
[ ... ]
> +	 */
> +	BUG_ON(!val || width > 32 || (GENMASK(width - 1, 0) != val));

[Severity: Medium]
Does evaluating __fls(val) before the BUG_ON(!val) check invoke undefined
behavior if val is 0?

Looking at __fls(), it uses __builtin_clzl(word) which is undefined when
the argument is 0:

include/asm-generic/bitops/builtin-__fls.h:__fls() {
    return (sizeof(word) * 8) - 1 - __builtin_clzl(word);
}

Could the compiler use Value Range Propagation to assume val is non-zero
due to the __fls() call, and silently optimize away the !val safety guard?

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

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

Thread overview: 41+ 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 ` 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   ` 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   ` 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   ` K Prateek Nayak
2026-06-30  5:07   ` sashiko-bot [this message]
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  4:55   ` K Prateek Nayak
2026-06-30  6:47   ` Guo Ren
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   ` K Prateek Nayak
2026-06-30  4:55 ` [PATCH v5 6/8] s390/runtime-const: " K Prateek Nayak
2026-06-30  4:55   ` 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   ` 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  4:55   ` K Prateek Nayak
2026-06-30  5:20   ` sashiko-bot
2026-07-01  7:57   ` Peter Zijlstra
2026-07-01  7:57     ` Peter Zijlstra
2026-07-01  8:41     ` Sebastian Andrzej Siewior
2026-07-01  8:41       ` Sebastian Andrzej Siewior
2026-07-01  9:07       ` K Prateek Nayak
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 16:17           ` Sebastian Andrzej Siewior
2026-07-02  8:59           ` Peter Zijlstra
2026-07-02  8:59             ` Peter Zijlstra
2026-07-02 10:56             ` Sebastian Andrzej Siewior
2026-07-02 10:56               ` Sebastian Andrzej Siewior
2026-07-02 11:18               ` Peter Zijlstra
2026-07-02 11:18                 ` Peter Zijlstra
2026-07-01 11:01       ` [PATCH v5 8/8] futex: Use runtime constants for __futex_hash() hot path Sebastian Andrzej Siewior
2026-07-01 11:01         ` Sebastian Andrzej Siewior
2026-07-01 19:58   ` Sebastian Andrzej Siewior
2026-07-01 19:58     ` Sebastian Andrzej Siewior

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=20260630050742.0BBB21F000E9@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.