Re: [PATCH v5 5/8] riscv/runtime-const: Introduce runtime_const_mask_32()

Charlie Jenkins <[email protected]>
Newsgroups org.kernel.vger.linux-arch,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-riscv,org.kernel.vger.linux-kernel,org.kernel.vger.linux-s390
Message-ID <alXQY4hgZYLmr8x7@blinky>
On Fri, Jul 10, 2026 at 01:47:10PM +0530, K Prateek Nayak wrote:
> Hello Charlie,
> 
> On 7/10/2026 1:22 PM, Charlie Jenkins wrote:
> > [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > On Tue, 30 Jun 2026 04:55:28 +0000, K Prateek Nayak <[email protected]> wrote:
> >> Futex hash computation requires a mask operation with read-only after
> >> init data that will be converted to a runtime constant in the subsequent
> >> commit.
> >>
> >> Introduce runtime_const_mask_32 to further optimize the mask operation
> >> in the futex hash computation hot path. Since all the current use-cases
> >> are of the form GENMASK(n, 0), with n > 0, following sequence:
> > 
> > I really appreciate you spending the time to do this, thank you!
> 
> My pleasure! And I really appreciate you taking time to review and test
> this series. Thanks a ton for that!
> 
> > 
> >>
> >>
> >> diff --git a/arch/riscv/include/asm/runtime-const.h b/arch/riscv/include/asm/runtime-const.h
> >> index 1ce02605d2e4..dbf96c937dbb 100644
> >> --- a/arch/riscv/include/asm/runtime-const.h
> >> +++ b/arch/riscv/include/asm/runtime-const.h
> >> @@ -262,6 +279,33 @@ static inline void __runtime_fixup_shift(void *where, unsigned long val)
> >> [ ... skip 24 lines ... ]
> >> +     BUG_ON(!val || width > 31 || (GENMASK(width - 1, 0) != val));
> >> +
> >> +     __runtime_fixup_shift(where, 32 - width);
> >> +     __runtime_fixup_shift(where + 4, 32 - width);
> >> +}
> >> +
> > 
> > It would be "optimal" to use an andi when the mask is <=11 bits since
> > andi can fit an 11 bit mask. What you have is good enough but I'll leave
> > my stab at doing the andi patching here in case you want to apply it.
> > 
> > From 9e5527aaddd464783af795aacdb6d094e11cc31e Mon Sep 17 00:00:00 2001
> > From: Charlie Jenkins <[email protected]>
> > Date: Thu, 9 Jul 2026 23:18:09 -0700
> > Subject: [PATCH] riscv: Optimize __runtime_fixup_mask for masks with <= 11
> >  bits
> 
> Peter seems to have merged the v5 series in his tree but If you could give
> your S-o-b, I can throw in a commit log, some testing along with a few
> cosmetic modifications, and send it for official review on top of
> queue:locking/core ;-)

I'm glad it got merged, I missed that! Here's my tag that I forgot to
add...

Signed-off-by: Charlie Jenkins <[email protected]>

> 
> > 
> > ---
> >  arch/riscv/include/asm/insn.h          |  2 ++
> >  arch/riscv/include/asm/runtime-const.h | 29 ++++++++++++++++++++++++--
> >  2 files changed, 29 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> > index c3005573e8c9..0a34cd7305d0 100644
> > --- a/arch/riscv/include/asm/insn.h
> > +++ b/arch/riscv/include/asm/insn.h
> > @@ -141,6 +141,7 @@
> >  #define RVG_OPCODE_JALR                0x67
> >  #define RVG_OPCODE_JAL         0x6f
> >  #define RVG_OPCODE_SYSTEM      0x73
> > +#define RVG_OPCODE_ANDI                0x13
> >  #define RVG_SYSTEM_CSR_OFF     20
> >  #define RVG_SYSTEM_CSR_MASK    GENMASK(12, 0)
> > 
> > @@ -175,6 +176,7 @@
> >  #define RVG_FUNCT3_BGE         0x5
> >  #define RVG_FUNCT3_BLTU                0x6
> >  #define RVG_FUNCT3_BGEU                0x7
> > +#define RVG_FUNCT3_ANDI                0x7
> > 
> >  /* parts of funct3 code for C extension*/
> >  #define RVC_FUNCT3_C_BEQZ      0x6
> > diff --git a/arch/riscv/include/asm/runtime-const.h b/arch/riscv/include/asm/runtime-const.h
> > index dbf96c937dbb..24a9b13081f7 100644
> > --- a/arch/riscv/include/asm/runtime-const.h
> > +++ b/arch/riscv/include/asm/runtime-const.h
> > @@ -9,6 +9,7 @@
> >  #include <asm/asm.h>
> >  #include <asm/alternative.h>
> >  #include <asm/cacheflush.h>
> > +#include <asm/insn.h>
> >  #include <asm/insn-def.h>
> >  #include <linux/memory.h>
> >  #include <asm/text-patching.h>
> > @@ -302,8 +303,32 @@ static inline void __runtime_fixup_mask(void *where, unsigned long val)
> >          */
> >         BUG_ON(!val || width > 31 || (GENMASK(width - 1, 0) != val));
> > 
> > -       __runtime_fixup_shift(where, 32 - width);
> > -       __runtime_fixup_shift(where + 4, 32 - width);
> > +       /*
> > +        * A riscv 'andi' instruction can fit an 11 bit immediate, so the mask
> > +        * can be directly applied. Otherwise fall back to SRLI + SLLI.
> > +        */
> > +       if (width < 11) {
> > +               __le16 *parcel = where;
> > +               u32 insn;
> > +               __le32 res, nop;
> > +
> > +               insn = (u32)le16_to_cpu(parcel[0]) | (u32)le16_to_cpu(parcel[1]) << 16;
> > +
> > +               /* Replace the slli/slliw with an andi */
> > +               insn &= 0x000fcf80;
> > +               insn |= val << 20 | RV_ENCODE_FUNCT3(ANDI) | RVG_OPCODE_ANDI;
> > +
> > +               res = cpu_to_le32(insn);
> > +               /* Replace the srli/srliw with a nop */
> > +               nop = cpu_to_le32(RISCV_INSN_NOP4);
> > +               mutex_lock(&text_mutex);
> > +               patch_text_nosync(where, &res, sizeof(insn));
> > +               patch_text_nosync(where + 4, &nop, sizeof(insn));
> > +               mutex_unlock(&text_mutex);
> > +       } else {
> > +               __runtime_fixup_shift(where, 32 - width);
> > +               __runtime_fixup_shift(where + 4, 32 - width);
> > +       }
> >  }
> > 
> >  static inline void runtime_const_fixup(void (*fn)(void *, unsigned long),
> > --
> > 2.54.0
> > 
> > 
> > I would prefer including this, but I am happy to approve this
> > regardless.
> 
> Ack! I'll keep it as an optimization on top to retain your attribution.
> 
> > 
> > Reviewed-by: Charlie Jenkins <[email protected]>
> > Tested-by: Charlie Jenkins <[email protected]>
> 
> Thank you again!
> 
> -- 
> Thanks and Regards,
> Prateek
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.