All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tj@kernel.org" <tj@kernel.org>,
	"00moses.alexander00@gmail.com" <00moses.alexander00@gmail.com>,
	"joseph.qi@linux.alibaba.com" <joseph.qi@linux.alibaba.com>
Cc: "nborisov@suse.com" <nborisov@suse.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>, "shli@fb.com" <shli@fb.com>
Subject: Re: [PATCH v2] blk-cgroup: remove entries in blkg_tree before queue release
Date: Wed, 11 Apr 2018 16:42:55 +0000	[thread overview]
Message-ID: <bd60302129cb89bdc3b4f402ce4e061f41851729.camel@wdc.com> (raw)
In-Reply-To: <20180411145632.GK793541@devbig577.frc2.facebook.com>

T24gV2VkLCAyMDE4LTA0LTExIGF0IDA3OjU2IC0wNzAwLCBUZWp1biBIZW8gd3JvdGU6DQo+IEFu
ZCBsb29raW5nIGF0IHRoZSBjaGFuZ2UsIGl0IGxvb2tzIGxpa2UgdGhlIHJpZ2h0IHRoaW5nIHdl
IHNob3VsZA0KPiBoYXZlIGRvbmUgaXMgY2FjaGluZyBAbG9jayBvbiB0aGUgcHJpbnRfYmxrZyBz
aWRlIGFuZCB3aGVuIHN3aXRjaGluZw0KPiBsb2NrcyBtYWtlIHN1cmUgYm90aCBsb2NrcyBhcmUg
aGVsZC4gIElPVywgZG8gdGhlIGZvbGxvd2luZyBpbg0KPiBibGtfY2xlYW51cF9xdWV1ZSgpDQo+
IA0KPiAJc3Bpbl9sb2NrX2lycShsb2NrKTsNCj4gCWlmIChxLT5xdWV1ZV9sb2NrICE9ICZxLT5f
X3F1ZXVlX2xvY2spIHsNCj4gCQlzcGluX2xvY2soJnEtPl9fcXVldWVfbG9jayk7DQo+IAkJcS0+
cXVldWVfbG9jayA9ICZxLT5fX3F1ZXVlX2xvY2s7DQo+IAkJc3Bpbl91bmxvY2soJnEtPl9fcXVl
dWVfbG9jayk7DQo+IAl9DQo+IAlzcGluX3VubG9ja19pcnEobG9jayk7DQo+IA0KPiBPdGhlcndp
c2UsIHRoZXJlIGNhbiBiZSB0d28gbG9jayBob2xkZXJzIHRoaW5raW5nIHRoZXkgaGF2ZSBleGNs
dXNpdmUNCj4gYWNjZXNzIHRvIHRoZSByZXF1ZXN0X3F1ZXVlLg0KDQpJIHRoaW5rIHRoYXQncyBh
IGJhZCBpZGVhLiBBIGJsb2NrIGRyaXZlciBpcyBhbGxvd2VkIHRvIGRlc3Ryb3kgdGhlDQpzcGlu
bG9jayBpdCBhc3NvY2lhdGVkIHdpdGggdGhlIHJlcXVlc3QgcXVldWUgYXMgc29vbiBhcyBibGtf
Y2xlYW51cF9xdWV1ZSgpDQpoYXMgZmluaXNoZWQuIElmIHRoZSBibG9jayBjZ3JvdXAgY29udHJv
bGxlciB3b3VsZCBjYWNoZSBhIHBvaW50ZXIgdG8gdGhlDQpibG9jayBkcml2ZXIgc3BpbmxvY2sg
dGhlbiB0aGF0IGNvdWxkIGNhdXNlIHRoZSBjZ3JvdXAgY29kZSB0byBhdHRlbXB0IHRvDQpsb2Nr
IGEgc3BpbmxvY2sgYWZ0ZXIgaXQgaGFzIGJlZW4gZGVzdHJveWVkLiBJIGRvbid0IHRoaW5rIHdl
IG5lZWQgdGhhdCBraW5kDQpvZiByYWNlIGNvbmRpdGlvbnMuDQoNCkJhcnQuDQoNCg0KDQo=

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@wdc.com>
To: "tj@kernel.org" <tj@kernel.org>,
	"00moses.alexander00@gmail.com" <00moses.alexander00@gmail.com>,
	"joseph.qi@linux.alibaba.com" <joseph.qi@linux.alibaba.com>
