All of lore.kernel.org
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: tytso@mit.edu
Cc: torvalds@linux-foundation.org, damien.lemoal@opensource.wdc.com,
	linux-ide@vger.kernel.org, adilger.kernel@dilger.ca,
	linux-ext4@vger.kernel.org, mingo@redhat.com,
	linux-kernel@vger.kernel.org, peterz@infradead.org,
	will@kernel.org, tglx@linutronix.de, rostedt@goodmis.org,
	joel@joelfernandes.org, sashal@kernel.org,
	daniel.vetter@ffwll.ch, chris@chris-wilson.co.uk,
	duyuyang@gmail.com, johannes.berg@intel.com, tj@kernel.org,
	willy@infradead.org, david@fromorbit.com, amir73il@gmail.com,
	gregkh@linuxfoundation.org, kernel-team@lge.com,
	linux-mm@kvack.org, akpm@linux-foundation.org, mhocko@kernel.org,
	minchan@kernel.org, hannes@cmpxchg.org, vdavydov.dev@gmail.com,
	sj@kernel.org, jglisse@redhat.com, dennis@kernel.org,
	cl@linux.com, penberg@kernel.org, rientjes@google.com,
	vbabka@suse.cz, ngupta@vflare.org, linux-block@vger.kernel.org,
	paolo.valente@linaro.org, josef@toxicpanda.com,
	linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk,
	jack@suse.cz, jack@suse.com, jlayton@kernel.org,
	dan.j.williams@intel.com, hch@infradead.org, djwong@kernel.org,
	dri-devel@lists.freedesktop.org, rodrigosiqueiramelo@gmail.com,
	melissa.srw@gmail.com, hamohammed.sa@gmail.com,
	42.hyeyoo@gmail.com
Subject: Re: [PATCH RFC v6 00/21] DEPT(Dependency Tracker)
Date: Tue, 10 May 2022 14:37:40 +0900	[thread overview]
Message-ID: <1652161060-26531-1-git-send-email-byungchul.park@lge.com> (raw)
In-Reply-To: <YnnAnzPFZZte/UR8@mit.edu>

Ted wrote:
> On Tue, May 10, 2022 at 09:32:13AM +0900, Byungchul Park wrote:
> > Yes, right. DEPT has never been optimized. It rather turns on
> > CONFIG_LOCKDEP and even CONFIG_PROVE_LOCKING when CONFIG_DEPT gets on
> > because of porting issue. I have no choice but to rely on those to
> > develop DEPT out of tree. Of course, that's what I don't like.
> 
> Sure, but blaming the overhead on unnecessary CONFIG_PROVE_LOCKING
> overhead can explain only a tiny fraction of the slowdown.  Consider:
> if time to first test (time to boot the kernel, setup the test
> environment, figure out which tests to run, etc.) is 12 seconds w/o
> LOCKDEP, 49 seconds with LOCKDEP/PROVE_LOCKING and 602 seconds with
> DEPT, you can really only blame 37 seconds out of the 602 seconds of
> DEPT on unnecessary PROVE_LOCKING overhead.
> 
> So let's assume we can get rid of all of the PROVE_LOCKING overhead.
> We're still talking about 12 seconds for time-to-first test without
> any lock debugging, versus ** 565 ** seconds for time-to-first test
> with DEPT.  That's a factor of 47x for DEPT sans LOCKDEP overhead,
> compared to a 4x overhead for PROVE_LOCKING.

Okay. I will work on it.

