linux-kernel - Re: [PATCH v3] locking/lockdep: add debug_show_all_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <274adab4-9922-1586-7593-08d9db5479a1@I-love.SAKURA.ne.jp>
Date:   Mon, 13 Feb 2023 20:34:55 +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 20:02, Peter Zijlstra wrote:
>> @@ -213,7 +213,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout)
>>   unlock:
>>  	rcu_read_unlock();
>>  	if (hung_task_show_lock)
>> -		debug_show_all_locks();
>> +		debug_show_all_lock_holders();
>>  
>>  	if (hung_task_show_all_bt) {
>>  		hung_task_show_all_bt = false;
> 
> This being the hung-task detector, which is mostly about sleeping locks.

Yes, the intent of this patch is to report tasks sleeping with locks held,
for the cause of hung task is sometimes a deadlock.

>> +	rcu_read_lock();
>> +	for_each_process_thread(g, p) {
>> +		if (!p->lockdep_depth)
>> +			continue;
>> +		if (p == current && p->lockdep_depth == 1)
>> +			continue;
>> +		sched_show_task(p);
> 
> 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. 

Locks held and kernel backtrace are helpful for describing deadlock
situation, but registers values are not. 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.