Cc: "nborisov@suse.com" <nborisov@suse.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>, "shli@fb.com" <shli@fb.com>
Subject: Re: [PATCH v2] blk-cgroup: remove entries in blkg_tree before queue release
Date: Wed, 11 Apr 2018 16:42:55 +0000	[thread overview]
Message-ID: <bd60302129cb89bdc3b4f402ce4e061f41851729.camel@wdc.com> (raw)
In-Reply-To: <20180411145632.GK793541@devbig577.frc2.facebook.com>

On Wed, 2018-04-11 at 07:56 -0700, Tejun Heo wrote:
> And looking at the change, it looks like the right thing we should
> have done is caching @lock on the print_blkg side and when switching
> locks make sure both locks are held.  IOW, do the following in
> blk_cleanup_queue()
> 
> 	spin_lock_irq(lock);
> 	if (q->queue_lock != &q->__queue_lock) {
> 		spin_lock(&q->__queue_lock);
> 		q->queue_lock = &q->__queue_lock;
> 		spin_unlock(&q->__queue_lock);
> 	}
> 	spin_unlock_irq(lock);
> 
> Otherwise, there can be two lock holders thinking they have exclusive
> access to the request_queue.

I think that's a bad idea. A block driver is allowed to destroy the
spinlock it associated with the request queue as soon as blk_cleanup_queue()
has finished. If the block cgroup controller would cache a pointer to the
block driver spinlock then that could cause the cgroup code to attempt to
lock a spinlock after it has been destroyed. I don't think we need that kind
of race conditions.

Bart.




  reply	other threads:[~2018-04-11 16:42 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-07 10:21 [PATCH v2] blk-cgroup: remove entries in blkg_tree before queue release Alexandru Moise
2018-04-09 22:09 ` Tejun Heo
2018-04-11 10:12   ` Alexandru Moise
2018-04-11 14:20     ` Tejun Heo
2018-04-11 14:28       ` Alexandru Moise
2018-04-11 14:46         ` Tejun Heo
2018-04-11 14:51           ` Tejun Heo
2018-04-11 14:56             ` Tejun Heo
2018-04-11 16:42               ` Bart Van Assche [this message]
2018-04-11 16:42                 ` Bart Van Assche
2018-04-11 17:00                 ` tj
2018-04-11 17:06                   ` Bart Van Assche
2018-04-11 17:06                     ` Bart Van Assche
2018-04-11 17:15                     ` tj
2018-04-11 17:26                       ` Bart Van Assche
2018-04-11 17:26                         ` Bart Van Assche
2018-04-11 17:30                         ` tj
2018-04-11 15:54         ` Bart Van Assche
2018-04-11 15:54           ` Bart Van Assche
2018-04-11 19:00           ` Alexandru Moise
2018-04-11 19:55             ` Bart Van Assche
2018-04-11 19:57               ` tj
2018-04-11 20:00                 ` Bart Van Assche
2018-04-11 20:00                   ` Bart Van Assche
2018-04-11 20:02                   ` tj
2018-04-11 20:23                     ` Bart Van Assche
2018-04-11 20:23                       ` Bart Van Assche
2018-04-11 21:23               ` Alexandru Moise
2018-04-11 21:28                 ` Bart Van Assche
2018-04-11 21:28                   ` Bart Van Assche
2018-04-11 22:58                   ` Alexandru Moise

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=bd60302129cb89bdc3b4f402ce4e061f41851729.camel@wdc.com \
    --to=bart.vanassche@wdc.com \
    --cc=00moses.alexander00@gmail.com \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=gregkh@linuxfoundation.org \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=shli@fb.com \
    --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.