Linux Trace Kernel
 help / color / mirror / Atom feed
From: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
To: Crystal Wood <crwood@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Gabriele Monaco <gmonaco@redhat.com>,
	linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 02/10] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD
Date: Thu, 12 Nov 2026 18:20:20 +0800	[thread overview]
Message-ID: <179447882048.254241.2527227025791137084@gmail.com> (raw)
In-Reply-To: <20261028220120.4005187-3-crwood@redhat.com>

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


  parent reply	other threads:[~2026-11-12 10:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-28 22:01 [PATCH v6 0/10] tracing/osnoise: Synchronization fixes Crystal Wood
2026-10-28 22:01 ` [PATCH v6 01/10] tracing/osnoise: Variable cleanup Crystal Wood
2026-10-28 22:01 ` [PATCH v6 02/10] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD Crystal Wood
2026-10-28 22:06   ` sashiko-bot
2026-11-12 10:20   ` Chaosheng Qu [this message]
2026-10-28 22:01 ` [PATCH v6 03/10] tracing/osnoise: Start all eligible cpus from hotplug workfn Crystal Wood
2026-10-28 22:01 ` [PATCH v6 04/10] tracing/osnoise: Hold a ref on osn->kthread Crystal Wood
2026-10-28 22:01 ` [PATCH v6 05/10] tracing/osnoise: Per-cpu mutexes and fd detachment Crystal Wood
2026-10-28 22:20   ` sashiko-bot
2026-10-28 22:01 ` [PATCH v6 06/10] tracing/osnoise: Split out per-cpu variable resets Crystal Wood
2026-10-28 22:24   ` sashiko-bot
2026-10-28 22:01 ` [PATCH v6 07/10] tracing/osnoise: kthreads: Verify CPU after migrate_disable() Crystal Wood
2026-10-28 22:01 ` [PATCH v6 08/10] tracing/osnoise: Take trace_types_lock in timerlat_fd_open Crystal Wood
2026-10-28 22:01 ` [PATCH v6 09/10] tracing/osnoise: Print taint message for timerlat Crystal Wood
2026-10-28 22:01 ` [PATCH v6 10/10] 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=179447882048.254241.2527227025791137084@gmail.com \
    --to=chaosheng.qu.linux@gmail.com \
    --cc=crwood@redhat.com \
    --cc=gmonaco@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox