All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tglx@linutronix.de" <tglx@linutronix.de>
Cc: "mingo@kernel.org" <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"hch@infradead.org" <hch@infradead.org>,
	"amir73il@gmail.com" <amir73il@gmail.com>,
	"linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"darrick.wong@oracle.com" <darrick.wong@oracle.com>,
	"johannes.berg@intel.com" <johannes.berg@intel.com>,
	"byungchul.park@lge.com" <byungchul.park@lge.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"idryomov@gmail.com" <idryomov@gmail.com>,
	"tj@kernel.org" <tj@kernel.org>,
	"kernel-team@lge.com" <kernel-team@lge.com>,
	"david@fromorbit.com" <david@fromorbit.com>
Subject: Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map
Date: Fri, 20 Oct 2017 19:58:54 +0000	[thread overview]
Message-ID: <1508529532.3029.15.camel@wdc.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1710200829340.3083@nanos>

T24gRnJpLCAyMDE3LTEwLTIwIGF0IDA4OjM0ICswMjAwLCBUaG9tYXMgR2xlaXhuZXIgd3JvdGU6
DQo+IE9uIFRodSwgMTkgT2N0IDIwMTcsIEJhcnQgVmFuIEFzc2NoZSB3cm90ZToNCj4gPiBBcmUg
dGhlcmUgYW55IGNvbXBsZXRpb24gb2JqZWN0cyBmb3Igd2hpY2ggdGhlIGNyb3NzLXJlbGVhc2Ug
Y2hlY2tpbmcgaXMNCj4gPiB1c2VmdWw/DQo+IA0KPiBBbGwgb2YgdGhlbSBieSBkZWZpbml0aW9u
Lg0KDQpTb3JyeSBidXQgSSdtIG5vdCBzdXJlIHRoYXQncyB0aGUgYmVzdCBwb3NzaWJsZSBhbnN3
ZXIuIEluIG15IG9waW5pb24NCmF2b2lkaW5nIHRoYXQgY29tcGxldGlvbiBvYmplY3RzIGhhdmUg
ZGVwZW5kZW5jaWVzIG9uIG90aGVyIGxvY2sgb2JqZWN0cywNCmUuZy4gYnkgYXZvaWRpbmcgdG8g
d2FpdCBvbiBhIGNvbXBsZXRpb24gb2JqZWN0IHdoaWxlIGhvbGRpbmcgYSBtdXRleCwgaXMgYQ0K
ZmFyIHN1cGVyaW9yIHN0cmF0ZWd5IG92ZXIgYWRkaW5nIGNyb3NzLXJlbGVhc2UgY2hlY2tpbmcg
dG8gY29tcGxldGlvbg0Kb2JqZWN0cy4gVGhlIGZvcm1lciBzdHJhdGVneSBuYW1lbHkgbWFrZXMg
aXQgdW5uZWNlc3NhcnkgdG8gYWRkDQpjcm9zcy1yZWxlYXNlIGNoZWNraW5nIHRvIGNvbXBsZXRp
b24gb2JqZWN0cyBiZWNhdXNlIHRoYXQgc3RyYXRlZ3kgZW5zdXJlcw0KdGhhdCB0aGVzZSBjb21w
bGV0aW9uIG9iamVjdHMgY2Fubm90IGdldCBpbnZvbHZlZCBpbiBhIGRlYWRsb2NrLiBUaGUgbGF0
dGVyDQpzdHJhdGVneSBjYW4gbGVhZCB0byBmYWxzZSBwb3NpdGl2ZSBkZWFkbG9jayByZXBvcnRz
IGJ5IHRoZSBsb2NrZGVwIGNvZGUsDQpzb21ldGhpbmcgbm9uZSBvZiB1cyB3YW50cy4NCg0KQSBw
b3NzaWJsZSBhbHRlcm5hdGl2ZSBzdHJhdGVneSBjb3VsZCBiZSB0byBlbmFibGUgY3Jvc3MtcmVs
ZWFzZSBjaGVja2luZw0Kb25seSBmb3IgdGhvc2UgY29tcGxldGlvbiBvYmplY3RzIGZvciB3aGlj
aCB3YWl0aW5nIG9jY3VycyBpbnNpZGUgYSBjcml0aWNhbA0Kc2VjdGlvbi4NCg0KPiA+IEFyZSB0
aGVyZSBhbnkgd2FpdF9mb3JfY29tcGxldGlvbigpIGNhbGxlcnMgdGhhdCBob2xkIGEgbXV0ZXgg
b3INCj4gPiBvdGhlciBsb2NraW5nIG9iamVjdD8NCj4gDQo+IFllcywgdGhlcmUgYXJlIGFsc28g
Y3Jvc3MgY29tcGxldGlvbiBkZXBlbmRlbmNpZXMuIFRoZXJlIGhhdmUgYmVlbiBzdWNoDQo+IGJ1
Z3MgYW5kIEkgZXhwZWN0IG1vcmUgdG8gYmUgdW5lYXJ0aGVkLg0KPiANCj4gSSByZWFsbHkgaGF2
ZSB0byBhc2sgd2hhdCB5b3VyIG1vdGl2aWF0aW9uIGlzIHRvIGZpZ2h0IHRoZSBsb2NrZGVwIGNv
dmVyYWdlDQo+IG9mIHN5bmNocm9uaXphdGlvbiBvYmplY3RzIHRvb3RoIGFuZCBuYWlsPw0KDQpB
cyBleHBsYWluZWQgaW4gYW5vdGhlciBlLW1haWwgdGhyZWFkLCB1bmxpa2UgdGhlIGxvY2sgaW52
ZXJzaW9uIGNoZWNraW5nDQpwZXJmb3JtZWQgYnkgdGhlIDw9IHY0LjEzIGxvY2tkZXAgY29kZSwg
Y3Jvc3MtcmVsZWFzZSBjaGVja2luZyBpcyBhIGhldXJpc3RpYw0KdGhhdCBkb2VzIG5vdCBoYXZl
IGEgc291bmQgdGhlb3JldGljYWwgYmFzaXMuIFRoZSBsb2NrIHZhbGlkYXRvciBpcyBhbg0KaW1w
b3J0YW50IHRvb2wgZm9yIGtlcm5lbCBkZXZlbG9wZXJzLiBJdCBpcyBpbXBvcnRhbnQgdGhhdCBp
dCBwcm9kdWNlcyBhcyBmZXcNCmZhbHNlIHBvc2l0aXZlcyBhcyBwb3NzaWJsZS4gU2luY2UgdGhl
IGNyb3NzLXJlbGVhc2UgY2hlY2tzIGFyZSBlbmFibGVkDQphdXRvbWF0aWNhbGx5IHdoZW4gZW5h
YmxpbmcgbG9ja2RlcCwgSSB0aGluayBpdCBpcyBub3JtYWwgdGhhdCBJLCBhcyBhIGtlcm5lbA0K
ZGV2ZWxvcGVyLCBjYXJlIHRoYXQgdGhlIGNyb3NzLXJlbGVhc2UgY2hlY2tzIHByb2R1Y2UgYXMg
ZmV3IGZhbHNlIHBvc2l0aXZlcw0KYXMgcG9zc2libGUuDQoNCkJhcnQu

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tglx@linutronix.de" <tglx@linutronix.de>
Cc: "mingo@kernel.org" <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"hch@infradead.org" <hch@infradead.org>,
	"amir73il@gmail.com" <amir73il@gmail.com>,
	"linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"darrick.wong@oracle.com" <darrick.wong@oracle.com>,
	"johannes.berg@intel.com" <johannes.berg@intel.com>,
	"byungchul.park@lge.com" <byungchul.park@lge.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"idryomov@gmail.com" <idryomov@gmail.com>,
	"tj@kernel.org" <tj@kernel.org>,
	"kernel-team@lge.com" <kernel-team@lge.com>,
	"david@fromorbit.com" <david@fromorbit.com>
