linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: Theodore Ts'o <tytso@mit.edu>,
	Matthew Wilcox <willy@infradead.org>,
	Byungchul Park <max.byungchul.park@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	david@fromorbit.com,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Amir Goldstein <amir73il@gmail.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	oleg@redhat.com, kernel-team@lge.com, daniel@ffwll.ch
Subject: Re: About the try to remove cross-release feature entirely by Ingo
Date: Wed, 3 Jan 2018 10:57:11 +0900	[thread overview]
Message-ID: <c32eb67d-8dc1-2e0a-e359-6f9fb3353906@lge.com> (raw)
In-Reply-To: <20171230154041.GB3366@thunk.org>

On 12/31/2017 12:40 AM, Theodore Ts'o wrote:
> On Fri, Dec 29, 2017 at 10:16:24PM -0800, Matthew Wilcox wrote:
>>
>> I disagree here.  As Ted says, it's the interactions between the
>> subsystems that leads to problems.  Everything's goig to work great
>> until somebody does something in a way that's never been tried before.
> 
> The question what is classified *well* mean?  At the extreme, we could
> put the locks for every single TCP connection into their own lockdep
> class.  But that would blow the limits in terms of the number of locks
> out of the water super-quickly --- and it would destroy the ability
> for lockdep to learn what the proper locking order should be.  Yet
> given Lockdep's current implementation, the only way to guarantee that
> there won't be any interactions between subsystems that cause false
> positives would be to categorizes locks for each TCP connection into
> their own class.
> 
> So this is why I get a little annoyed when you say, "it's just a
> matter of classification".  NO IT IS NOT.  We can not possibly
> classify things "correctly" to completely limit false positives
> without completely destroying lockdep's scalability as it is currently

You seem to admit that it can be solved by proper classification but
say that it's *not realistic* because of the limitation of lockdep.

Right?

I've agreed with you for that point. I also think it's very hard to
do it because of the lockdep design and the only way might be to fix
lockdep fundamentally, that may be the one we should do ultimately.

Is it the best decision to keep it removed until lockdep get fixed
fundamentally? If I believe it were, I would have kept quiet. But, I
don't think so. Almost other users had already gotten benifit from
it except the special case.

And it would be appriciated if you remind that I suggested 3 methods
+ 1 (by Amir) before for that reason.

I don't want to force it forward but just want the facts to be shared.
I felt like I failed it because of the lack of explanation.

> As far as the "just invalidate the waiter", the problem is that it
> requires source level changes to invalidate the waiter, and for

Or, no change is needed if we adopt the (4)th option (by Amir), in
which we keep waiters invalidated by default and validate waiters
explicitly only when it needs.

> different use cases, we will need to validate different waiters.  For
> example, in the example I gave, we would have to invalidate *all* TCP
> waiters/locks in order to prevent false positives.  But that makes the

No. Only invalidating waiters is enough. For now, the waiter in
submit_bio_wait() is the only one to invalidate.

> lockdep useless for all TCP locks.  What's the solution?  I claim that

Even if we invalidate waiters, TCP locks can still work with lockdep.
Invalidating waiters *never* affect lockdep checking for typical locks
at all.

> The only way it can work is to either dump it on the reposibility of
> the people debugging lockdep reports to make source level changes to
> other subsystems which they aren't the maintainers of to suppress
> false positives that arise due to how the subsystems are being used
> together in their particular configuration ---- or you can try to

You seem to misunderstand it a lot.. The only thing we have to is to
use init_completion_nomap() instead of init_completion() for the
problematic completion object. So far, the completion in
submit_bio_wait() has been the only one.

If you belive that we have a lot of problematic completions(waiters)
so that we cannot handle it, the (4) by Amir can be an option.

Just to be sure, there were several false positives by cross-release.
Something was due to confliction between manual acquire()s added
before and automatic cross-release, both of which are for detecting
deadlocks by a specific completion(waiter). Or, something was solved
by classifying locks properly simply. And this case of
submit_bio_wait() is the first case that we cannot classify locks
simply and need to consider other options.

-- 
Thanks,
Byungchul

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2018-01-03  1:57 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13  6:24 Byungchul Park
2017-12-13  7:13 ` Byungchul Park
2017-12-13 15:23   ` Bart Van Assche
2017-12-14  3:07   ` Theodore Ts'o
2017-12-14  5:58     ` Byungchul Park
2017-12-14 11:18     ` Peter Zijlstra
2017-12-14 13:30       ` Byungchul Park
2017-12-13 10:46 ` [PATCH] locking/lockdep: Remove the cross-release locking checks Ingo Molnar
2017-12-14  5:01   ` Byungchul Park
2017-12-15  4:05     ` Byungchul Park
2017-12-15  6:24       ` Theodore Ts'o
2017-12-15  7:38         ` Byungchul Park
2017-12-15  8:39         ` Byungchul Park
2017-12-15 21:15           ` Theodore Ts'o
2017-12-16  2:41             ` Byungchul Park
2017-12-29  1:47 ` About the try to remove cross-release feature entirely by Ingo Byungchul Park
2017-12-29  2:02   ` Byungchul Park
2017-12-29  3:51   ` Theodore Ts'o
2017-12-29  7:28     ` Byungchul Park
2017-12-30  6:16       ` Matthew Wilcox
2017-12-30 15:40         ` Theodore Ts'o
2017-12-30 20:44           ` Matthew Wilcox
2017-12-30 22:40             ` Theodore Ts'o
2017-12-30 23:00               ` Theodore Ts'o
2018-01-01 10:18                 ` Matthew Wilcox
2018-01-01 16:00                   ` Theodore Ts'o
2018-01-03  2:38                     ` Byungchul Park
2018-01-03  2:28                   ` Byungchul Park
2018-01-03  2:58                     ` Dave Chinner
2018-01-03  5:48                       ` Byungchul Park
2018-01-05 16:49                   ` J. Bruce Fields
2018-01-05 17:05                     ` J. Bruce Fields
2018-01-03  2:10               ` Byungchul Park
2018-01-03  7:05                 ` Theodore Ts'o
2018-01-03  8:10                   ` Byungchul Park
2018-01-03  8:23                     ` Byungchul Park
2018-01-03  1:57           ` Byungchul Park [this message]
2018-01-02  7:57         ` Byungchul Park
2017-12-29  8:09   ` Amir Goldstein
2017-12-29  9:46     ` 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=c32eb67d-8dc1-2e0a-e359-6f9fb3353906@lge.com \
    --to=byungchul.park@lge.com \
    --cc=amir73il@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=david@fromorbit.com \
    --cc=kernel-team@lge.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=max.byungchul.park@gmail.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --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