From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "longman@redhat.com" <longman@redhat.com>,
"rostedt@goodmis.org" <rostedt@goodmis.org>
Cc: "bfields@fieldses.org" <bfields@fieldses.org>,
"mingo@kernel.org" <mingo@kernel.org>,
"jlayton@poochiereds.net" <jlayton@poochiereds.net>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops
Date: Fri, 18 Aug 2017 16:21:46 +0000 [thread overview]
Message-ID: <1503073304.2622.5.camel@wdc.com> (raw)
In-Reply-To: <5a5d0743-d2db-89c8-59cc-542835baeccf@redhat.com>
T24gRnJpLCAyMDE3LTA4LTE4IGF0IDA5OjU1IC0wNDAwLCBXYWltYW4gTG9uZyB3cm90ZToNCj4g
T24gMDgvMTcvMjAxNyAwNTozMCBQTSwgU3RldmVuIFJvc3RlZHQgd3JvdGU6DQo+ID4gT24gVGh1
LCAxNyBBdWcgMjAxNyAxNzoxMDowNyAtMDQwMA0KPiA+IFN0ZXZlbiBSb3N0ZWR0IDxyb3N0ZWR0
QGdvb2RtaXMub3JnPiB3cm90ZToNCj4gPiA+IEluc3RlYWQgb2YgcGxheWluZyBnYW1lcyB3aXRo
IHRha2luZyB0aGUgbG9jaywgdGhlIG9ubHkgd2F5IHRoaXMgcmFjZQ0KPiA+ID4gaXMgaGl0LCBp
cyBpZiB0aGUgcGFydGl0aW9uIGlzIGJlaW5nIGRlbGV0ZWQgYW5kIHRoZSBzeXNmcyBhdHRyaWJ1
dGUgaXMNCj4gPiA+IGJlaW5nIHJlYWQgYXQgdGhlIHNhbWUgdGltZSwgY29ycmVjdD8gSW4gdGhh
dCBjYXNlLCBqdXN0IHJldHVybg0KPiA+ID4gLUVOT0RFViwgYW5kIGJlIGRvbmUgd2l0aCBpdC4N
Cj4gPiANCj4gPiBOZXZlcm1pbmQgdGhhdCB3b250IHdvcmsuIFRvbyBiYWQgdGhlcmUncyBub3Qg
YSBtdXRleF9sb2NrX3RpbWVvdXQoKQ0KPiA+IHRoYXQgd2UgY291bGQgdXNlIGluIGEgbG9vcC4g
SXQgd291bGQgc29sdmUgdGhlIGlzc3VlIG9mIGZvcndhcmQNCj4gPiBwcm9ncmVzcyB3aXRoIFJU
IHRhc2tzLCBhbmQgd2lsbCBicmVhayBhZnRlciBhIHRpbWVvdXQgaW4gY2FzZSBvZg0KPiA+IGRl
YWRsb2NrLg0KPiANCj4gSSB0aGluayBpdCB3aWxsIGJlIHVzZWZ1bCB0byBoYXZlIG11dGV4X3Rp
bWVkX2xvY2soKS4gUlQtbXV0ZXggZG9lcyBoYXZlDQo+IGEgdGltZWQgdmVyc2lvbiwgc28gSSBn
dWVzcyBpdCBzaG91bGRuJ3QgYmUgaGFyZCB0byBpbXBsZW1lbnQgb25lIGZvcg0KPiBtdXRleC4g
SSBjYW4gdGFrZSBhIHNob3QgYXQgdHJ5aW5nIHRvIGRvIHRoYXQuDQoNCihqdXN0IGNhdWdodCB1
cCB3aXRoIHRoZSBlbnRpcmUgZS1tYWlsIHRocmVhZCkNCg0KU29ycnkgV2FpbWFuIGJ1dCBwZXJz
b25hbGx5IEkgdGhvcm91Z2hseSBkZXRlc3QgbG9vcHMgYXJvdW5kIG11dGV4X3RyeWxvY2soKSBv
cg0KbXV0ZXhfdGltZWRfbG9jaygpIGJlY2F1c2Ugc3VjaCBsb29wcyBhcmUgdXN1YWxseSB1c2Vk
IHRvIHBhcGVyIG92ZXIgYSBwcm9ibGVtDQppbnN0ZWFkIG9mIGZpeGluZyB0aGUgcm9vdCBjYXVz
ZS4gV2hhdCBJIHVuZGVyc3Rvb2QgZnJvbSB0aGUgY29tbWVudCBpbiB2MSBvZiB5b3VyDQpwYXRj
aCBpcyB0aGF0IGJkX211dGV4IGlzIG5vdCBvbmx5IGhlbGQgZHVyaW5nIGJsb2NrIGRldmljZSBj
cmVhdGlvbiBhbmQgcmVtb3ZhbA0KYnV0IGFkZGl0aW9uYWxseSB0aGF0IGJkX211dGV4IGlzIG9i
dGFpbmVkIGluc2lkZSBzeXNmcyBhdHRyaWJ1dGUgY2FsbGJhY2sgbWV0aG9kcz8NClRoYXQgcGF0
dGVybiBpcyBndWFyYW50ZWVkIHRvIGxlYWQgdG8gZGVhZGxvY2tzLiBTaW5jZSB0aGUgYmxvY2sg
ZGV2aWNlIHJlbW92YWwNCmNvZGUgd2FpdHMgdW50aWwgYWxsIHN5c2ZzIGNhbGxiYWNrIG1ldGhv
ZHMgaGF2ZSBmaW5pc2hlZCB0aGVyZSBpcyBubyBuZWVkIHRvDQpwcm90ZWN0IGFnYWluc3QgYmxv
Y2sgZGV2aWNlIHJlbW92YWwgaW5zaWRlIHRoZSBzeXNmcyBjYWxsYmFjayBtZXRob2RzLiBNeSBw
cm9wb3NhbA0KaXMgdG8gc3BsaXQgYmRfbXV0ZXg6IG9uZSBnbG9iYWwgbXV0ZXggdGhhdCBzZXJp
YWxpemVzIGJsb2NrIGRldmljZSBjcmVhdGlvbiBhbmQNCnJlbW92YWwgYW5kIG9uZSBtdXRleCBw
ZXIgYmxvY2sgZGV2aWNlIHRoYXQgc2VyaWFsaXplcyBjaGFuZ2VzIHRvIGEgc2luZ2xlIGJsb2Nr
DQpkZXZpY2UuIE9idGFpbmluZyB0aGUgZ2xvYmFsIG11dGV4IGZyb20gaW5zaWRlIGEgYmxvY2sg
ZGV2aWNlIHN5c2ZzIGNhbGxiYWNrDQpmdW5jdGlvbiBpcyBub3Qgc2FmZSBidXQgb2J0YWluaW5n
IHRoZSBwZXItYmxvY2stZGV2aWNlIG11dGV4IGZyb20gaW5zaWRlIGEgc3lzZnMNCmNhbGxiYWNr
IGZ1bmN0aW9uIGlzIHNhZmUuDQoNCkJhcnQu
WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "longman@redhat.com" <longman@redhat.com>,
"rostedt@goodmis.org" <rostedt@goodmis.org>
Cc: "bfields@fieldses.org" <bfields@fieldses.org>,
"mingo@kernel.org" <mingo@kernel.org>,
"jlayton@poochiereds.net" <jlayton@poochiereds.net>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops
Date: Fri, 18 Aug 2017 16:21:46 +0000 [thread overview]
Message-ID: <1503073304.2622.5.camel@wdc.com> (raw)
In-Reply-To: <5a5d0743-d2db-89c8-59cc-542835baeccf@redhat.com>
On Fri, 2017-08-18 at 09:55 -0400, Waiman Long wrote:
> On 08/17/2017 05:30 PM, Steven Rostedt wrote:
> > On Thu, 17 Aug 2017 17:10:07 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > > Instead of playing games with taking the lock, the only way this race
> > > is hit, is if the partition is being deleted and the sysfs attribute is
> > > being read at the same time, correct? In that case, just return
> > > -ENODEV, and be done with it.
> >
> > Nevermind that wont work. Too bad there's not a mutex_lock_timeout()
> > that we could use in a loop. It would solve the issue of forward
> > progress with RT tasks, and will break after a timeout in case of
> > deadlock.
>
> I think it will be useful to have mutex_timed_lock(). RT-mutex does have
> a timed version, so I guess it shouldn't be hard to implement one for
> mutex. I can take a shot at trying to do that.
(just caught up with the entire e-mail thread)
Sorry Waiman but personally I thoroughly detest loops around mutex_trylock() or
mutex_timed_lock() because such loops are usually used to paper over a problem
instead of fixing the root cause. What I understood from the comment in v1 of your
patch is that bd_mutex is not only held during block device creation and removal
but additionally that bd_mutex is obtained inside sysfs attribute callback methods?
That pattern is guaranteed to lead to deadlocks. Since the block device removal
code waits until all sysfs callback methods have finished there is no need to
protect against block device removal inside the sysfs callback methods. My proposal
is to split bd_mutex: one global mutex that serializes block device creation and
removal and one mutex per block device that serializes changes to a single block
device. Obtaining the global mutex from inside a block device sysfs callback
function is not safe but obtaining the per-block-device mutex from inside a sysfs
callback function is safe.
Bart.
next prev parent reply other threads:[~2017-08-18 16:21 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-16 20:40 [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops Waiman Long
2017-08-17 13:34 ` Steven Rostedt
2017-08-17 16:24 ` Waiman Long
2017-08-17 16:24 ` Waiman Long
2017-08-17 20:30 ` Steven Rostedt
2017-08-17 20:41 ` Bart Van Assche
2017-08-17 20:41 ` Bart Van Assche
2017-08-17 20:56 ` Steven Rostedt
2017-08-17 21:10 ` Steven Rostedt
2017-08-17 21:27 ` Waiman Long
2017-08-17 21:27 ` Waiman Long
2017-08-17 22:13 ` Steven Rostedt
2017-08-17 22:18 ` Steven Rostedt
2017-08-17 23:23 ` Steven Rostedt
2017-08-18 13:42 ` Waiman Long
2017-08-17 21:30 ` Steven Rostedt
2017-08-18 13:55 ` Waiman Long
2017-08-18 13:55 ` Waiman Long
2017-08-18 16:21 ` Bart Van Assche [this message]
2017-08-18 16:21 ` Bart Van Assche
2017-08-18 17:22 ` Waiman Long
2017-08-18 17:22 ` Waiman Long
2017-08-18 17:26 ` Steven Rostedt
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=1503073304.2622.5.camel@wdc.com \
--to=bart.vanassche@wdc.com \
--cc=axboe@kernel.dk \
--cc=bfields@fieldses.org \
--cc=jlayton@poochiereds.net \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mingo@kernel.org \
--cc=rostedt@goodmis.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.