Re: [PATCH v4 06/13] riscv: introduce riscv_has_extension_[un]likely()

[Conor Dooley]

Hey Jisheng,

On Sun, Jan 15, 2023 at 11:49:46PM +0800, Jisheng Zhang wrote:
> Generally, riscv ISA extensions are fixed for any specific hardware
> platform, so a hart's features won't change after booting. This
> chacteristic makes it straightforward to use a static branch to check
> if a specific ISA extension is supported or not to optimize
> performance.
> 
> However, some ISA extensions such as SVPBMT and ZICBOM are handled
> via. the alternative sequences.
> 
> Basically, for ease of maintenance, we prefer to use static branches
> in C code, but recently, Samuel found that the static branch usage in
> cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As
> Samuel pointed out, "Having a static branch in cpu_relax() is
> problematic because that function is widely inlined, including in some
> quite complex functions like in the VDSO. A quick measurement shows
> this static branch is responsible by itself for around 40% of the jump
> table."
> 
> Samuel's findings pointed out one of a few downsides of static branches
> usage in C code to handle ISA extensions detected at boot time:
> static branch's metadata in the __jump_table section, which is not
> discarded after ISA extensions are finalized, wastes some space.
> 
> I want to try to solve the issue for all possible dynamic handling of
> ISA extensions at boot time. Inspired by Mark[2], this patch introduces
> riscv_has_extension_*() helpers, which work like static branches but
> are patched using alternatives, thus the metadata can be freed after
> patching.
> 
> Link: https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel@xxxxxxxxxxxx/ [1]
> Link: https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland@xxxxxxx/ [2]
> Signed-off-by: Jisheng Zhang <jszhang@xxxxxxxxxx>
> Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>

It'd be great, if, in the future, you would hold off on sending new
versions of patchsets where the previous version is still being
discussed [3].
~3 days between versions is not very much, especially when that includes
a weekend!
I know you replied there earlier today with your opinion, but please
give people a chance to read and respond, before resubmitting, so as not
to split discussion between several threads.

3 - https://lore.kernel.org/linux-riscv/2398293.3Lj2Plt8kZ@diego/

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature