Message-ID: <393a440f-5f82-432c-bc24-e8de33e29d75@I-love.SAKURA.ne.jp>
Date:   Mon, 13 Feb 2023 22:49:21 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...hat.com>,
        Waiman Long <longman@...hat.com>,
        Will Deacon <will@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v3] locking/lockdep: add debug_show_all_lock_holders()

On 2023/02/13 21:49, Peter Zijlstra wrote:
>>> And sched_show_task() being an utter piece of crap that will basically
>>> print garbage for anything that's running (it doesn't have much
>>> options).
>>>
>>> Should we try and do better? dump_cpu_task() prefers
>>> trigger_single_cpu_backtrace(), which sends an interrupt in order to get
>>> active registers for the CPU.
>>
>> What is the intent of using trigger_single_cpu_backtrace() here?
>> check_hung_uninterruptible_tasks() is calling trigger_all_cpu_backtrace()
>> if sysctl_hung_task_all_cpu_backtrace is set.
> 
> Then have that also print the held locks for those tasks. And skip over
> them again later.
> 
>> Locks held and kernel backtrace are helpful for describing deadlock
>> situation, but registers values are not.
> 
> Register state is required to start the unwind. You can't unwind a
> running task out of thin-air.

Excuse me. There are two types of TASK_RUNNING tasks, one is that a thread
is actually running on some CPU, and the other is that a thread is waiting
for CPU to become available for that thread, aren't there?

lockdep_print_held_locks() does not show locks held even if a thread is
waiting for CPU to become available for that thread, does it?

But sched_show_task() can show backtrace even if a thread is waiting for
CPU to become available for that thread, can't it?

Therefore, calling sched_show_task() helps understanding what that thread
is doing when lockdep_print_held_locks() did not show locks held.

> 
>> What is important is that tasks which are not on CPUs are reported,
>> for when a task is reported as hung, that task must be sleeping.
>> Therefore, I think sched_show_task() is fine.
> 
> The backtraces generated by sched_show_task() for a running task are
> absolutely worthless, might as well not print them.

"a thread actually running on some CPU" or
"a thread waiting for CPU to become available for that thread",
which does this "running task" mean?

> 
> And if I read your Changelog right, you explicitly wanted useful
> backtraces for the running tasks -- such that you could see what they
> were doing while holding the lock the other tasks were blocked on.

Yes, we can get useful backtraces for threads that are waiting for CPU
to become available for that thread. That's why sched_show_task() is chosen.

> 
> The only way to do that is to send an interrupt, the interrupt will have
> the register state for the interrupted task -- including the stack
> pointer. By virtue of running the interrupt handler we know the stack
> won't shrink, so we can then safely traverse the stack starting from the
> given stack pointer.

But trigger_single_cpu_backtrace() is for a thread actually running on some CPU,
isn't it? While it would be helpful to get backtrace of a thread that is actually
running on some CPU, it would be helpless not getting backtrace of a thread
that is waiting for CPU to become available for that thread.

We can later get backtrace of threads actually running on some CPU using
trigger_all_cpu_backtrace() via sysctl_hung_task_all_cpu_backtrace setting,
though I seldom find useful backtraces via trigger_all_cpu_backtrace(); it is
likely that khungtaskd thread and some random workqueue thread (which are
irrelevant to hung task) are reported via trigger_all_cpu_backtrace()...

Powered by blists - more mailing lists