> > Plus, for now, I'm focusing on removing false positives. Once it's
> > considered settled down, I will work on performance optimizaition. But
> > it should still keep relying on Lockdep CONFIGs and adding additional
> > overhead on it until DEPT can be developed in the tree.
> 
> Well, please take a look at the false positive which I reported.  I
> suspect that in order to fix that particular false positive, we'll
> either need to have a way to disable DEPT on waiting on all page/folio
> dirty bits, or it will need to treat pages from different inodes
> and/or address spaces as being entirely separate classes, instead of
> collapsing all inode dirty bits, and all of various inode's mutexes
> (such as ext4's i_data_sem) as being part of a single object class.

I'd rather solve it by assigning different classes to different types of
inode. This is the right way.

> > DEPT is tracking way more objects than Lockdep so it's inevitable to be
> > slower, but let me try to make it have the similar performance to
> > Lockdep.
> 
> In order to eliminate some of these false positives, I suspect it's
> going to increase the number of object classes that DEPT will need to
> track even *more*.  At which point, the cost/benefit of DEPT may get
> called into question, especially if all of the false positives can't
> be suppressed.

Look. Let's talk in general terms. There's no way to get rid of the
false positives all the way. It's a decision issue for *balancing*
between considering potential cases and only real ones. Definitely,
potential is not real. The more potential things we consider, the higher
the chances are, that false positives appear.

But yes. The advantage we'd take by detecting potential ones should be
higher than the risk of being bothered by false ones. Do you think a
tool is useless if it produces a few false positives? Of course, it'd
be a problem if it's too many, but otherwise, I think it'd be a great
tool if the advantage > the risk.

Don't get me wrong here. It doesn't mean DEPT is perfect for now. The
performance should be improved and false alarms that appear should be
removed, of course. I'm talking about the direction.

For now, there's no tool to track wait/event itself in Linux kernel -
a subset of the functionality exists tho. DEPT is the 1st try for that
purpose and can be a useful tool by the right direction.

I know what you are concerning about. I bet it's false positives that
are going to bother you once merged. I'll insist that DEPT shouldn't be
used as a mandatory testing tool until considered stable enough. But
what about ones who would take the advantage use DEPT. Why don't you
think of folks who will take the advantage from the hints about
dependency of synchronization esp. when their subsystem requires very
complicated synchronization? Should a tool be useful only in a final
testing stage? What about the usefulness during development stage?

It's worth noting DEPT works with any wait/event so any lockups e.g.
even by HW-SW interface, retry logic or the like can be detected by DEPT
once all waits and events are tagged properly. I believe the advantage
by that is much higher than the bad side facing false alarms. It's just
my opinion. I'm goning to respect the majority opinion.

	Byungchul
> 
> 					- Ted
> 

WARNING: multiple messages have this Message-ID (diff)
From: Byungchul Park <byungchul.park@lge.com>
To: tytso@mit.edu
Cc: hamohammed.sa@gmail.com, jack@suse.cz, peterz@infradead.org,
	daniel.vetter@ffwll.ch, amir73il@gmail.com, david@fromorbit.com,
	dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk,
	linux-mm@kvack.org, linux-ide@vger.kernel.org,
	adilger.kernel@dilger.ca, joel@joelfernandes.org,
	42.hyeyoo@gmail.com, cl@linux.com, will@kernel.org,
	duyuyang@gmail.com, sashal@kernel.org, paolo.valente@linaro.org,
	damien.lemoal@opensource.wdc.com, willy@infradead.org,
	hch@infradead.org, mingo@redhat.com, djwong@kernel.org,
	vdavydov.dev@gmail.com, rientjes@google.com, dennis@kernel.org,
	linux-ext4@vger.kernel.org, ngupta@vflare.org,
	johannes.berg@intel.com, jack@suse.com, dan.j.williams@intel.com,
	josef@toxicpanda.com, rostedt@goodmis.org,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	jglisse@redhat.com, viro@zeniv.linux.org.uk, tglx@linutronix.de,
	mhocko@kernel.org, vbabka@suse.cz, melissa.srw@gmail.com,
	sj@kernel.org, rodrigosiqueiramelo@gmail.com,
	kernel-team@lge.com, gregkh@linuxfoundation.org,
	jlayton@kernel.org, linux-kernel@vger.kernel.org,
	penberg@kernel.org, minchan@kernel.org, hannes@cmpxchg.org,
	tj@kernel.org, akpm@linux-foundation.org,
	torvalds@linux-foundation.org
Subject: Re: [PATCH RFC v6 00/21] DEPT(Dependency Tracker)
Date: Tue, 10 May 2022 14:37:40 +0900	[thread overview]
Message-ID: <1652161060-26531-1-git-send-email-byungchul.park@lge.com> (raw)
In-Reply-To: <YnnAnzPFZZte/UR8@mit.edu>

Ted wrote:
> On Tue, May 10, 2022 at 09:32:13AM +0900, Byungchul Park wrote:
> > Yes, right. DEPT has never been optimized. It rather turns on
> > CONFIG_LOCKDEP and even CONFIG_PROVE_LOCKING when CONFIG_DEPT gets on
> > because of porting issue. I have no choice but to rely on those to
> > develop DEPT out of tree. Of course, that's what I don't like.
> 
> Sure, but blaming the overhead on unnecessary CONFIG_PROVE_LOCKING
> overhead can explain only a tiny fraction of the slowdown.  Consider:
> if time to first test (time to boot the kernel, setup the test
> environment, figure out which tests to run, etc.) is 12 seconds w/o
> LOCKDEP, 49 seconds with LOCKDEP/PROVE_LOCKING and 602 seconds with
> DEPT, you can really only blame 37 seconds out of the 602 seconds of
> DEPT on unnecessary PROVE_LOCKING overhead.
> 
> So let's assume we can get rid of all of the PROVE_LOCKING overhead.
> We're still talking about 12 seconds for time-to-first test without
> any lock debugging, versus ** 565 ** seconds for time-to-first test
> with DEPT.  That's a factor of 47x for DEPT sans LOCKDEP overhead,
> compared to a 4x overhead for PROVE_LOCKING.

Okay. I will work on it.

> > Plus, for now, I'm focusing on removing false positives. Once it's
> > considered settled down, I will work on performance optimizaition. But
> > it should still keep relying on Lockdep CONFIGs and adding additional
> > overhead on it until DEPT can be developed in the tree.
> 
> Well, please take a look at the false positive which I reported.  I
> suspect that in order to fix that particular false positive, we'll
> either need to have a way to disable DEPT on waiting on all page/folio
> dirty bits, or it will need to treat pages from different inodes
> and/or address spaces as being entirely separate classes, instead of
> collapsing all inode dirty bits, and all of various inode's mutexes
> (such as ext4's i_data_sem) as being part of a single object class.

I'd rather solve it by assigning different classes to different types of
inode. This is the right way.

> > DEPT is tracking way more objects than Lockdep so it's inevitable to be
> > slower, but let me try to make it have the similar performance to
> > Lockdep.
> 
> In order to eliminate some of these false positives, I suspect it's
> going to increase the number of object classes that DEPT will need to
> track even *more*.  At which point, the cost/benefit of DEPT may get
> called into question, especially if all of the false positives can't
> be suppressed.

Look. Let's talk in general terms. There's no way to get rid of the
false positives all the way. It's a decision issue for *balancing*
between considering potential cases and only real ones. Definitely,
potential is not real. The more potential things we consider, the higher
the chances are, that false positives appear.

But yes. The advantage we'd take by detecting potential ones should be
higher than the risk of being bothered by false ones. Do you think a
tool is useless if it produces a few false positives? Of course, it'd
be a problem if it's too many, but otherwise, I think it'd be a great
tool if the advantage > the risk.

Don't get me wrong here. It doesn't mean DEPT is perfect for now. The
performance should be improved and false alarms that appear should be
removed, of course. I'm talking about the direction.

For now, there's no tool to track wait/event itself in Linux kernel -
a subset of the functionality exists tho. DEPT is the 1st try for that
purpose and can be a useful tool by the right direction.

I know what you are concerning about. I bet it's false positives that
are going to bother you once merged. I'll insist that DEPT shouldn't be
used as a mandatory testing tool until considered stable enough. But
what about ones who would take the advantage use DEPT. Why don't you
think of folks who will take the advantage from the hints about
dependency of synchronization esp. when their subsystem requires very
complicated synchronization? Should a tool be useful only in a final
testing stage? What about the usefulness during development stage?

It's worth noting DEPT works with any wait/event so any lockups e.g.
even by HW-SW interface, retry logic or the like can be detected by DEPT
once all waits and events are tagged properly. I believe the advantage
by that is much higher than the bad side facing false alarms. It's just
my opinion. I'm goning to respect the majority opinion.

	Byungchul
> 
> 					- Ted
> 

  reply	other threads:[~2022-05-10  5:39 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-04  8:17 [PATCH RFC v6 00/21] DEPT(Dependency Tracker) Byungchul Park
2022-05-04  8:17 ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 01/21] llist: Move llist_{head,node} definition to types.h Byungchul Park
2022-05-04  8:17   ` [PATCH RFC v6 01/21] llist: Move llist_{head, node} " Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 02/21] dept: Implement Dept(Dependency Tracker) Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04 13:29   ` kernel test robot
2022-05-21  3:24   ` Hyeonggon Yoo
2022-05-21  3:24     ` Hyeonggon Yoo
2022-05-04  8:17 ` [PATCH RFC v6 03/21] dept: Apply Dept to spinlock Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 04/21] dept: Apply Dept to mutex families Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 05/21] dept: Apply Dept to rwlock Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 06/21] dept: Apply Dept to wait_for_completion()/complete() Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 07/21] dept: Apply Dept to seqlock Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-21  5:25   ` Hyeonggon Yoo
2022-05-21  5:25     ` Hyeonggon Yoo
2022-05-24  6:00     ` Byungchul Park
2022-05-24  6:00       ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 08/21] dept: Apply Dept to rwsem Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 09/21] dept: Add proc knobs to show stats and dependency graph Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 10/21] dept: Introduce split map concept and new APIs for them Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 11/21] dept: Apply Dept to wait/event of PG_{locked,writeback} Byungchul Park
2022-05-04  8:17   ` [PATCH RFC v6 11/21] dept: Apply Dept to wait/event of PG_{locked, writeback} Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 12/21] dept: Apply SDT to swait Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 13/21] dept: Apply SDT to wait(waitqueue) Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 14/21] locking/lockdep, cpu/hotplus: Use a weaker annotation in AP thread Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 15/21] dept: Distinguish each syscall context from another Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 16/21] dept: Distinguish each work " Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04 11:23   ` Sergey Shtylyov
2022-05-04 11:23     ` Sergey Shtylyov
2022-05-04  8:17 ` [PATCH RFC v6 17/21] dept: Disable Dept within the wait_bit layer by default Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 18/21] dept: Disable Dept on struct crypto_larval's completion for now Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 19/21] dept: Differentiate onstack maps from others of different tasks in class Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 20/21] dept: Do not add dependencies between events within scheduler and sleeps Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04  8:17 ` [PATCH RFC v6 21/21] dept: Unstage wait when tagging a normal sleep wait Byungchul Park
2022-05-04  8:17   ` Byungchul Park
2022-05-04 18:17 ` [PATCH RFC v6 00/21] DEPT(Dependency Tracker) Linus Torvalds
2022-05-04 18:17   ` Linus Torvalds
2022-05-06  0:11   ` Byungchul Park
2022-05-06  0:11     ` Byungchul Park
2022-05-07  7:20     ` Hyeonggon Yoo
2022-05-07  7:20       ` Hyeonggon Yoo
2022-05-09  0:16       ` Byungchul Park
2022-05-09  0:16         ` Byungchul Park
2022-05-09 20:47         ` Steven Rostedt
2022-05-09 20:47           ` Steven Rostedt
2022-05-09 23:38           ` Byungchul Park
2022-05-09 23:38             ` Byungchul Park
2022-05-10 14:12             ` Steven Rostedt
2022-05-10 14:12               ` Steven Rostedt
2022-05-10 23:26               ` Byungchul Park
2022-05-10 23:26                 ` Byungchul Park
2022-05-10 11:18         ` Hyeonggon Yoo
2022-05-10 11:18           ` Hyeonggon Yoo
2022-05-10 23:39           ` Byungchul Park
2022-05-10 23:39             ` Byungchul Park
2022-05-11 10:04             ` Hyeonggon Yoo
2022-05-11 10:04               ` Hyeonggon Yoo
2022-05-19 10:11               ` Catalin Marinas
2022-05-19 10:11                 ` Catalin Marinas
2022-05-23  2:43                 ` Byungchul Park
2022-05-23  2:43                   ` Byungchul Park
2022-05-09  1:22   ` Byungchul Park
2022-05-09  1:22     ` Byungchul Park
2022-05-09 21:05 ` Theodore Ts'o
2022-05-09 21:05   ` Theodore Ts'o
2022-05-09 22:28   ` Theodore Ts'o
2022-05-09 22:28     ` Theodore Ts'o
2022-05-10  0:32     ` Byungchul Park
2022-05-10  0:32       ` Byungchul Park
2022-05-10  1:32       ` Theodore Ts'o
2022-05-10  1:32         ` Theodore Ts'o
2022-05-10  5:37         ` Byungchul Park [this message]
2022-05-10  5:37           ` Byungchul Park
2022-05-11  1:16           ` Byungchul Park
2022-05-11  1:16             ` Byungchul Park
2022-05-12  5:25 ` [REPORT] syscall reboot + umh + firmware fallback Byungchul Park
2022-05-12  5:25   ` Byungchul Park
2022-05-12  9:15   ` Tejun Heo
2022-05-12  9:15     ` Tejun Heo
2022-05-12 11:18     ` Byungchul Park
2022-05-12 11:18       ` Byungchul Park
2022-05-12 13:56       ` Theodore Ts'o
2022-05-12 13:56         ` Theodore Ts'o
2022-05-23  1:10         ` Byungchul Park
2022-05-23  1:10           ` Byungchul Park
2022-05-12 16:41       ` Tejun Heo
2022-05-12 16:41         ` Tejun Heo

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=1652161060-26531-1-git-send-email-byungchul.park@lge.com \
    --to=byungchul.park@lge.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=akpm@linux-foundation.org \
    --cc=amir73il@gmail.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=cl@linux.com \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=david@fromorbit.com \
    --cc=dennis@kernel.org \
    --cc=djwong@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=duyuyang@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hamohammed.sa@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=hch@infradead.org \
    --cc=jack@suse.com \
    --cc=jack@suse.cz \
    --cc=jglisse@redhat.com \
    --cc=jlayton@kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=johannes.berg@intel.com \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@lge.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=melissa.srw@gmail.com \
    --cc=mhocko@kernel.org \
    --cc=minchan@kernel.org \
    --cc=mingo@redhat.com \
    --cc=ngupta@vflare.org \
    --cc=paolo.valente@linaro.org \
    --cc=penberg@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=rodrigosiqueiramelo@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=sashal@kernel.org \
    --cc=sj@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=vbabka@suse.cz \
    --cc=vdavydov.dev@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.