mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	torvalds@linux-foundation.org, peterz@infradead.org,
	mingo@redhat.com, will@kernel.org, linux-kernel@vger.kernel.org,
	joel@joelfernandes.org, alexander.levin@microsoft.com,
	daniel.vetter@ffwll.ch, chris@chris-wilson.co.uk,
	duyuyang@gmail.com, johannes.berg@intel.com, tj@kernel.org,
	tytso@mit.edu, willy@infradead.org, david@fromorbit.com,
	amir73il@gmail.com, bfields@fieldses.org,
	gregkh@linuxfoundation.org, kernel-team@lge.com
Subject: Re: [RFC] Are you good with Lockdep?
Date: Thu, 12 Nov 2020 17:10:30 +0900	[thread overview]
Message-ID: <20201112081030.GB14554@X58A-UD3R> (raw)
In-Reply-To: <87d00jo55p.fsf@nanos.tec.linutronix.de>

On Thu, Nov 12, 2020 at 12:16:50AM +0100, Thomas Gleixner wrote:
> Wrappers which make things simpler are always useful, but the lack of
> wrappers does not justify a wholesale replacement.

Totally right. Lack of wrappers doesn't matter at all. That could be
achieved easily by modifying the original e.i. Lockdep. That's why I
didn't mention wrapper things in the description. (Sorry if I misled
you so it looked like I mentioned just wrappers. I should've explained
it in more detail.)

xxx_wait(), xxx_event() and xxx_event_context_start() are much more than
wrappers. It was about what deadlock detection tool should work based on.

> We all know that lockdep has limitations but I yet have to see a proper
> argument why this can't be solved incrementaly on top of the existing
> infrastructure.

This is exactly what I'd like to address. As you can see in the first
mail, the reasons why this can't be solved incrementaly are:

1. Lockdep's design and implementation are too complicated to be
   generalized so as to allow multi-reporting. Quite big change onto
   Lockdep would be required.

   I think allowing multi-reporting is very important for tools like
   Lockdep. As long as false positive in the single-reporting manner
   bothers folks, tools like Lockdep cannot be enhanced so as to have
   stronger capability.

2. Does Lockdep do what a deadlock detection tool should do? From
   internal engine to APIs, all the internal data structure and
   algotithm of Lockdep is only looking at lock(?) acquisition order.
   Fundamentally Lockdep cannot work correctly with all general cases,
   for example, read/write/trylock and any wait/event.

   This can be done by re-introducing cross-release but still partially.
   A deadlock detector tool should thoroughly focus on *waits* and
   *events* to be more perfect at detecting deadlock because the fact is
   *waits* and their *events* that never reach cause deadlock.

   With the philosophy of Lockdep, we can only handle partial cases
   fundamently. We have no choice but to do various work-around or adopt
   tricky ways to cover more cases if we keep using Lockdep.

> That said, I'm not at all interested in a wholesale replacement of
> lockdep which will take exactly the same amount of time to stabilize and
> weed out the shortcomings again.

I don't want to bother ones who don't want to be bothered from the tool.
But I think some day we need a new tool doing exactly what it should do
for deadlock detection for sure.

I'm willing to make it matured on my own, or with ones who need a
stronger tool or willing to make it matured together - I wish tho.
That's why I suggest to make both there until the new tool gets
considered stable.

FYI, roughly Lockdep is doing:

   1. Dependency check
   2. Lock usage correctness check (including RCU)
   3. IRQ related usage correctness check with IRQFLAGS

2 and 3 should be there forever which is subtle and have gotten matured.
But 1 is not. I've been talking about 1. But again, it's not about
replacing it right away but having both for a while. I'm gonna try my
best to make it better.

Thanks,
Byungchul

  reply	other threads:[~2020-11-12  8:11 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11  5:05 Byungchul Park
2020-11-11 10:54 ` Ingo Molnar
2020-11-11 14:36   ` Steven Rostedt
2020-11-11 23:16     ` Thomas Gleixner
2020-11-12  8:10       ` Byungchul Park [this message]
2020-11-12 14:26         ` Steven Rostedt
2020-11-12 14:52           ` Matthew Wilcox
2020-11-16  8:57             ` Byungchul Park
2020-11-16 15:37               ` Matthew Wilcox
2020-11-18  1:45                 ` Boqun Feng
2020-11-18  3:30                   ` Matthew Wilcox
2020-11-23 13:15                 ` Byungchul Park
2020-11-12 14:58           ` Byungchul Park
2020-11-16  9:05             ` Byungchul Park
2020-11-23 10:45               ` Byungchul Park
2020-11-12 10:32     ` Byungchul Park
2020-11-12 13:56       ` Daniel Vetter
2020-11-16  8:45         ` Byungchul Park
2020-11-12  6:15   ` Byungchul Park
2020-11-12  8:51     ` Byungchul Park
2020-11-12  9:46       ` Byungchul Park
2020-11-23 11:05 ` [RFC] Dept(Dependency Tracker) Implementation Byungchul Park
2020-11-23 11:36   ` [RFC 1/6] dept: Implement Dept(Dependency Tracker) Byungchul Park
2020-11-23 11:36     ` [RFC 2/6] dept: Apply Dept to spinlock Byungchul Park
2020-11-23 11:36     ` [RFC 3/6] dept: Apply Dept to mutex families Byungchul Park
2020-11-23 11:36     ` [RFC 4/6] dept: Apply Dept to rwlock Byungchul Park
2020-11-23 11:36     ` [RFC 5/6] dept: Apply Dept to wait_for_completion()/complete() Byungchul Park
2020-11-23 11:36     ` [RFC 6/6] dept: Assign custom dept_keys or disable to avoid false positives Byungchul Park
2020-11-23 12:29   ` [RFC] Dept(Dependency Tracker) Implementation Byungchul Park
2020-11-23 11:13 ` [RFC] Dept(Dependency Tracker) Report Example Byungchul Park
2020-11-23 12:14   ` Byungchul Park

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=20201112081030.GB14554@X58A-UD3R \
    --to=byungchul.park@lge.com \
    --cc=alexander.levin@microsoft.com \
    --cc=amir73il@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=david@fromorbit.com \
    --cc=duyuyang@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel@joelfernandes.org \
    --cc=johannes.berg@intel.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=will@kernel.org \
    --cc=willy@infradead.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

all inboxes | Powered by JetHome®