Subject: Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map
Date: Fri, 20 Oct 2017 19:58:54 +0000	[thread overview]
Message-ID: <1508529532.3029.15.camel@wdc.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1710200829340.3083@nanos>

On Fri, 2017-10-20 at 08:34 +0200, Thomas Gleixner wrote:
> On Thu, 19 Oct 2017, Bart Van Assche wrote:
> > Are there any completion objects for which the cross-release checking is
> > useful?
> 
> All of them by definition.

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 mutex, is a
far superior strategy over adding cross-release checking to completion
objects. The former strategy namely makes it unnecessary to add
cross-release checking to completion objects because that strategy ensures
that these completion objects cannot get involved in a deadlock. The latter
strategy can lead to false positive deadlock reports by the lockdep code,
something none of us wants.

A possible alternative strategy could be to enable cross-release checking
only for those completion objects for which waiting occurs inside a critical
section.

> > Are there any wait_for_completion() callers that hold a mutex or
> > other locking object?
> 
> Yes, there are also cross completion dependencies. There have been such
> bugs and I expect more to be unearthed.
> 
> I really have to ask what your motiviation is to fight the lockdep coverage
> of synchronization objects tooth and nail?

As explained in another e-mail thread, unlike the lock inversion checking
performed by the <= v4.13 lockdep code, cross-release checking is a heuristic
that does not have a sound theoretical basis. The lock validator is an
important tool for kernel developers. It is important that it produces as few
false positives as possible. Since the cross-release checks are enabled
automatically when enabling lockdep, I think it is normal that I, as a kernel
developer, care that the cross-release checks produce as few false positives
as possible.

Bart.

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tglx@linutronix.de" <tglx@linutronix.de>
Cc: "mingo@kernel.org" <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"hch@infradead.org" <hch@infradead.org>,
	"amir73il@gmail.com" <amir73il@gmail.com>,
	"linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"darrick.wong@oracle.com" <darrick.wong@oracle.com>,
	"johannes.berg@intel.com" <johannes.berg@intel.com>,
	"byungchul.park@lge.com" <byungchul.park@lge.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"idryomov@gmail.com" <idryomov@gmail.com>,
	"tj@kernel.org" <tj@kernel.org>,
	"kernel-team@lge.com" <kernel-team@lge.com>,
	"david@fromorbit.com" <david@fromorbit.com>
