Re: About the try to remove cross-release feature entirely by Ingo

On 1/3/2018 5:10 PM, Byungchul Park wrote: On 1/3/2018 4:05 PM, Theodore Ts'o wrote: On Wed, Jan 03, 2018 at 11:10:37AM +0900, Byungchul Park wrote: The point I was trying to drive home is that "all we have to do is just classify everything well or just invalidate the right lock

Re: About the try to remove cross-release feature entirely by Ingo

On 1/3/2018 4:05 PM, Theodore Ts'o wrote: On Wed, Jan 03, 2018 at 11:10:37AM +0900, Byungchul Park wrote: The point I was trying to drive home is that "all we have to do is just classify everything well or just invalidate the right lock Just to be sure, we don't have to invalidate

Re: About the try to remove cross-release feature entirely by Ingo

On 1/3/2018 11:58 AM, Dave Chinner wrote: On Wed, Jan 03, 2018 at 11:28:44AM +0900, Byungchul Park wrote: On 1/1/2018 7:18 PM, Matthew Wilcox wrote: On Sat, Dec 30, 2017 at 06:00:57PM -0500, Theodore Ts'o wrote: Also, what to do with TCP connections which are created in userspace (with some

Re: About the try to remove cross-release feature entirely by Ingo

On 1/2/2018 1:00 AM, Theodore Ts'o wrote: On Mon, Jan 01, 2018 at 02:18:55AM -0800, Matthew Wilcox wrote: Clarification: all TCP connections that are used by kernel code would need to be in their own separate lock class. All TCP connections used only by userspace could be in their own shared

Re: About the try to remove cross-release feature entirely by Ingo

On 12/31/2017 7:40 AM, Theodore Ts'o wrote: On Sat, Dec 30, 2017 at 12:44:17PM -0800, Matthew Wilcox wrote: I'm not sure I agree with this part. What if we add a new TCP lock class for connections which are used for filesystems/network block devices/...? Yes, it'll be up to each user to set

Re: About the try to remove cross-release feature entirely by Ingo

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

Re: About the try to remove cross-release feature entirely by Ingo

On 12/30/2017 3:16 PM, Matthew Wilcox wrote: On Fri, Dec 29, 2017 at 04:28:51PM +0900, Byungchul Park wrote: On Thu, Dec 28, 2017 at 10:51:46PM -0500, Theodore Ts'o wrote: On Fri, Dec 29, 2017 at 10:47:36AM +0900, Byungchul Park wrote: (1) The best way: To classify all waiters correctly

Re: About the try to remove cross-release feature entirely by Ingo

On 12/29/2017 5:09 PM, Amir Goldstein wrote: On Fri, Dec 29, 2017 at 3:47 AM, Byungchul Park <byungchul.p...@lge.com> wrote: 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 relati

Re: About the try to remove cross-release feature entirely by Ingo

On Thu, Dec 28, 2017 at 10:51:46PM -0500, Theodore Ts'o wrote: > On Fri, Dec 29, 2017 at 10:47:36AM +0900, Byungchul Park wrote: > > > >(1) The best way: To classify all waiters correctly. > > It's really not all waiters, but all *locks*, no? Thanks for your opinion.

Re: About the try to remove cross-release feature entirely by Ingo

On Fri, Dec 29, 2017 at 10:47:36AM +0900, Byungchul Park wrote: > 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

Re: About the try to remove cross-release feature entirely by Ingo

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. &

Re: [PATCH] locking/lockdep: Remove the cross-release locking checks

On Sat, Dec 16, 2017 at 6:15 AM, Theodore Ts'o <ty...@mit.edu> wrote: > On Fri, Dec 15, 2017 at 05:39:25PM +0900, Byungchul Park wrote: >> >> All locks should belong to one class if each path of acquisition >> can be switchable each other within the class at any time

Re: [PATCH] locking/lockdep: Remove the cross-release locking checks

s, using e.g. completion_init_nomap(). >From: Byungchul Park <byungchul.p...@lge.com> >Date: Fri, 8 Dec 2017 18:27:45 +0900 >Subject: Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray > >1) Firstly, it's hard to assign lock classes *properly*. By >default, it relies on

