Re: [PATCH 4/4] tracing/osnoise: Take trace_types_lock in timerlat_fd_open

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Mixing `goto`-based error handling with scope-based `guard()` cleanup in `timerlat_fd_open()`.

Pre-existing issues:
- [High] Uninitialized per-CPU mutex and CPU ID for hotplugged CPUs in `init_osnoise_tracer()`.
--

--- Patch [4]: [PATCH 4/4] tracing/osnoise: Take trace_types_lock in timerlat_fd_open ---
commit d0fbb203fad6d30d667803da9a34c6adb7c10f72
Author: Crystal Wood <crwood@redhat.com>

tracing/osnoise: Take trace_types_lock in timerlat_fd_open

This commit replaces the use of interface_lock with trace_types_lock using
a guard(mutex) in timerlat_fd_open. It prevents a user fd from being opened
when osnoise_options_write has already stopped threads but not yet set
OSN_WORKLOAD.

> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index 0f27af6ade3e1..3d12ce8a591e7 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -2466,16 +2466,14 @@ static int timerlat_fd_open(struct inode *inode, struct file *file)
>  	long cpu = (long) inode->i_cdev;
>  	int ret = 0;
>  
> -	mutex_lock(&interface_lock);
> +	guard(mutex)(&trace_types_lock);

[Severity: Low]
Does this mix goto-based error handling with scope-based guard cleanup?

The function still relies on a goto err; block for error handling (which
unlocks osn_var->lock and calls migrate_enable()), but it now uses guard
for trace_types_lock.

The cleanup guidelines suggest converting all resources to scope-based
cleanup, or none of them, to avoid mixing semantics and ownership rules
in the same routine.

[ ... ]

[Severity: High]
This is a pre-existing issue, but does init_osnoise_tracer fail to
initialize per-CPU data for hotplugged CPUs?

Looking at init_osnoise_tracer in kernel/trace/trace_osnoise.c:

kernel/trace/trace_osnoise.c:init_osnoise_tracer() {
    ...
    for_each_online_cpu(cpu) {
        struct osnoise_variables *osn = cpu_osn_var(cpu);

        mutex_init(&osn->lock);
        osn->cpu = cpu;
    }
    ...
}

Because this iterates over online CPUs rather than possible CPUs, if a CPU
is offline during boot or hot-added later, its mutex and cpu ID will remain
uninitialized.

When that CPU comes online, osnoise_hotplug_workfn calls start_kthread(cpu),
which attempts to lock the uninitialized osn->lock. Could this cause a
lockdep warning or kernel crash due to uninitialized list_head pointers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824211544.3984835-1-crwood@redhat.com?part=4