All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Crystal Wood" <crwood@redhat.com>
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/9] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD
Date: Sat, 10 Oct 2026 02:34:24 +0000	[thread overview]
Message-ID: <sashiko-outbox-166221@kernel.org> (raw)
In-Reply-To: <20261010022338.2507589-6-crwood@redhat.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing/osnoise: WARN_ON triggered on restart with NO_OSNOISE_WORKLOAD
--

commit 0c861cff53ad6b5caf50d13aaff67f66512d0646
Author: Crystal Wood <crwood@redhat.com>

tracing/osnoise: start_kthread: Always check OSN_WORKLOAD

This commit modifies start_kthread() to check OSN_WORKLOAD up front and
return early if it is not set, instead of only checking it inside the
non-timerlat branch.

> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -2077,16 +2077,17 @@ static int start_kthread(unsigned int cpu)
>  	if (osn->kthread)
>  		goto out;
>  
> +	/* Can still happen with timerlat via cpu hotplug */
> +	if (!test_bit(OSN_WORKLOAD, &osnoise_options)) {
> +		if (!timerlat_enabled())
> +			osn->sampling = true;
> +		goto out;

[Severity: Medium]
If OSN_WORKLOAD is not set, this code sets osn->sampling to true and
returns without creating a kthread. Later, when the tracer is stopped,
stop_kthread() does nothing because osn_var->kthread is NULL:

stop_kthread() {
    ...
    } else if (osn_var->kthread) {
        ...
        osn_reset_cpu(cpu);
    }
}

Because osn_reset_cpu() is skipped, the sampling state is leaked. When the
tracer is subsequently restarted, osn_var_reset() checks the state and
triggers a kernel warning:

osn_var_reset() {
    ...
    WARN_ON(osn->sampling);
    ...
}

Could this leave a stale sampling state that persists across tracer
stop/start, triggering a WARN_ON in osn_var_reset() and breaking the
restartability of the tracer without a workload?

> +	}
> +
>  	if (timerlat_enabled()) {
>  		snprintf(comm, 24, "timerlat/%d", cpu);
>  		main = timerlat_main;
>  	} else {

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

  reply	other threads:[~2026-10-10  2:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-10  2:23 [PATCH v2 0/9] tracing/osnoise: Synchronization fixes Crystal Wood
2026-10-10  2:23 ` [PATCH v2 1/9] tracing/osnoise: Variable cleanup Crystal Wood
2026-10-10  2:23 ` [PATCH v2 2/9] tracing/osnoise: Hold a ref on osn->kthread Crystal Wood
2026-10-10  2:23 ` [PATCH v2 3/9] tracing/osnoise: Per-cpu mutexes and fd detachment Crystal Wood
2026-10-10  2:35   ` sashiko-bot
2026-10-10  2:23 ` [PATCH v2 4/9] tracing/osnoise: kthreads: Verify CPU after migrate_disable() Crystal Wood
2026-10-10  2:23 ` [PATCH v2 5/9] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD Crystal Wood
2026-10-10  2:34   ` sashiko-bot [this message]
2026-10-10  2:23 ` [PATCH v2 6/9] tracing/osnoise: Take trace_types_lock in timerlat_fd_open Crystal Wood
2026-10-10  2:23 ` [PATCH v2 7/9] tracing/osnoise: Split out per-cpu variable resets Crystal Wood
2026-10-10  2:36   ` sashiko-bot
2026-10-10  2:23 ` [PATCH v2 8/9] tracing/osnoise: Print taint message for timerlat Crystal Wood
2026-10-10  2:23 ` [PATCH v2 9/9] tracing/osnoise: Refuse timerlat fd open if tracer inactive Crystal Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-166221@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=crwood@redhat.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.