Re: [PATCH] locking/lockdep: Remove the cross-release locking checks

On Fri, Dec 15, 2017 at 3:24 PM, Theodore Ts'o <ty...@mit.edu> wrote: > On Fri, Dec 15, 2017 at 01:05:43PM +0900, Byungchul Park wrote: >> For example, in the case of fs issues, for now we can >> invalidate wait_for_completion() in submit_bio_wait() > > And this

Re: [PATCH] locking/lockdep: Remove the cross-release locking checks

On Thu, Dec 14, 2017 at 2:01 PM, Byungchul Park <max.byungchul.p...@gmail.com> wrote: > On Wed, Dec 13, 2017 at 7:46 PM, Ingo Molnar <mi...@kernel.org> wrote: >> >> * Byungchul Park <max.byungchul.p...@gmail.com> wrote: >> >>> Lockdep works, based on

Re: About the try to remove cross-release feature entirely by Ingo

On Thu, Dec 14, 2017 at 8:18 PM, Peter Zijlstra wrote: > On Wed, Dec 13, 2017 at 10:07:11PM -0500, Theodore Ts'o wrote: >> interpreted this as the lockdep maintainers saying, "hey, not my >> fault, it's the subsystem maintainer's fault for not properly >> classifying the

Re: About the try to remove cross-release feature entirely by Ingo

On Thu, Dec 14, 2017 at 12:07 PM, Theodore Ts'o <ty...@mit.edu> wrote: > On Wed, Dec 13, 2017 at 04:13:07PM +0900, Byungchul Park wrote: >> >> Therefore, I want to say the fundamental problem >> comes from classification, not cross-release >> specific. >

Re: [PATCH] locking/lockdep: Remove the cross-release locking checks

On Wed, Dec 13, 2017 at 7:46 PM, Ingo Molnar <mi...@kernel.org> wrote: > > * Byungchul Park <max.byungchul.p...@gmail.com> wrote: > >> Lockdep works, based on the following: >> >>(1) Classifying locks properly >>(2) Checking relationship betwe

Re: About the try to remove cross-release feature entirely by Ingo

On Wed, Dec 13, 2017 at 3:24 PM, Byungchul Park <max.byungchul.p...@gmail.com> 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 >

About the try to remove cross-release feature entirely by Ingo

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

Re: [PATCH] locking/lockdep: Add CONFIG_LOCKDEP_AGGRESSIVE

On 12/13/2017 2:00 AM, Linus Torvalds wrote: On Mon, Dec 11, 2017 at 9:20 PM, Byungchul Park <byungchul.p...@lge.com> wrote: The *problem* is false positives, since locks and waiters in kernel are not classified properly So the problem is that those false positives apparently end up

Re: [PATCH] locking/lockdep: Add CONFIG_LOCKDEP_AGGRESSIVE

On 12/12/2017 10:03 PM, Theodore Ts'o wrote: On Tue, Dec 12, 2017 at 02:20:32PM +0900, Byungchul Park wrote: The *problem* is false positives, since locks and waiters in kernel are not classified properly, at the moment, which is just a fact that is not related to cross-release stuff at all

Re: [PATCH] locking/lockdep: Make CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS optional

+cc da...@fromorbit.com +cc ty...@mit.edu +cc wi...@infradead.org +cc torva...@linux-foundation.org +cc amir7...@gmail.com On 12/12/2017 4:11 PM, Byungchul Park wrote: At the moment, it's rather premature to enable CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS by default, because

[PATCH] locking/lockdep: Make CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS optional

to be optional. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- lib/Kconfig.debug | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 2689b7c..bc099f1 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1092,8 +

Re: [PATCH] locking/lockdep: Add CONFIG_LOCKDEP_AGGRESSIVE

On 12/12/2017 6:06 AM, Linus Torvalds wrote: On Sun, Dec 10, 2017 at 7:50 PM, Theodore Ts'o wrote: CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS can result in a large number of false positives because lockdep doesn't understand how to deal with multiple stacked loop

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