Subject: Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map
Date: Fri, 20 Oct 2017 19:58:54 +0000	[thread overview]
Message-ID: <1508529532.3029.15.camel@wdc.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1710200829340.3083@nanos>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2022 bytes --]

On Fri, 2017-10-20 at 08:34 +0200, Thomas Gleixner wrote:
> On Thu, 19 Oct 2017, Bart Van Assche wrote:
> > Are there any completion objects for which the cross-release checking is
> > useful?
> 
> All of them by definition.

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 mutex, is a
far superior strategy over adding cross-release checking to completion
objects. The former strategy namely makes it unnecessary to add
cross-release checking to completion objects because that strategy ensures
that these completion objects cannot get involved in a deadlock. The latter
strategy can lead to false positive deadlock reports by the lockdep code,
something none of us wants.

A possible alternative strategy could be to enable cross-release checking
only for those completion objects for which waiting occurs inside a critical
section.

> > Are there any wait_for_completion() callers that hold a mutex or
> > other locking object?
> 
> Yes, there are also cross completion dependencies. There have been such
> bugs and I expect more to be unearthed.
> 
> I really have to ask what your motiviation is to fight the lockdep coverage
> of synchronization objects tooth and nail?

As explained in another e-mail thread, unlike the lock inversion checking
performed by the <= v4.13 lockdep code, cross-release checking is a heuristic
that does not have a sound theoretical basis. The lock validator is an
important tool for kernel developers. It is important that it produces as few
false positives as possible. Since the cross-release checks are enabled
automatically when enabling lockdep, I think it is normal that I, as a kernel
developer, care that the cross-release checks produce as few false positives
as possible.

Bart.N‹§²æìr¸›zǧu©ž²Æ {\b­†éì¹»\x1c®&Þ–)îÆi¢žØ^n‡r¶‰šŽŠÝ¢j$½§$¢¸\x05¢¹¨­è§~Š'.)îÄÃ,yèm¶ŸÿÃ\f%Š{±šj+ƒðèž×¦j)Z†·Ÿ

  reply	other threads:[~2017-10-20 19:58 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-18  9:38 Fix false positive by LOCKDEP_CROSSRELEASE Byungchul Park
2017-10-18  9:38 ` Byungchul Park
2017-10-18  9:38 ` [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map Byungchul Park
2017-10-18  9:38   ` Byungchul Park
2017-10-19 23:24   ` Bart Van Assche
2017-10-19 23:24     ` Bart Van Assche
2017-10-20  6:14     ` Byungchul Park
2017-10-20  6:14       ` Byungchul Park
2017-10-20  6:34     ` Thomas Gleixner
2017-10-20  6:34       ` Thomas Gleixner
2017-10-20 19:58       ` Bart Van Assche [this message]
2017-10-20 19:58         ` Bart Van Assche
2017-10-20 19:58         ` Bart Van Assche
2017-10-21  2:23         ` Byungchul Park
2017-10-21  2:23           ` Byungchul Park
2017-10-22 14:34           ` Bart Van Assche
2017-10-22 14:34             ` Bart Van Assche
2017-10-23  2:08             ` Byungchul Park
2017-10-23  2:08               ` Byungchul Park
2017-10-25  7:07               ` Bart Van Assche
2017-10-25  7:07                 ` Bart Van Assche
2017-10-25 11:49                 ` Byungchul Park
2017-10-25 11:49                   ` Byungchul Park
2017-10-18  9:38 ` [RESEND PATCH 2/3] lockdep: Remove unnecessary acquisitions wrt workqueue flush Byungchul Park
2017-10-18  9:38   ` Byungchul Park
2017-10-18  9:38 ` [RESEND PATCH 3/3] lockdep: Assign a lock_class per gendisk used for wait_for_completion() Byungchul Park
2017-10-18  9:38   ` Byungchul Park
2017-10-18  9:59   ` Ingo Molnar
2017-10-18  9:59     ` Ingo Molnar
2017-10-19  1:57     ` Byungchul Park
2017-10-19  1:57       ` Byungchul Park
2017-10-18 14:29 ` Fix false positive by LOCKDEP_CROSSRELEASE Bart Van Assche
2017-10-18 14:29   ` Bart Van Assche
2017-10-19  1:57   ` Byungchul Park
2017-10-19  1:57     ` Byungchul Park
2017-10-19 14:52     ` Bart Van Assche
2017-10-19 14:52       ` Bart Van Assche
2017-10-19 14:52       ` Bart Van Assche

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=1508529532.3029.15.camel@wdc.com \
    --to=bart.vanassche@wdc.com \
    --cc=amir73il@gmail.com \
    --cc=byungchul.park@lge.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=idryomov@gmail.com \
    --cc=johannes.berg@intel.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=linux-xfs@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.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.