linux-kernel - Re: [PATCH (repost)] 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: <3e027453-fda4-3891-3ec3-5623f1525e56@redhat.com>
Date:   Fri, 16 Sep 2022 10:51:14 -0400
From:   Waiman Long <longman@...hat.com>
To:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Shaokun Zhang <zhangshaokun@...ilicon.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Petr Mladek <pmladek@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ben Dooks <ben.dooks@...ive.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Xiaoming Ni <nixiaoming@...wei.com>,
        John Ogness <john.ogness@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH (repost)] locking/lockdep: add
 debug_show_all_lock_holders()

On 9/16/22 10:15, Tetsuo Handa wrote:
> Currently, check_hung_uninterruptible_tasks() reports details of locks
> held in the system. Also, lockdep_print_held_locks() does not report
> details of locks held by a thread if that thread is in TASK_RUNNING state.
> Several years of experience of debugging without vmcore tells me that
> these limitations have been a barrier for understanding what went wrong
> in syzbot's "INFO: task hung in" reports.
>
> I initially thought that the cause of "INFO: task hung in" reports is
> due to over-stressing. But I understood that over-stressing is unlikely.
> I now consider that there likely is a deadlock/livelock bug where lockdep
> cannot report as a deadlock when "INFO: task hung in" is reported.
>
> A typical case is that thread-1 is waiting for something to happen (e.g.
> wait_event_*()) with a lock held. When thread-2 tries to hold that lock
> using e.g. mutex_lock(), check_hung_uninterruptible_tasks() reports that
> thread-2 is hung and thread-1 is holding a lock which thread-2 is trying
> to hold. But currently check_hung_uninterruptible_tasks() cannot report
> the exact location of thread-1 which gives us an important hint for
> understanding why thread-1 is holding that lock for so long period.
>
> When check_hung_uninterruptible_tasks() reports a thread waiting for a
> lock, it is important to report backtrace of threads which already held
> that lock. Therefore, allow check_hung_uninterruptible_tasks() to report
> the exact location of threads which is holding any lock.

I am not against this patch, but I do like to see you wrapping your code 
in a __debug_show_all_locks() wrapper, for instance, with flags and make 
debug_show_all_locks() uses the new wrapper to avoid code redundancy.


> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> ---
> This is repost of https://lkml.kernel.org/r/82af40cc-bf85-2b53-b8f9-dfc12e66a781@I-love.SAKURA.ne.jp .
> I think there was no critical objection which blocks this change.
>
> I wish that lockdep continues tracking locks (i.e. debug_locks remains 1)
> even after something went wrong, for recently I sometimes encounter problems
> that disable lockdep during boot stage.
>
> It would be noisy to report possibility of e.g. circular locking dependency
> every time due to keeping debug_locks enabled. But tracking locks even after
> something went wrong will help debug_show_all_lock_holders() to survive
> problems during boot stage.
>
> I'm not expecting lockdep to report the same problem forever.
> Reporting possibility of each problem pattern (e.g. circular locking dependency)
> up to once, by using cmpxchg() inside reporting functions that call printk(),
> would be enough.
>
> I'm expecting lockdep to continue working without calling printk() even after
> one of problem patterns (e.g. circular locking dependency) was printk()ed, so that
> debug_show_all_locks()/debug_show_all_lock_holders() can call printk() when needed.
>
> Changing debug_locks behavior is a future patch. For now, this patch alone
> will help debugging Greg's usb.git#usb-testing tree which is generating
> many "INFO: task hung in" reports.

Boqun is actually working on a modularization patch to make some lockdep 
checking still active even after a lockdep bug is reported. I think he 
will take into consideration about this request.

Cheers,
Longman