On 12/5/2017 2:46 PM, Byungchul Park wrote: On 12/5/2017 2:30 PM, Matthew Wilcox wrote: On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote: For now, wait_for_completion() / complete() works with lockdep, add lock_page() / unlock_page() and its family to lockdep support. Changes

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

On 12/5/2017 2:30 PM, Matthew Wilcox wrote: On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote: For now, wait_for_completion() / complete() works with lockdep, add lock_page() / unlock_page() and its family to lockdep support. Changes from v1 - Move lockdep_map_cross outside

[PATCH v2 4/4] lockdep: Add a boot parameter enabling to track page locks using lockdep and disable it by default

To track page locks using lockdep, we need a huge memory space for lockdep_map per page. So, it would be better to make it disabled by default and provide a boot parameter to turn it on. Do it. Suggested-by: Michal Hocko <mho...@kernel.org> Signed-off-by: Byungchul Park <byungchul.p..

[PATCH v2 2/4] lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked

frequently. We might miss many chances to check deadlock if we ignore it. Make __Set(__Clear)PageLockded considered as well. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/page-flags.h | 30 +- 1 file changed, 29 insertions(+), 1 deletion(-)

[PATCH v2 3/4] lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext

CONFIG_LOCKDEP_PAGELOCK needs to keep lockdep_map_cross per page. Since it's a debug feature, it's preferred to keep it in struct page_ext rather than struct page. Move it to struct page_ext. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/mm_types.h | 4 --- i

[PATCH v2 1/4] lockdep: Apply crossrelease to PG_locked locks

locks. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/mm_types.h | 8 include/linux/pagemap.h | 101 --- lib/Kconfig.debug| 7 mm/filemap.c | 4 +- mm/page_alloc.c | 3 ++ 5

[PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

allowing the allocation for debugging Byungchul Park (4): lockdep: Apply crossrelease to PG_locked locks lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext lockdep: Add a boot parameter enabling to track page locks

Re: [PATCH] locking/Documentation: Revise Documentation/locking/crossrelease.txt

On Thu, Nov 16, 2017 at 08:22:37AM +0100, Ingo Molnar wrote: > > * Byungchul Park <byungchul.p...@lge.com> wrote: > > > On Sat, Nov 11, 2017 at 10:45:24PM +0900, Byungchul Park wrote: > > > This is the big one including all of version 3. > > > > >

Re: [PATCH 1/3] lockdep: Apply crossrelease to PG_locked locks

On Thu, Nov 16, 2017 at 02:07:46PM +0100, Michal Hocko wrote: > On Thu 16-11-17 21:48:05, Byungchul Park wrote: > > On 11/16/2017 9:02 PM, Michal Hocko wrote: > > > for each struct page. So you are doubling the size. Who is going to > > > enable this config option? You

Re: [PATCH 1/3] lockdep: Apply crossrelease to PG_locked locks

On 11/16/2017 9:02 PM, Michal Hocko wrote: for each struct page. So you are doubling the size. Who is going to enable this config option? You are moving this to page_ext in a later patch which is a good step but it doesn't go far enough because this still consumes those resources. Is there any

Re: [PATCH] locking/Documentation: Revise Documentation/locking/crossrelease.txt

On 11/16/2017 4:22 PM, Ingo Molnar wrote: * Byungchul Park <byungchul.p...@lge.com> wrote: On Sat, Nov 11, 2017 at 10:45:24PM +0900, Byungchul Park wrote: This is the big one including all of version 3. You can take only this. Hello Ingo, Could you consider this? Yeah, I'll have

[PATCH 0/3] lockdep/crossrelease: Apply crossrelease to page locks

For now, wait_for_completion() / complete() works with lockdep. Add lock_page() / unlock_page() and its family to lockdep support. Byungchul Park (3): lockdep: Apply crossrelease to PG_locked locks lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked lockdep: Move data

[PATCH 1/3] lockdep: Apply crossrelease to PG_locked locks

locks. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/mm_types.h | 8 include/linux/pagemap.h | 101 --- lib/Kconfig.debug| 7 mm/filemap.c | 4 +- mm/page_alloc.c | 3 ++ 5

[PATCH 3/3] lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext

CONFIG_LOCKDEP_PAGELOCK needs to keep lockdep_map_cross per page. Since it's a debug feature, it's preferred to keep it in struct page_ext rather than struct page. Move it to struct page_ext. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/mm_types.h | 4 --- i

[PATCH 2/3] lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked

frequently. We might miss many chances to check deadlock if we ignore it. Make __Set(__Clear)PageLockded considered as well. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/page-flags.h | 30 +- 1 file changed, 29 insertions(+), 1 deletion(-)

Re: [PATCH] locking/Documentation: Revise Documentation/locking/crossrelease.txt

On Sat, Nov 11, 2017 at 10:45:24PM +0900, Byungchul Park wrote: > This is the big one including all of version 3. > > You can take only this. Hello Ingo, Could you consider this? I want to offer a better base to someone who helps the doc enhanced. Of course, in the case

[PATCH] locking/Documentation: Revise Documentation/locking/crossrelease.txt

Revise Documentation/locking/crossrelease.txt to improve its readability. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/locking/crossrelease.txt | 329 - 1 file changed, 155 insertions(+), 174 deletions(-) diff --git a/Documen

[PATCH v3 0/5] Revise crossrelease.txt

Ingo pointed out. - Leave original contents unchanged as much as possible. Changes from v1 - Run several tools checking english spell and grammar over the text. - Simplify the document more. Byungchul Park (5): locking/Documentation: Remove meaningless examples and a note locking

[PATCH v3 3/5] locking/Documentation: Fix weird expressions.

Fix Weird expressions not reported by checker tools by myself. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/locking/crossrelease.txt | 87 ++ 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/Documentation/l

[PATCH v3 5/5] locking/Documentation: Align crossrelease.txt with the width

No change of contents at all. Only adjust the width. (Please merge this to another after the review.) Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/locking/crossrelease.txt | 59 +- 1 file changed, 30 insertions(+), 29 del

[PATCH v3 4/5] locking/Documentation: Add an example to help crossrelease.txt more readable

Add an example explaining the rationale that the limitation that old lockdep implies, can be relaxed. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/locking/crossrelease.txt | 23 +++ 1 file changed, 23 insertions(+) diff --git a/Documen

[PATCH v3 2/5] locking/Documentation: Fix typos and clear grammar errors

crossrelease.txt includes many typos and grammar errors. Fix them using a few spell checkers and grammar checkers. Clear errors are also fixed by myself. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/locking/crossrelease.txt | 170 --

[PATCH v3 1/5] locking/Documentation: Remove meaningless examples and a note

crossrelease.txt is too verbose and includes two meaningless examples and an unnecessary note. Remove them. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/locking/crossrelease.txt | 48 +- 1 file changed, 1 insertion(+), 47 del

Re: [PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

On 11/10/2017 4:30 PM, Ingo Molnar wrote: * Byungchul Park <byungchul.p...@lge.com> wrote: Event C depends on event A. Event A depends on event B. Event B depends on event C. - NOTE: Precisely speaking, a dependency is one between whether a - waiter for an eve

Re: [PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

On Thu, Nov 09, 2017 at 04:20:36PM +0900, Byungchul Park wrote: > Changes from v1 > - Run several tools checking english spell and grammar over the text. > - Simplify the document more. Checker tools also reported other words e.g. crosslock, crossrelease, lockdep, mutex, lockles

[PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

Changes from v1 - Run several tools checking english spell and grammar over the text. - Simplify the document more. -8<- >From 412bc9eb0d22791f70f7364bda189feb41899ff9 Mon Sep 17 00:00:00 2001 From: Byungchul Park <byungchul.p...@lge.com> Date: Thu, 9 Nov 2017 16:12:23 +

[PATCH] locking/lockdep: Revise Documentation/locking/crossrelease.txt

t;8- >From c7795104ca6ac6dd9f7fd944aee23a2011a6d3a2 Mon Sep 17 00:00:00 2001 From: Byungchul Park <byungchul.p...@lge.com> Date: Mon, 30 Oct 2017 14:51:26 +0900 Subject: [PATCH] locking/lockdep: Revise Documentation/locking/crossrelease.txt The document should've been written with a better

Re: [PATCH v5 9/9] block: Assign a lock_class per gendisk used for wait_for_completion()

On Thu, Oct 26, 2017 at 07:50:46AM +0200, Ingo Molnar wrote: > > * Jens Axboe <ax...@kernel.dk> wrote: > > > On 10/25/2017 03:13 AM, Ingo Molnar wrote: > > > > > > * Byungchul Park <byungchul.p...@lge.com> wrote: > > > > > >&

Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map

On Wed, Oct 25, 2017 at 07:07:06AM +, Bart Van Assche wrote: > > Please, point out logical problems of cross-release than saying it's > > impossbile according to the paper. > > Isn't that the same? If it's impossible to use lock-graphs for detecting > deadlocks > in programs that use

[PATCH v5 4/9] locking/lockdep: Add a boot parameter allowing unwind in cross-release and disable it by default

n Hovold <jo...@kernel.org> Analyzed-by: Thomas Gleixner <t...@linutronix.de> Suggested-by: Thomas Gleixner <t...@linutronix.de> Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/admin-guide/kernel-parameters.txt | 3 +++ kernel/locking/lockdep.c

[PATCH v5 1/9] block: use DECLARE_COMPLETION_ONSTACK in submit_bio_wait

From: Christoph Hellwig Simplify the code by getting rid of the submit_bio_ret structure. Signed-off-by: Christoph Hellwig --- block/bio.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/block/bio.c b/block/bio.c index

[PATCH v5 5/9] locking/lockdep: Remove the BROKEN flag from CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS

Now that the performance regression is fixed, re-enable CONFIG_LOCKDEP_CROSSRELEASE=y and CONFIG_LOCKDEP_COMPLETIONS=y. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- lib/Kconfig.debug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Kconfig.debug

[PATCH v5 9/9] block: Assign a lock_class per gendisk used for wait_for_completion()

assign different lock classes to different devices is to do it for each gendisk. In other words, this patch assigns a lockdep_map per gendisk and uses it when initializing completion in submit_bio_wait(). Of course, it might be too conservative. But, making it safest for now and extended by

[PATCH v5 2/9] locking/lockdep: Provide empty lockdep_map structure for !CONFIG_LOCKDEP

By this patch, the lockdep_map structure takes no space if lockdep is disabled, making a debug facility's impact on unreleated kernel less. Thanks to this, we don't need #ifdef to sparate code due to the lockdep_map structure. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- i

[PATCH v5 6/9] locking/lockdep: Introduce CONFIG_BOOTPARAM_LOCKDEP_CROSSRELEASE_FULLSTACK

The boot parameter, crossrelease_fullstack, was introduced to control whether to enable unwind in cross-release or not. Add a Kconfig doing the same thing. Suggested-by: Ingo Molnar <mi...@kernel.org> Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- kernel/locking/lo

[PATCH v5 7/9] completion: Add support for initializing completion with lockdep_map

completions in that way. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/completion.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/linux/completion.h b/include/linux/completion.h index 9121803..4da4991 100644 --- a/include/linux/completion.h

[PATCH v5 8/9] workqueue: Remove unnecessary acquisitions wrt workqueue flush

The workqueue added manual acquisitions to catch deadlock cases. Now crossrelease was introduced, some of those are redundant, since wait_for_completion() already includes the acquisition for itself. Remove it. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/workq

[PATCH v5 0/9] cross-release: Enhence performance and fix false positives

go suggested - Fix commit message writing out CONFIG_ variable - Introduce a new kernel parameter, crossrelease_fullstack - Replace the number with the output of *perf* - Separate a patch removing white space Byungchul Park (8): locking/lockdep: Provide empty lockdep_map structure for !CON

Re: [PATCH v4 7/7] block: Assign a lock_class per gendisk used for wait_for_completion()

On Wed, Oct 25, 2017 at 08:01:24AM +0200, Ingo Molnar wrote: > Beyond the #ifdef reduction I mentioned in the other thread, there's four > other > things I noticed that need to be fixed in this patch: > > - Please write out 'minor' instead of the 'm' abbreviation that is > meaningless. >

[PATCH v4 7/7] block: Assign a lock_class per gendisk used for wait_for_completion()

assign different lock classes to different devices is to do it for each gendisk. In other words, this patch assigns a lockdep_map per gendisk and uses it when initializing completion in submit_bio_wait(). Of course, it might be too conservative. But, making it safest for now and extended by

[PATCH v4 1/7] block: use DECLARE_COMPLETION_ONSTACK in submit_bio_wait

From: Christoph Hellwig Simplify the code by getting rid of the submit_bio_ret structure. Signed-off-by: Christoph Hellwig --- block/bio.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/block/bio.c b/block/bio.c index

[PATCH v4 6/7] workqueue: Remove unnecessary acquisitions wrt workqueue flush

The workqueue added manual acquisitions to catch deadlock cases. Now crossrelease was introduced, some of those are redundant, since wait_for_completion() already includes the acquisition for itself. Removed it. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/workq

[PATCH v4 2/7] locking/lockdep: Add a boot parameter allowing unwind in cross-release and disable it by default

ter stats for 'qemu_booting_time.sh bzImage' (10 runs): 2.963669551 seconds time elapsed( +- 0.11% ) I.e. lockdep cross-release performance is now indistinguishable from vanilla lockdep. Reported-by: Johan Hovold <jo...@kernel.org> Suggested-by: Thomas

[PATCH v4 3/7] locking/lockdep: Remove the BROKEN flag from CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS

Now that the performance regression is fixed, re-enable CONFIG_LOCKDEP_CROSSRELEASE=y and CONFIG_LOCKDEP_COMPLETIONS=y. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- lib/Kconfig.debug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Kconfig.debug

[PATCH v4 5/7] completion: Add support for initializing completion with lockdep_map

completions in that way. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/completion.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/linux/completion.h b/include/linux/completion.h index cae5400..02f8cde 100644 --- a/include/linux/completion.h

[PATCH v4 0/7] cross-release: Enhence performance and fix false positives

as Ingo suggested - Fix commit message writing out CONFIG_ variable - Introduce a new kernel parameter, crossrelease_fullstack - Replace the number with the output of *perf* - Separate a patch removing white space Byungchul Park (6): locking/lockdep: Add a boot parameter allowing unwind in cross

Re: [PATCH v3 4/8] lockdep: Add a kernel parameter, crossrelease_fullstack

On Tue, Oct 24, 2017 at 12:08:58PM +0200, Ingo Molnar wrote: > This is really unnecessarily complex. I mis-understood your suggestion. I will change it. > The proper logic is to introduce the crossrelease_fullstack boot parameter, > and to > also have a Kconfig option that enables it: > >

[PATCH v3 1/8] block: use DECLARE_COMPLETION_ONSTACK in submit_bio_wait

From: Christoph Hellwig Simplify the code by getting rid of the submit_bio_ret structure. Signed-off-by: Christoph Hellwig --- block/bio.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/block/bio.c b/block/bio.c index

[PATCH v3 6/8] lockdep: Remove unnecessary acquisitions wrt workqueue flush

The workqueue added manual acquisitions to catch deadlock cases. Now crossrelease was introduced, some of those are redundant, since wait_for_completion() already includes the acquisition for itself. Removed it. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/workq

[PATCH v3 8/8] block: Assign a lock_class per gendisk used for wait_for_completion()

fferent lock classes to different devices is to do it for each gendisk. In other words, this patch assigns a lockdep_map per gendisk and uses it when initializing completion in submit_bio_wait(). Of course, it might be too conservative. But, making it safest for now and extended by block layer e

[PATCH v3 7/8] genhd.h: Remove trailing white space

Trailing white space is not accepted in kernel coding style. Remove them. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/genhd.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/genhd.h b/include/linux/genhd.h index ea652bf..6

[PATCH v3 3/8] lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE

Now the performance regression was fixed, re-enable CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- lib/Kconfig.debug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Kconfig.debug

[PATCH v3 5/8] completion: Add support for initializing completion with lockdep_map

completions in that way. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/completion.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/linux/completion.h b/include/linux/completion.h index cae5400..02f8cde 100644 --- a/include/linux/completion.h

[PATCH v3 2/8] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default

ter stats for 'qemu_booting_time.sh bzImage' (10 runs): 2.963669551 seconds time elapsed ( +- 0.11% ) Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/lockdep.h | 4 kernel/locking/lockdep.c | 5 + lib/Kconfig.debug| 15 +++

[PATCH v3 4/8] lockdep: Add a kernel parameter, crossrelease_fullstack

. Change CONFIG_CROSSRELEASE_STACK_TRACE default from N to Y, and introduce the new kernel parameter. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- Documentation/admin-guide/kernel-parameters.txt | 3 +++ kernel/locking/lockdep.c| 18 -

[PATCH v3 0/8] cross-release: enhence performance and fix false positives

kconfig description as Ingo suggested - Fix commit message writing out CONFIG_ variable - Introduce a new kernel parameter, crossrelease_fullstack - Replace the number with the output of *perf* - Separate a patch removing white space Byungchul Park (7): lockdep: Introduce CROSSRELEASE_STACK_TRACE

Re: [PATCH v2 4/4] lockdep: Assign a lock_class per gendisk used for wait_for_completion()

On Sun, Oct 22, 2017 at 11:36:30PM -0700, Christoph Hellwig wrote: > On Mon, Oct 23, 2017 at 08:53:35AM +0900, Byungchul Park wrote: > > On Fri, Oct 20, 2017 at 07:44:51AM -0700, Christoph Hellwig wrote: > > > The Subject prefix for this should be "block:". > >

Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map

On Sun, Oct 22, 2017 at 02:34:56PM +, Bart Van Assche wrote: > On Sat, 2017-10-21 at 11:23 +0900, Byungchul Park wrote: > > On Sat, Oct 21, 2017 at 4:58 AM, Bart Van Assche <bart.vanass...@wdc.com> > > wrote: > > > As explained in another e-mail thread, unli

Re: [PATCH v2 4/4] lockdep: Assign a lock_class per gendisk used for wait_for_completion()

On Fri, Oct 20, 2017 at 07:44:51AM -0700, Christoph Hellwig wrote: > The Subject prefix for this should be "block:". > > > @@ -945,7 +945,7 @@ int submit_bio_wait(struct bio *bio) > > { > > struct submit_bio_ret ret; > > > > - init_completion(); > > + init_completion_with_map(,

Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map

On Sat, Oct 21, 2017 at 4:58 AM, Bart Van Assche wrote: > Sorry but I'm not sure that's the best possible answer. In my opinion > avoiding that completion objects have dependencies on other lock objects, > e.g. by avoiding to wait on a completion object while holding a

Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map

On Thu, Oct 19, 2017 at 11:24:00PM +, Bart Van Assche wrote: > Are there any completion objects for which the cross-release checking is > useful? Are there any wait_for_completion() callers that hold a mutex or > other locking object? Check /proc/lockdep, then you can find all dependencies

[PATCH v2 4/4] lockdep: Assign a lock_class per gendisk used for wait_for_completion()

fferent lock classes to different devices is to do it for each gendisk. In other words, this patch assigns a lockdep_map per gendisk and uses it when initializing completion in submit_bio_wait(). Of course, it might be too conservative. But, making it safest for now and extended by block layer e

[PATCH v2 3/4] genhd.h: Remove trailing white space

Trailing white space is not accepted in kernel coding style. Remove them. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/genhd.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/genhd.h b/include/linux/genhd.h index ea652bf..6

[PATCH v2 2/4] lockdep: Remove unnecessary acquisitions wrt workqueue flush

The workqueue added manual acquisitions to catch deadlock cases. Now crossrelease was introduced, some of those are redundant, since wait_for_completion() already includes the acquisition for itself. Removed it. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/workq

[PATCH v2 1/4] completion: Add support for initializing completion with lockdep_map

Sometimes, we want to initialize completions with sparate lockdep maps to assign lock classes under control. For example, the workqueue code manages lockdep maps, as it can classify lockdep maps properly. Provided a function for that purpose. Signed-off-by: Byungchul Park <byungchul.p...@lge.

Re: [RESEND PATCH 3/3] lockdep: Assign a lock_class per gendisk used for wait_for_completion()

On Wed, Oct 18, 2017 at 11:59:16AM +0200, Ingo Molnar wrote: > > * Byungchul Park <byungchul.p...@lge.com> wrote: > > > diff --git a/block/bio.c b/block/bio.c > > index 9a63597..0d4d6c0 100644 > > --- a/block/bio.c > > +++ b/block/bio.c > > @@ -941,7

Re: Fix false positive by LOCKDEP_CROSSRELEASE

On Wed, Oct 18, 2017 at 02:29:56PM +, Bart Van Assche wrote: > On Wed, 2017-10-18 at 18:38 +0900, Byungchul Park wrote: > > Several false positives were reported, so I tried to fix them. > > > > It would be appreciated if you tell me if it works as expected, or let >

[RESEND PATCH 2/3] lockdep: Remove unnecessary acquisitions wrt workqueue flush

The workqueue added manual acquisitions to catch deadlock cases. Now crossrelease was introduced, some of those are redundant, since wait_for_completion() already includes the acquisition for itself. Removed it. Signed-off-by: Byungchul Park <byungchul.p...@lge.com> --- include/linux/workq

[RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map

Sometimes, we want to initialize completions with sparate lockdep maps to assign lock classes under control. For example, the workqueue code manages lockdep maps, as it can classify lockdep maps properly. Provided a function for that purpose. Signed-off-by: Byungchul Park <byungchul.p...@lge.

[RESEND PATCH 3/3] lockdep: Assign a lock_class per gendisk used for wait_for_completion()

fferent lock classes to different devices is to do it for each gendisk. In other words, this patch assigns a lockdep_map per gendisk and uses it when initializing completion in submit_bio_wait(). Of course, it might be too conservative. But, making it safest for now and extended by block layer experts

Fix false positive by LOCKDEP_CROSSRELEASE

Several false positives were reported, so I tried to fix them. It would be appreciated if you tell me if it works as expected, or let me know your opinion. Thank you, Byungchul

RE: possible circular locking dependency detected [was: linux-next: Tree for Aug 22]

> -Original Message- > From: Peter Zijlstra [mailto:pet...@infradead.org] > Sent: Wednesday, August 30, 2017 5:48 PM > To: Sergey Senozhatsky > Cc: Byungchul Park; Bart Van Assche; linux-ker...@vger.kernel.org; linux- > bl...@vger.kernel.org; martin.peter...@oracle.co

Re: possible circular locking dependency detected [was: linux-next: Tree for Aug 22]

On Wed, Aug 30, 2017 at 02:20:37PM +0900, Sergey Senozhatsky wrote: > Byungchul, a quick question. Hello Sergey, > have you measured the performance impact? somehow my linux-next is Yeah, it might have performance impact inevitably. > notably slower than earlier 4.13 linux-next. (e.g.

Re: possible circular locking dependency detected [was: linux-next: Tree for Aug 22]

On Wed, Aug 23, 2017 at 12:38:13PM +0800, Boqun Feng wrote: > From: Boqun Feng > Date: Wed, 23 Aug 2017 12:12:16 +0800 > Subject: [PATCH] lockdep: Print proper scenario if cross deadlock detected at > acquisition time > > For a potential deadlock about CROSSRELEASE as

Re: possible circular locking dependency detected [was: linux-next: Tree for Aug 22]

On Wed, Aug 23, 2017 at 11:49:51AM +0800, Boqun Feng wrote: > Hi Byungchul, > > On Wed, Aug 23, 2017 at 09:03:04AM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 09:43:56PM +, Bart Van Assche wrote: > > > On Tue, 2017-08-22 at 19:47 +0900,

  1   2   >