Re: [PATCH v6 02/10] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD

Re: [PATCH v6 02/10] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD

[Chaosheng Qu]

On Wed, 28 Oct 2026 17:01:20 -0500
Crystal Wood <crwood@redhat.com> wrote:

> We can get here on timerlat with OSN_WORKLOAD cleared via
> osnoise_hotplug_workfn().

Confirming this from the other side, because it is worse than a stray
thread.  On 7.3-rc7, with `rtla timerlat top -q -c 0,1` and cpu1 cycled
offline and back online mid-run, the CPU coming up installs a
"timerlat/1" kthread even though OSN_WORKLOAD is clear.  stop_kthread()
picks the teardown path from the *current* OSN_WORKLOAD, so when the
trace stops it takes the user-thread branch for that kthread:

    WARNING: at stop_kthread+0xc6/0xd0, CPU#1: rtla/57
    WARNING: at __put_task_struct+0xdf/0x100, CPU#1: timerlat/1/72
    WARNING: at free_task+0x7d/0x90, CPU#1: kthreadd/2
    BUG: kernel NULL pointer dereference, address: 0000000000000018  (repeated)

kill_pid() plus put_task_struct() is only right for the user thread,
which owns the reference taken in timerlat_fd_open().  A kthread is owned
by kthread_stop(), so the extra put frees it while it is still running.
A plain offline (never bringing the CPU back) does not reproduce it,
because nothing calls start_kthread() then.  So 02/10 fixes a null-deref
here, not just a redundant thread.

One thing about the series itself:

02/10 sets osn->sampling, but `struct osnoise_variables *osn` is not
declared in start_kthread() until 04/10:

    @@ -2053,15 +2081,18 @@ static int start_kthread(unsigned int cpu)
    +	struct osnoise_variables *osn = cpu_osn_var(cpu);

so 02/10 alone does not compile with CONFIG_OSNOISE_TRACER=y.  Sashiko
flagged this on v6 and it looks like it is still there in the posted
version.  I applied 02/10 on its own on 7.3-rc7 to be sure:

    kernel/trace/trace_osnoise.c: In function 'start_kthread':
    kernel/trace/trace_osnoise.c:2043:25: error: 'osn' undeclared (first use in this function)
     2043 |                         osn->sampling = true;

    +	if (!test_bit(OSN_WORKLOAD, &osnoise_options)) {
    +		if (!timerlat_enabled())
    +			osn->sampling = true;

Either move the "osn" declaration into 02/10, or keep
`cpu_osn_var(cpu)->sampling` in this patch and let 04/10 switch it.  The
second keeps each patch buildable on its own, which helps anyone
bisecting the series.

FWIW I also checked the "elder kernels deliver SIGKILL" note you added to
timerlat-tracer.rst in 05/10: on 7.3-rc7 the user thread really is gone
after offline, and rtla's supervisor only decrements its count, it never
reforks.  So that paragraph reads correctly to me.

Thanks,
Chaosheng

Re: [PATCH v6 02/10] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD

[Crystal Wood]

On Thu, 2026-11-12 at 18:20 +0800, Chaosheng Qu wrote:
> On Wed, 28 Oct 2026 17:01:20 -0500
> Crystal Wood <crwood@redhat.com> wrote:
> 
> > We can get here on timerlat with OSN_WORKLOAD cleared via
> > osnoise_hotplug_workfn().
> 
> Confirming this from the other side, because it is worse than a stray
> thread.  On 7.3-rc7, with `rtla timerlat top -q -c 0,1` and cpu1 cycled
> offline and back online mid-run, the CPU coming up installs a
> "timerlat/1" kthread even though OSN_WORKLOAD is clear.  stop_kthread()
> picks the teardown path from the *current* OSN_WORKLOAD, so when the
> trace stops it takes the user-thread branch for that kthread:
> 
>     WARNING: at stop_kthread+0xc6/0xd0, CPU#1: rtla/57
>     WARNING: at __put_task_struct+0xdf/0x100, CPU#1: timerlat/1/72
>     WARNING: at free_task+0x7d/0x90, CPU#1: kthreadd/2
>     BUG: kernel NULL pointer dereference, address: 0000000000000018  (repeated)
> 
> kill_pid() plus put_task_struct() is only right for the user thread,
> which owns the reference taken in timerlat_fd_open().  A kthread is owned
> by kthread_stop(), so the extra put frees it while it is still running.
> A plain offline (never bringing the CPU back) does not reproduce it,
> because nothing calls start_kthread() then.  So 02/10 fixes a null-deref
> here, not just a redundant thread.

If you stop at patch 2 I'm sure you'll find other ways to make it go
boom too :-)

> One thing about the series itself:
> 
> 02/10 sets osn->sampling, but `struct osnoise_variables *osn` is not
> declared in start_kthread() until 04/10:
> 
>     @@ -2053,15 +2081,18 @@ static int start_kthread(unsigned int cpu)
>     +	struct osnoise_variables *osn = cpu_osn_var(cpu);
> 
> so 02/10 alone does not compile with CONFIG_OSNOISE_TRACER=y.  Sashiko
> flagged this on v6 and it looks like it is still there in the posted
> version.  I applied 02/10 on its own on 7.3-rc7 to be sure:

This should be fixed in v7.  Not sure what you meant by "the posted
version" other than v6 itself.

> 
>     kernel/trace/trace_osnoise.c: In function 'start_kthread':
>     kernel/trace/trace_osnoise.c:2043:25: error: 'osn' undeclared (first use in this function)
>      2043 |                         osn->sampling = true;
> 
>     +	if (!test_bit(OSN_WORKLOAD, &osnoise_options)) {
>     +		if (!timerlat_enabled())
>     +			osn->sampling = true;
> 
> Either move the "osn" declaration into 02/10, or keep
> `cpu_osn_var(cpu)->sampling` in this patch and let 04/10 switch it.  The
> second keeps each patch buildable on its own, which helps anyone
> bisecting the series.

I moved the declaration into patch 1, which seemed more appropriate
given that patch is all about cleaning up the per-cpu references.

-Crystal