If CONFIG_MICROCODE_LATE_LOADING is not enabled, the compiler warns:
'microcode_mutex' defined but not used
Since reload_store() is the only function using this mutex, move the
mutex definititon there. Then it is also within the #ifdef block for
CONFIG_MICROCODE_LATE_LOADING.
Signed-off-by: John Ogness <[email protected]>
---
arch/x86/kernel/cpu/microcode/core.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 7a329e561354..e7b8f7ad105d 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -49,20 +49,6 @@ bool initrd_gone;
LIST_HEAD(microcode_cache);
-/*
- * Synchronization.
- *
- * All non cpu-hotplug-callback call sites use:
- *
- * - microcode_mutex to synchronize with each other;
- * - cpus_read_lock/unlock() to synchronize with
- * the cpu-hotplug-callback call sites.
- *
- * We guarantee that only a single cpu is being
- * updated at any particular moment of time.
- */
-static DEFINE_MUTEX(microcode_mutex);
-
struct ucode_cpu_info ucode_cpu_info[NR_CPUS];
struct cpu_info_ctx {
@@ -465,6 +451,20 @@ static int microcode_reload_late(void)
return ret;
}
+/*
+ * Synchronization.
+ *
+ * All non cpu-hotplug-callback call sites use:
+ *
+ * - microcode_mutex to synchronize with each other;
+ * - cpus_read_lock/unlock() to synchronize with
+ * the cpu-hotplug-callback call sites.
+ *
+ * We guarantee that only a single cpu is being
+ * updated at any particular moment of time.
+ */
+static DEFINE_MUTEX(microcode_mutex);
+
static ssize_t reload_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t size)
base-commit: 1e760fa3596e8c7f08412712c168288b79670d78
--
2.30.2
On Fri, Mar 24, 2023 at 11:47:20AM +0000, John Ogness wrote:
> If CONFIG_MICROCODE_LATE_LOADING is not enabled, the compiler warns:
>
> 'microcode_mutex' defined but not used
How do you trigger this? I haven't seen this until now in my builds and
CONFIG_MICROCODE_LATE_LOADING is not enabled here either in most
cases:
CONFIG_MICROCODE=y
CONFIG_MICROCODE_INTEL=y
CONFIG_MICROCODE_AMD=y
# CONFIG_MICROCODE_LATE_LOADING is not set
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 2023-03-24, Borislav Petkov <[email protected]> wrote:
>> If CONFIG_MICROCODE_LATE_LOADING is not enabled, the compiler warns:
>>
>> 'microcode_mutex' defined but not used
>
> How do you trigger this?
I was doing some tests with CONFIG_PREEMPT_RT. I did not think that
mattered, since the mutex is obviously defined but not used for
!CONFIG_MICROCODE_LATE_LOADING.
Digging deeper I see that initializing @wait_list in
__MUTEX_INITIALIZER() is what is allowing unused global mutexes to go
unnoticed. Since with CONFIG_PREEMPT_RT the mutex is different and
initialized differently, it is (correctly) detected as unused.
CONFIG_PREEMPT_RT cannot be enabled yet, so this patch is not urgent for
mainline. But at some point it will need fixing.
John Ogness
On Fri, Mar 24, 2023 at 02:17:10PM +0106, John Ogness wrote:
> I was doing some tests with CONFIG_PREEMPT_RT. I did not think that
> mattered, since the mutex is obviously defined but not used for
> !CONFIG_MICROCODE_LATE_LOADING.
Oh sure, I see that. I'd still like to have in the commit message why
exactly it happens.
> Digging deeper I see that initializing @wait_list in
> __MUTEX_INITIALIZER() is what is allowing unused global mutexes to go
> unnoticed. Since with CONFIG_PREEMPT_RT the mutex is different and
> initialized differently, it is (correctly) detected as unused.
>
> CONFIG_PREEMPT_RT cannot be enabled yet, so this patch is not urgent for
> mainline. But at some point it will need fixing.
Sure, send it to me when it can be triggered with a .config with the
upstream tree, along with exact explanation how it happens and I'll
queue it.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 2023-03-24, Borislav Petkov <[email protected]> wrote:
>> I was doing some tests with CONFIG_PREEMPT_RT. I did not think that
>> mattered, since the mutex is obviously defined but not used for
>> !CONFIG_MICROCODE_LATE_LOADING.
>
> Oh sure, I see that. I'd still like to have in the commit message why
> exactly it happens.
Note that an unused mutex is being defined, even when the compiler is
not warning about it.
>> CONFIG_PREEMPT_RT cannot be enabled yet, so this patch is not urgent for
>> mainline. But at some point it will need fixing.
>
> Sure, send it to me when it can be triggered with a .config with the
> upstream tree, along with exact explanation how it happens and I'll
> queue it.
OK, but the kernel test robots will probably beat me to it. ;-)
John Ogness
On Fri, Mar 24, 2023 at 05:39:35PM +0106, John Ogness wrote:
> Note that an unused mutex is being defined, even when the compiler is
> not warning about it.
Are you sure?
$ make arch/x86/kernel/cpu/microcode/core.s
grep microcode_mutex arch/x86/kernel/cpu/microcode/core.s
$
$ make CC=clang arch/x86/kernel/cpu/microcode/core.s
$ grep microcode_mutex arch/x86/kernel/cpu/microcode/core.s
$
Looks to me like it gets optimized away.
> OK, but the kernel test robots will probably beat me to it. ;-)
That's exactly the question: they haven't because this is not a new
situation. Their compilers are optimizing unused stuff away too.
:-)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi John,
I am a new real-time kernel developer. I am still encountering this with
the linux-rt-devel repo and need to apply the patch manually to build.
On Fri, Mar 24, 2023 at 02:17:10PM +0106, John Ogness wrote:
> Digging deeper I see that initializing @wait_list in
> __MUTEX_INITIALIZER() is what is allowing unused global mutexes to go
> unnoticed. Since with CONFIG_PREEMPT_RT the mutex is different and
> initialized differently, it is (correctly) detected as unused.
As far as I can tell @wait_list has been removed from both torvalds and
linux-rt-devel's tree (it is still in the comments).
What would be the next step in getting this resolved?
John B. Wyatt IV