linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: Byungchul Park <max.byungchul.park@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	david@fromorbit.com, tytso@mit.edu, willy@infradead.org,
	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
Subject: Re: About the try to remove cross-release feature entirely by Ingo
Date: Fri, 29 Dec 2017 10:47:36 +0900	[thread overview]
Message-ID: <20171229014736.GA10341@X58A-UD3R> (raw)
In-Reply-To: <CANrsvRPQcWz-p_3TYfNf+Waek3bcNNPniXhFzyyS=7qbCqzGyg@mail.gmail.com>

On Wed, Dec 13, 2017 at 03:24:29PM +0900, Byungchul Park wrote:
> Lockdep works, based on the following:
> 
>    (1) Classifying locks properly
>    (2) Checking relationship between the classes
> 
> If (1) is not good or (2) is not good, then we
> might get false positives.
> 
> For (1), we don't have to classify locks 100%
> properly but need as enough as lockdep works.
> 
> For (2), we should have a mechanism w/o
> logical defects.
> 
> Cross-release added an additional capacity to
> (2) and requires (1) to get more precisely classified.
> 
> Since the current classification level is too low for
> cross-release to work, false positives are being
> reported frequently with enabling cross-release.
> Yes. It's a obvious problem. It needs to be off by
> default until the classification is done by the level
> that cross-release requires.
> 
> But, the logic (2) is valid and logically true. Please
> keep the code, mechanism, and logic.

I admit the cross-release feature had introduced several false positives
about 4 times(?), maybe. And I suggested roughly 3 ways to solve it. I
should have explained each in more detail. The lack might have led some
to misunderstand.

   (1) The best way: To classify all waiters correctly.

      Ultimately the problems should be solved in this way. But it
      takes a lot of time so it's not easy to use the way right away.
      And I need helps from experts of other sub-systems.

      While talking about this way, I made a trouble.. I still believe
      that each sub-system expert knows how to solve dependency problems
      most, since each has own dependency rule, but it was not about
      responsibility. I've never wanted to charge someone else it but me.

   (2) The 2nd way: To make cross-release off by default.

      At the beginning, I proposed cross-release being off by default.
      Honestly, I was happy and did it when Ingo suggested it on by
      default once lockdep on. But I shouldn't have done that but kept
      it off by default. Cross-release can make some happy but some
      unhappy until problems go away through (1) or (2).

   (3) The 3rd way: To invalidate waiters making trouble.

      Of course, this is not the best. Now that you have already spent
      a lot of time to fix original lockdep's problems since lockdep was
      introduced in 2006, we don't need to use this way for typical
      locks except a few special cases. Lockdep is fairly robust by now.

      And I understand you don't want to spend more time to fix
      additional problems again. Now that the situation is different
      from the time, 2006, it's not too bad to use this way to handle
      the issues.

IMO, the ways can be considered together at a time, which perhaps would
be even better.

Talking about what Ingo said in the commit msg.. I want to ask him back,
if he did it with no false positives at the moment merging it in 2006,
without using (2) or (3) method. I bet he know what it means.. And
classifying locks/waiters correctly is not something uglifying code but
a way to document code better. I've felt ill at ease because of the
unnatural and forced explanation.

--
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:[~2017-12-29  1:48 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 ` Byungchul Park [this message]
2017-12-29  2:02   ` About the try to remove cross-release feature entirely by Ingo 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
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=20171229014736.GA10341@X58A-UD3R \
    --to=byungchul.park@lge.com \
    --cc=amir73il@gmail.com \
    --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