Re: [PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case

[Ingo Molnar <mingo@kernel.org>]

* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, May 05, 2025 at 07:15:00AM +0200, Ingo Molnar wrote:
> > 
> > Fix this build bug:
> > 
> >   ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]
> > 
> > by adding the 'static' storage class to the !CONFIG_MICROCODE 
> > prototype.
> 
> Thanks, I'll merge it with my patch.

Please don't forcibly rebase trees like that, because you've just 
reintroduced a bug this way in tip:x86/urgent:

  # 5214a9f6c0f5 x86/microcode: Consolidate the loader enablement checking

  +static inline bool __init microcode_loader_disabled(void) { return false; }

static inlines don't need an __init tag ...

This was done correctly in the commit you removed:

  59e820c6de60 ("x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case")

  +static inline bool microcode_loader_disabled(void) { return false; }

> > Also, while at it, add all the other storage classes as well for this 
> > block of prototypes, 'extern' and 'static', respectively.
> 
> This I won't add as it is unnecessary.

Technically the 'int' in 'unsigned int' is 'unnecessary' and could be 
skipped as well with 'unsigned', right? Yet clean kernel code uses it 
because it's easier to read and more consistent. Likewise, the 'extern' 
storage class is a well-known and widespread way in the kernel to 
document public APIs, a counterpart to 'static'. Most of our major 
headers use that style.

Your change is a doubly poor choice in this particular case of 
<asm/microcode.h> as well, because the header continues with an 
'extern':

  extern unsigned long initrd_start_early;

So by dropping that cleanup you reintroduced an inconsistency as 
well...

Thanks,

	Ingo