[PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

Several block drivers call alloc_disk() followed by put_disk() if
something fails before device_add_disk() is called without calling
blk_cleanup_queue(). Make sure that also for this scenario a request
queue is dissociated from the cgroup controller. This patch avoids
that loading the parport_pc, paride and pf drivers triggers the
following kernel crash:

BUG: KASAN: null-ptr-deref in pi_init+0x42e/0x580 [paride]
Read of size 4 at addr 0000000000000008 by task modprobe/744
Call Trace:
dump_stack+0x9a/0xeb
kasan_report+0x139/0x350
pi_init+0x42e/0x580 [paride]
pf_init+0x2bb/0x1000 [pf]
do_one_initcall+0x8e/0x405
do_init_module+0xd9/0x2f2
load_module+0x3ab4/0x4700
SYSC_finit_module+0x176/0x1a0
do_syscall_64+0xee/0x2b0
entry_SYSCALL_64_after_hwframe+0x42/0xb7

Reported-by: Alexandru Moise <00moses.alexander00@gmail.com>
Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Alexandru Moise <00moses.alexander00@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
---
 block/blk-sysfs.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f8457d6f0190..2e134da78f82 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -788,11 +788,37 @@ static void blk_free_queue_rcu(struct rcu_head *rcu_head)
 static void __blk_release_queue(struct work_struct *work)
 {
 	struct request_queue *q = container_of(work, typeof(*q), release_work);
+	struct blkcg_gq *gq;
 
 	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
 		blk_stat_remove_callback(q, q->poll_cb);
 	blk_stat_free_callback(q->poll_cb);
 
+	if (!blk_queue_dead(q)) {
+		/*
+		 * Last reference was dropped without having called
+		 * blk_cleanup_queue().
+		 */
+		WARN_ONCE(blk_queue_init_done(q),
+			  "request queue %p has been registered but blk_cleanup_queue() has not been called for that queue\n",
+			  q);
+		bdi_put(q->backing_dev_info);
+		blkcg_exit_queue(q);
+
+		if (q->elevator) {
+			ioc_clear_queue(q);
+			elevator_exit(q, q->elevator);
+		}
+	}
+
+	rcu_read_lock();
+	gq = blkg_lookup(&blkcg_root, q);
+	rcu_read_unlock();
+
+	WARN(gq,
+	     "request queue %p is being released but it has not yet been removed from the blkcg controller\n",
+	     q);
+
 	blk_free_queue_stats(q->stats);
 
 	blk_exit_rl(q, &q->root_rl);
-- 
2.16.2

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Alexandru Moise]

On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> Several block drivers call alloc_disk() followed by put_disk() if
> something fails before device_add_disk() is called without calling
> blk_cleanup_queue(). Make sure that also for this scenario a request
> queue is dissociated from the cgroup controller. This patch avoids
> that loading the parport_pc, paride and pf drivers triggers the
> following kernel crash:
> 
> BUG: KASAN: null-ptr-deref in pi_init+0x42e/0x580 [paride]
> Read of size 4 at addr 0000000000000008 by task modprobe/744
> Call Trace:
> dump_stack+0x9a/0xeb
> kasan_report+0x139/0x350
> pi_init+0x42e/0x580 [paride]
> pf_init+0x2bb/0x1000 [pf]
> do_one_initcall+0x8e/0x405
> do_init_module+0xd9/0x2f2
> load_module+0x3ab4/0x4700
> SYSC_finit_module+0x176/0x1a0
> do_syscall_64+0xee/0x2b0
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> 
> Reported-by: Alexandru Moise <00moses.alexander00@gmail.com>
> Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Alexandru Moise <00moses.alexander00@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
>  block/blk-sysfs.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index f8457d6f0190..2e134da78f82 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -788,11 +788,37 @@ static void blk_free_queue_rcu(struct rcu_head *rcu_head)
>  static void __blk_release_queue(struct work_struct *work)
>  {
>  	struct request_queue *q = container_of(work, typeof(*q), release_work);
> +	struct blkcg_gq *gq;
>  
>  	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
>  		blk_stat_remove_callback(q, q->poll_cb);
>  	blk_stat_free_callback(q->poll_cb);
>  
> +	if (!blk_queue_dead(q)) {
> +		/*
> +		 * Last reference was dropped without having called
> +		 * blk_cleanup_queue().
> +		 */
> +		WARN_ONCE(blk_queue_init_done(q),
> +			  "request queue %p has been registered but blk_cleanup_queue() has not been called for that queue\n",
> +			  q);
> +		bdi_put(q->backing_dev_info);
> +		blkcg_exit_queue(q);
> +
> +		if (q->elevator) {
> +			ioc_clear_queue(q);
> +			elevator_exit(q, q->elevator);
> +		}
> +	}
> +
> +	rcu_read_lock();
> +	gq = blkg_lookup(&blkcg_root, q);
> +	rcu_read_unlock();
> +
> +	WARN(gq,
> +	     "request queue %p is being released but it has not yet been removed from the blkcg controller\n",
> +	     q);
> +
>  	blk_free_queue_stats(q->stats);

This solution is good. Thanks for fixing this!

Tested-by: Alexandru Moise <00moses.alexander00@gmail.com>

../Alex

>  
>  	blk_exit_rl(q, &q->root_rl);
> -- 
> 2.16.2
> 

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Alexandru Moise]

On Thu, Apr 12, 2018 at 06:20:34AM +0200, Alexandru Moise wrote:
> On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> > Several block drivers call alloc_disk() followed by put_disk() if
> > something fails before device_add_disk() is called without calling
> > blk_cleanup_queue(). Make sure that also for this scenario a request
> > queue is dissociated from the cgroup controller. This patch avoids
> > that loading the parport_pc, paride and pf drivers triggers the
> > following kernel crash:
> > 
> > BUG: KASAN: null-ptr-deref in pi_init+0x42e/0x580 [paride]
> > Read of size 4 at addr 0000000000000008 by task modprobe/744
> > Call Trace:
> > dump_stack+0x9a/0xeb
> > kasan_report+0x139/0x350
> > pi_init+0x42e/0x580 [paride]
> > pf_init+0x2bb/0x1000 [pf]
> > do_one_initcall+0x8e/0x405
> > do_init_module+0xd9/0x2f2
> > load_module+0x3ab4/0x4700
> > SYSC_finit_module+0x176/0x1a0
> > do_syscall_64+0xee/0x2b0
> > entry_SYSCALL_64_after_hwframe+0x42/0xb7
> > 
> > Reported-by: Alexandru Moise <00moses.alexander00@gmail.com>
> > Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
> > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> > Cc: Alexandru Moise <00moses.alexander00@gmail.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > ---
> >  block/blk-sysfs.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> > index f8457d6f0190..2e134da78f82 100644
> > --- a/block/blk-sysfs.c
> > +++ b/block/blk-sysfs.c
> > @@ -788,11 +788,37 @@ static void blk_free_queue_rcu(struct rcu_head *rcu_head)
> >  static void __blk_release_queue(struct work_struct *work)
> >  {
> >  	struct request_queue *q = container_of(work, typeof(*q), release_work);
> > +	struct blkcg_gq *gq;
> >  
> >  	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
> >  		blk_stat_remove_callback(q, q->poll_cb);
> >  	blk_stat_free_callback(q->poll_cb);
> >  
> > +	if (!blk_queue_dead(q)) {
> > +		/*
> > +		 * Last reference was dropped without having called
> > +		 * blk_cleanup_queue().
> > +		 */
> > +		WARN_ONCE(blk_queue_init_done(q),
> > +			  "request queue %p has been registered but blk_cleanup_queue() has not been called for that queue\n",
> > +			  q);
> > +		bdi_put(q->backing_dev_info);
> > +		blkcg_exit_queue(q);
> > +
> > +		if (q->elevator) {
> > +			ioc_clear_queue(q);
> > +			elevator_exit(q, q->elevator);
> > +		}
> > +	}
> > +
> > +	rcu_read_lock();
> > +	gq = blkg_lookup(&blkcg_root, q);
> > +	rcu_read_unlock();
> > +
> > +	WARN(gq,
> > +	     "request queue %p is being released but it has not yet been removed from the blkcg controller\n",
> > +	     q);
> > +
> >  	blk_free_queue_stats(q->stats);
> 
> This solution is good. Thanks for fixing this!
> 
> Tested-by: Alexandru Moise <00moses.alexander00@gmail.com>
> 
> ../Alex

Oh, you also need the no-ops for !CONFIG_BLK_CGROUP, also there is no
blkcg_root then as well.

Thanks,
../Alex
> 
> >  
> >  	blk_exit_rl(q, &q->root_rl);
> > -- 
> > 2.16.2
> > 

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Alexandru Moise]

On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> Several block drivers call alloc_disk() followed by put_disk() if
> something fails before device_add_disk() is called without calling
> blk_cleanup_queue(). Make sure that also for this scenario a request
> queue is dissociated from the cgroup controller. This patch avoids
> that loading the parport_pc, paride and pf drivers triggers the
> following kernel crash:
> 
> BUG: KASAN: null-ptr-deref in pi_init+0x42e/0x580 [paride]
> Read of size 4 at addr 0000000000000008 by task modprobe/744
> Call Trace:
> dump_stack+0x9a/0xeb
> kasan_report+0x139/0x350
> pi_init+0x42e/0x580 [paride]
> pf_init+0x2bb/0x1000 [pf]
> do_one_initcall+0x8e/0x405
> do_init_module+0xd9/0x2f2
> load_module+0x3ab4/0x4700
> SYSC_finit_module+0x176/0x1a0
> do_syscall_64+0xee/0x2b0
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> 
> Reported-by: Alexandru Moise <00moses.alexander00@gmail.com>
> Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Alexandru Moise <00moses.alexander00@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
>  block/blk-sysfs.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index f8457d6f0190..2e134da78f82 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -788,11 +788,37 @@ static void blk_free_queue_rcu(struct rcu_head *rcu_head)
>  static void __blk_release_queue(struct work_struct *work)
>  {
>  	struct request_queue *q = container_of(work, typeof(*q), release_work);
> +	struct blkcg_gq *gq;
>  
>  	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
>  		blk_stat_remove_callback(q, q->poll_cb);
>  	blk_stat_free_callback(q->poll_cb);
>  
> +	if (!blk_queue_dead(q)) {
> +		/*
> +		 * Last reference was dropped without having called
> +		 * blk_cleanup_queue().
> +		 */
> +		WARN_ONCE(blk_queue_init_done(q),
> +			  "request queue %p has been registered but blk_cleanup_queue() has not been called for that queue\n",
> +			  q);
> +		bdi_put(q->backing_dev_info);
> +		blkcg_exit_queue(q);
> +
> +		if (q->elevator) {
> +			ioc_clear_queue(q);
> +			elevator_exit(q, q->elevator);
> +		}
> +	}
> +
> +	rcu_read_lock();
> +	gq = blkg_lookup(&blkcg_root, q);
> +	rcu_read_unlock();
> +
> +	WARN(gq,
> +	     "request queue %p is being released but it has not yet been removed from the blkcg controller\n",
> +	     q);
> +
>  	blk_free_queue_stats(q->stats);

This solution is good. Thanks for fixing this!

Tested-by: Alexandru Moise <00moses.alexander00@gmail.com>

../Alex

>  
>  	blk_exit_rl(q, &q->root_rl);
> -- 
> 2.16.2
> 

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Christoph Hellwig]

On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> Several block drivers call alloc_disk() followed by put_disk() if
> something fails before device_add_disk() is called without calling
> blk_cleanup_queue(). Make sure that also for this scenario a request
> queue is dissociated from the cgroup controller. This patch avoids
> that loading the parport_pc, paride and pf drivers triggers the
> following kernel crash:

Can we move the cleanup to put_disk in general and not just for
this case?  Having alloc/free routines pair up generally avoids
a lot of confusion.

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

T24gVGh1LCAyMDE4LTA0LTEyIGF0IDA3OjM0ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gT24gV2VkLCBBcHIgMTEsIDIwMTggYXQgMDc6NTg6NTJQTSAtMDYwMCwgQmFydCBWYW4g
QXNzY2hlIHdyb3RlOg0KPiA+IFNldmVyYWwgYmxvY2sgZHJpdmVycyBjYWxsIGFsbG9jX2Rpc2so
KSBmb2xsb3dlZCBieSBwdXRfZGlzaygpIGlmDQo+ID4gc29tZXRoaW5nIGZhaWxzIGJlZm9yZSBk
ZXZpY2VfYWRkX2Rpc2soKSBpcyBjYWxsZWQgd2l0aG91dCBjYWxsaW5nDQo+ID4gYmxrX2NsZWFu
dXBfcXVldWUoKS4gTWFrZSBzdXJlIHRoYXQgYWxzbyBmb3IgdGhpcyBzY2VuYXJpbyBhIHJlcXVl
c3QNCj4gPiBxdWV1ZSBpcyBkaXNzb2NpYXRlZCBmcm9tIHRoZSBjZ3JvdXAgY29udHJvbGxlci4g
VGhpcyBwYXRjaCBhdm9pZHMNCj4gPiB0aGF0IGxvYWRpbmcgdGhlIHBhcnBvcnRfcGMsIHBhcmlk
ZSBhbmQgcGYgZHJpdmVycyB0cmlnZ2VycyB0aGUNCj4gPiBmb2xsb3dpbmcga2VybmVsIGNyYXNo
Og0KPiANCj4gQ2FuIHdlIG1vdmUgdGhlIGNsZWFudXAgdG8gcHV0X2Rpc2sgaW4gZ2VuZXJhbCBh
bmQgbm90IGp1c3QgZm9yDQo+IHRoaXMgY2FzZT8gIEhhdmluZyBhbGxvYy9mcmVlIHJvdXRpbmVz
IHBhaXIgdXAgZ2VuZXJhbGx5IGF2b2lkcw0KPiBhIGxvdCBvZiBjb25mdXNpb24uDQoNCkhlbGxv
IENocmlzdG9waCwNCg0KQXQgbGVhc3QgdGhlIFNDU0kgVUxQIGRyaXZlcnMgZHJvcCB0aGUgbGFz
dCByZWZlcmVuY2UgdG8gYSBkaXNrIGFmdGVyDQp0aGUgYmxrX2NsZWFudXBfcXVldWUoKSBjYWxs
LiBBcyBleHBsYWluZWQgaW4gdGhlIGRlc2NyaXB0aW9uIG9mIGNvbW1pdA0KYTA2MzA1N2Q3Yzcz
LCByZW1vdmluZyBhIHJlcXVlc3QgcXVldWUgZnJvbSBibGtjZyBtdXN0IGhhcHBlbiBiZWZvcmUN
CmJsa19jbGVhbnVwX3F1ZXVlKCkgZmluaXNoZXMgYmVjYXVzZSBhIGJsb2NrIGRyaXZlciBtYXkg
ZnJlZSB0aGUNCnJlcXVlc3QgcXVldWUgc3BpbmxvY2sgaW1tZWRpYXRlbHkgYWZ0ZXIgYmxrX2Ns
ZWFudXBfcXVldWUoKSByZXR1cm5zLg0KU28gSSBkb24ndCB0aGluayB0aGF0IHdlIGNhbiBtb3Zl
IHRoZSBjb2RlIHRoYXQgcmVtb3ZlcyBhIHJlcXVlc3QNCnF1ZXVlIGZyb20gYmxrY2cgaW50byBw
dXRfZGlzaygpLiBBbm90aGVyIGNoYWxsZW5nZSBpcyB0aGF0IHNvbWUgYmxvY2sNCmRyaXZlcnMg
KGUuZy4gc2tkKSBjbGVhciB0aGUgZGlzay0+cXVldWUgcG9pbnRlciBpZiBkZXZpY2VfYWRkX2Rp
c2soKQ0KaGFzIG5vdCBiZWVuIGNhbGxlZCB0byBhdm9pZCB0aGF0IHB1dF9kaXNrKCkgY2F1c2Vz
IGEgcmVxdWVzdCBxdWV1ZQ0KcmVmZXJlbmNlIGNvdW50IGltYmFsYW5jZS4NCg0KQmFydC4NCg0K
DQoNCg==

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[hch]

On Thu, Apr 12, 2018 at 11:52:11AM +0000, Bart Van Assche wrote:
> On Thu, 2018-04-12 at 07:34 +0200, Christoph Hellwig wrote:
> > On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> > > Several block drivers call alloc_disk() followed by put_disk() if
> > > something fails before device_add_disk() is called without calling
> > > blk_cleanup_queue(). Make sure that also for this scenario a request
> > > queue is dissociated from the cgroup controller. This patch avoids
> > > that loading the parport_pc, paride and pf drivers triggers the
> > > following kernel crash:
> > 
> > Can we move the cleanup to put_disk in general and not just for
> > this case?  Having alloc/free routines pair up generally avoids
> > a lot of confusion.
> 
> Hello Christoph,
> 
> At least the SCSI ULP drivers drop the last reference to a disk after
> the blk_cleanup_queue() call. As explained in the description of commit
> a063057d7c73, removing a request queue from blkcg must happen before
> blk_cleanup_queue() finishes because a block driver may free the
> request queue spinlock immediately after blk_cleanup_queue() returns.
> So I don't think that we can move the code that removes a request
> queue from blkcg into put_disk(). Another challenge is that some block
> drivers (e.g. skd) clear the disk->queue pointer if device_add_disk()
> has not been called to avoid that put_disk() causes a request queue
> reference count imbalance.

Which sounds like a very good reason not to use a driver controller
lock for internals like blkcq.

In fact splitting the lock used for synchronizing access to queue
fields from the driver controller lock used to synchronize I/O
in the legacy path in long overdue.

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

Hello,

On Thu, Apr 12, 2018 at 03:14:40PM +0200, hch@lst.de wrote:
> > At least the SCSI ULP drivers drop the last reference to a disk after
> > the blk_cleanup_queue() call. As explained in the description of commit
> > a063057d7c73, removing a request queue from blkcg must happen before
> > blk_cleanup_queue() finishes because a block driver may free the
> > request queue spinlock immediately after blk_cleanup_queue() returns.
> > So I don't think that we can move the code that removes a request
> > queue from blkcg into put_disk(). Another challenge is that some block
> > drivers (e.g. skd) clear the disk->queue pointer if device_add_disk()
> > has not been called to avoid that put_disk() causes a request queue
> > reference count imbalance.
> 
> Which sounds like a very good reason not to use a driver controller
> lock for internals like blkcq.
> 
> In fact splitting the lock used for synchronizing access to queue
> fields from the driver controller lock used to synchronize I/O
> in the legacy path in long overdue.

It'd be probably a lot easier to make sure the shared lock doesn't go
away till all the request_queues using it go away.  The choice is
between refcnting something which carries the lock and double locking
in hot paths.  Can't think of many downsides of the former approach.

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[hch]

On Thu, Apr 12, 2018 at 06:48:12AM -0700, tj@kernel.org wrote:
> > Which sounds like a very good reason not to use a driver controller
> > lock for internals like blkcq.
> > 
> > In fact splitting the lock used for synchronizing access to queue
> > fields from the driver controller lock used to synchronize I/O
> > in the legacy path in long overdue.
> 
> It'd be probably a lot easier to make sure the shared lock doesn't go
> away till all the request_queues using it go away.  The choice is
> between refcnting something which carries the lock and double locking
> in hot paths.  Can't think of many downsides of the former approach.

We've stopped sharing request_queues between different devices a while
ago.  The problem is just that for legacy devices the driver still controls
the lock life time, and it might be shorter than the queue lifetime.
Note that for blk-mq we basically don't use the queue_lock at all,
and certainly not in the hot path.

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

Hello,

On Thu, Apr 12, 2018 at 03:56:51PM +0200, hch@lst.de wrote:
> On Thu, Apr 12, 2018 at 06:48:12AM -0700, tj@kernel.org wrote:
> > > Which sounds like a very good reason not to use a driver controller
> > > lock for internals like blkcq.
> > > 
> > > In fact splitting the lock used for synchronizing access to queue
> > > fields from the driver controller lock used to synchronize I/O
> > > in the legacy path in long overdue.
> > 
> > It'd be probably a lot easier to make sure the shared lock doesn't go
> > away till all the request_queues using it go away.  The choice is
> > between refcnting something which carries the lock and double locking
> > in hot paths.  Can't think of many downsides of the former approach.
> 
> We've stopped sharing request_queues between different devices a while
> ago.  The problem is just that for legacy devices the driver still controls
> the lock life time, and it might be shorter than the queue lifetime.
> Note that for blk-mq we basically don't use the queue_lock at all,
> and certainly not in the hot path.

Cool, can we just factor out the queue lock from those drivers?  It's
just really messy to be switching locks like we do in the cleanup
path.

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

On Thu, Apr 12, 2018 at 06:58:21AM -0700, tj@kernel.org wrote:
> Cool, can we just factor out the queue lock from those drivers?  It's
> just really messy to be switching locks like we do in the cleanup
> path.

So, looking at a couple drivers, it looks like all we'd need is a
struct which contains a spinlock and a refcnt.  Switching the drivers
to use that is a bit tedious but straight-forward and the block layer
can simply put that struct on queue release.

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Tejun Heo]

On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> Several block drivers call alloc_disk() followed by put_disk() if
> something fails before device_add_disk() is called without calling
> blk_cleanup_queue(). Make sure that also for this scenario a request
> queue is dissociated from the cgroup controller. This patch avoids
> that loading the parport_pc, paride and pf drivers triggers the
> following kernel crash:
> 
> BUG: KASAN: null-ptr-deref in pi_init+0x42e/0x580 [paride]
> Read of size 4 at addr 0000000000000008 by task modprobe/744
> Call Trace:
> dump_stack+0x9a/0xeb
> kasan_report+0x139/0x350
> pi_init+0x42e/0x580 [paride]
> pf_init+0x2bb/0x1000 [pf]
> do_one_initcall+0x8e/0x405
> do_init_module+0xd9/0x2f2
> load_module+0x3ab4/0x4700
> SYSC_finit_module+0x176/0x1a0
> do_syscall_64+0xee/0x2b0
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> 
> Reported-by: Alexandru Moise <00moses.alexander00@gmail.com>
> Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Alexandru Moise <00moses.alexander00@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>

So, this might be correct for this reported case but I don't think
it's correct in general.  There's no synchronization between blkcg
q->lock usages and blk_cleanup_queue().  e.g. hotunplugging and
shutting down a device while file operations are still in progress can
easily blow this up.

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

On 04/12/18 07:51, Tejun Heo wrote:
> On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
>> Several block drivers call alloc_disk() followed by put_disk() if
>> something fails before device_add_disk() is called without calling
>> blk_cleanup_queue(). Make sure that also for this scenario a request
>> queue is dissociated from the cgroup controller. This patch avoids
>> that loading the parport_pc, paride and pf drivers triggers the
>> following kernel crash:
>>
>> BUG: KASAN: null-ptr-deref in pi_init+0x42e/0x580 [paride]
>> Read of size 4 at addr 0000000000000008 by task modprobe/744
>> Call Trace:
>> dump_stack+0x9a/0xeb
>> kasan_report+0x139/0x350
>> pi_init+0x42e/0x580 [paride]
>> pf_init+0x2bb/0x1000 [pf]
>> do_one_initcall+0x8e/0x405
>> do_init_module+0xd9/0x2f2
>> load_module+0x3ab4/0x4700
>> SYSC_finit_module+0x176/0x1a0
>> do_syscall_64+0xee/0x2b0
>> entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>
>> Reported-by: Alexandru Moise <00moses.alexander00@gmail.com>
>> Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
>> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
>> Cc: Alexandru Moise <00moses.alexander00@gmail.com>
>> Cc: Tejun Heo <tj@kernel.org>
> 
> So, this might be correct for this reported case but I don't think
> it's correct in general.  There's no synchronization between blkcg
> q->lock usages and blk_cleanup_queue().  e.g. hotunplugging and
> shutting down a device while file operations are still in progress can
> easily blow this up.

Hello Tejun,

I have retested hotunplugging by rerunning the srp-test software. It 
seems like you overlooked that this patch does not remove the 
blkcg_exit_queue() call from blk_cleanup_queue()? If a device is 
hotunplugged it is up to the block driver to call blk_cleanup_queue(). 
And blk_cleanup_queue() will call blkcg_exit_queue().

Bart.

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Tejun Heo]

Hello, Bart.

On Thu, Apr 12, 2018 at 08:09:17AM -0600, Bart Van Assche wrote:
> I have retested hotunplugging by rerunning the srp-test software. It
> seems like you overlooked that this patch does not remove the
> blkcg_exit_queue() call from blk_cleanup_queue()? If a device is
> hotunplugged it is up to the block driver to call
> blk_cleanup_queue(). And blk_cleanup_queue() will call
> blkcg_exit_queue().

Hmm... what'd prevent blg_lookup_and_create() racing against that?

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

T24gVGh1LCAyMDE4LTA0LTEyIGF0IDA4OjM3IC0wNzAwLCBUZWp1biBIZW8gd3JvdGU6DQo+IE9u
IFRodSwgQXByIDEyLCAyMDE4IGF0IDA4OjA5OjE3QU0gLTA2MDAsIEJhcnQgVmFuIEFzc2NoZSB3
cm90ZToNCj4gPiBJIGhhdmUgcmV0ZXN0ZWQgaG90dW5wbHVnZ2luZyBieSByZXJ1bm5pbmcgdGhl
IHNycC10ZXN0IHNvZnR3YXJlLiBJdA0KPiA+IHNlZW1zIGxpa2UgeW91IG92ZXJsb29rZWQgdGhh
dCB0aGlzIHBhdGNoIGRvZXMgbm90IHJlbW92ZSB0aGUNCj4gPiBibGtjZ19leGl0X3F1ZXVlKCkg
Y2FsbCBmcm9tIGJsa19jbGVhbnVwX3F1ZXVlKCk/IElmIGEgZGV2aWNlIGlzDQo+ID4gaG90dW5w
bHVnZ2VkIGl0IGlzIHVwIHRvIHRoZSBibG9jayBkcml2ZXIgdG8gY2FsbA0KPiA+IGJsa19jbGVh
bnVwX3F1ZXVlKCkuIEFuZCBibGtfY2xlYW51cF9xdWV1ZSgpIHdpbGwgY2FsbA0KPiA+IGJsa2Nn
X2V4aXRfcXVldWUoKS4NCj4gDQo+IEhtbS4uLiB3aGF0J2QgcHJldmVudCBibGdfbG9va3VwX2Fu
ZF9jcmVhdGUoKSByYWNpbmcgYWdhaW5zdCB0aGF0Pw0KDQpIZWxsbyBUZWp1biwNCg0KRGlkIHlv
dSBwZXJoYXBzIG1lYW4gYmxrZ19sb29rdXBfY3JlYXRlKCk/IFRoYXQgZnVuY3Rpb24gaGFzIG9u
ZSBjYWxsZXIsDQpuYW1lbHkgYmxrY2dfYmlvX2lzc3VlX2NoZWNrKCkuIFRoZSBvbmx5IGNhbGxl
ciBvZiB0aGF0IGZ1bmN0aW9uIGlzDQpnZW5lcmljX21ha2VfcmVxdWVzdF9jaGVja3MoKS4gQSBw
YXRjaCB3YXMgcG9zdGVkIG9uIHRoZSBsaW51eC1ibG9jayBtYWlsaW5nDQpsaXN0IHJlY2VudGx5
IHRoYXQgc3Vycm91bmRzIHRoYXQgY2FsbCB3aXRoIGJsa19xdWV1ZV9lbnRlcigpIC8gYmxrX3F1
ZXVlX2V4aXQoKS4NCkkgdGhpbmsgdGhhdCBwcmV2ZW50cyB0aGF0IGJsa2NnX2V4aXRfcXVldWUo
KSBpcyBjYWxsZWQgY29uY3VycmVudGx5IHdpdGgNCmJsa2dfbG9va3VwX2NyZWF0ZSgpLg0KDQpC
YXJ0Lg0KDQoNCg0K

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

Hello,

On Thu, Apr 12, 2018 at 04:03:52PM +0000, Bart Van Assche wrote:
> On Thu, 2018-04-12 at 08:37 -0700, Tejun Heo wrote:
> > On Thu, Apr 12, 2018 at 08:09:17AM -0600, Bart Van Assche wrote:
> > > I have retested hotunplugging by rerunning the srp-test software. It
> > > seems like you overlooked that this patch does not remove the
> > > blkcg_exit_queue() call from blk_cleanup_queue()? If a device is
> > > hotunplugged it is up to the block driver to call
> > > blk_cleanup_queue(). And blk_cleanup_queue() will call
> > > blkcg_exit_queue().
> > 
> > Hmm... what'd prevent blg_lookup_and_create() racing against that?
> 
> Hello Tejun,
> 
> Did you perhaps mean blkg_lookup_create()? That function has one caller,

Ah yeah, sorry about the sloppiness.

> namely blkcg_bio_issue_check(). The only caller of that function is
> generic_make_request_checks(). A patch was posted on the linux-block mailing
> list recently that surrounds that call with blk_queue_enter() / blk_queue_exit().
> I think that prevents that blkcg_exit_queue() is called concurrently with
> blkg_lookup_create().

Yeah, that'd solve the problem for that particular path, but what
that's doing is adding another layer of refcnting which is used to
implement the revoke (or sever) semantics.  This is a fragile approach
tho because it isn't clear who's protecting what and there's nothing
in blkg's usage which suggests it'd be protected that way and we're
basically working around a different underlying problem (lock
switching) by expanding the coverage of a different lock.

I'd much prefer fixing the lock switching problem properly than
working around that shortcoming this way.

Thans.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

T24gVGh1LCAyMDE4LTA0LTEyIGF0IDA5OjEyIC0wNzAwLCB0akBrZXJuZWwub3JnIHdyb3RlOg0K
PiA+IERpZCB5b3UgcGVyaGFwcyBtZWFuIGJsa2dfbG9va3VwX2NyZWF0ZSgpPyBUaGF0IGZ1bmN0
aW9uIGhhcyBvbmUgY2FsbGVyLA0KPiA+IG5hbWVseSBibGtjZ19iaW9faXNzdWVfY2hlY2soKS4g
VGhlIG9ubHkgY2FsbGVyIG9mIHRoYXQgZnVuY3Rpb24gaXMNCj4gPiBnZW5lcmljX21ha2VfcmVx
dWVzdF9jaGVja3MoKS4gQSBwYXRjaCB3YXMgcG9zdGVkIG9uIHRoZSBsaW51eC1ibG9jayBtYWls
aW5nDQo+ID4gbGlzdCByZWNlbnRseSB0aGF0IHN1cnJvdW5kcyB0aGF0IGNhbGwgd2l0aCBibGtf
cXVldWVfZW50ZXIoKSAvIGJsa19xdWV1ZV9leGl0KCkuDQo+ID4gSSB0aGluayB0aGF0IHByZXZl
bnRzIHRoYXQgYmxrY2dfZXhpdF9xdWV1ZSgpIGlzIGNhbGxlZCBjb25jdXJyZW50bHkgd2l0aA0K
PiA+IGJsa2dfbG9va3VwX2NyZWF0ZSgpLg0KPiANCj4gWWVhaCwgdGhhdCdkIHNvbHZlIHRoZSBw
cm9ibGVtIGZvciB0aGF0IHBhcnRpY3VsYXIgcGF0aCwgYnV0IHdoYXQNCj4gdGhhdCdzIGRvaW5n
IGlzIGFkZGluZyBhbm90aGVyIGxheWVyIG9mIHJlZmNudGluZyB3aGljaCBpcyB1c2VkIHRvDQo+
IGltcGxlbWVudCB0aGUgcmV2b2tlIChvciBzZXZlcikgc2VtYW50aWNzLiAgVGhpcyBpcyBhIGZy
YWdpbGUgYXBwcm9hY2gNCj4gdGhvIGJlY2F1c2UgaXQgaXNuJ3QgY2xlYXIgd2hvJ3MgcHJvdGVj
dGluZyB3aGF0IGFuZCB0aGVyZSdzIG5vdGhpbmcNCj4gaW4gYmxrZydzIHVzYWdlIHdoaWNoIHN1
Z2dlc3RzIGl0J2QgYmUgcHJvdGVjdGVkIHRoYXQgd2F5IGFuZCB3ZSdyZQ0KPiBiYXNpY2FsbHkg
d29ya2luZyBhcm91bmQgYSBkaWZmZXJlbnQgdW5kZXJseWluZyBwcm9ibGVtIChsb2NrDQo+IHN3
aXRjaGluZykgYnkgZXhwYW5kaW5nIHRoZSBjb3ZlcmFnZSBvZiBhIGRpZmZlcmVudCBsb2NrLg0K
DQpIZWxsbyBUZWp1biwNCg0KQW55IGNvZGUgdGhhdCBzdWJtaXRzIGEgYmlvIG9yIHJlcXVlc3Qg
bmVlZHMgYmxrX3F1ZXVlX2VudGVyKCkgLw0KYmxrX3F1ZXVlX2V4aXQoKSBhbnl3YXkuIFBsZWFz
ZSBoYXZlIGEgbG9vayBhdCB0aGUgZm9sbG93aW5nIGNvbW1pdCAtIHlvdSB3aWxsDQpzZWUgdGhh
dCB0aGF0IGNvbW1pdCByZWR1Y2VzIHRoZSBudW1iZXIgb2YgYmxrX3F1ZXVlX2VudGVyKCkgLyBi
bGtfcXVldWVfZXhpdCgpDQpjYWxscyBpbiB0aGUgaG90IHBhdGg6DQoNCmh0dHBzOi8vZ2l0Lmtl
cm5lbC5vcmcvcHViL3NjbS9saW51eC9rZXJuZWwvZ2l0L2F4Ym9lL2xpbnV4LWJsb2NrLmdpdC9j
b21taXQvP2g9Zm9yLWxpbnVzJmlkPTM3Zjk1NzlmNGMzMWE2ZDY5OGRiZjMwMTZkN2JmMTMyZjky
ODhkMzANCg0KVGhhbmtzLA0KDQpCYXJ0Lg0KDQoNCg==

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

Hello,

On Thu, Apr 12, 2018 at 04:29:09PM +0000, Bart Van Assche wrote:
> Any code that submits a bio or request needs blk_queue_enter() /
> blk_queue_exit() anyway. Please have a look at the following commit - you will
> see that that commit reduces the number of blk_queue_enter() / blk_queue_exit()
> calls in the hot path:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-linus&id=37f9579f4c31a6d698dbf3016d7bf132f9288d30

So, this can work, but it's still pretty fragile.

* Lock switching is fragile and we really should get rid of it.  This
  is very likely to come back and bite us.

* Right now, blk_queue_enter/exit() doesn't have any annotations.
  IOW, there's no way for paths which need enter locked to actually
  assert that.  If we're gonna protect more things with queue
  enter/exit, it gotta be annotated.

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

T24gVGh1LCAyMDE4LTA0LTEyIGF0IDExOjExIC0wNzAwLCB0akBrZXJuZWwub3JnIHdyb3RlOg0K
PiAqIFJpZ2h0IG5vdywgYmxrX3F1ZXVlX2VudGVyL2V4aXQoKSBkb2Vzbid0IGhhdmUgYW55IGFu
bm90YXRpb25zLg0KPiAgIElPVywgdGhlcmUncyBubyB3YXkgZm9yIHBhdGhzIHdoaWNoIG5lZWQg
ZW50ZXIgbG9ja2VkIHRvIGFjdHVhbGx5DQo+ICAgYXNzZXJ0IHRoYXQuICBJZiB3ZSdyZSBnb25u
YSBwcm90ZWN0IG1vcmUgdGhpbmdzIHdpdGggcXVldWUNCj4gICBlbnRlci9leGl0LCBpdCBnb3R0
YSBiZSBhbm5vdGF0ZWQuDQoNCkhlbGxvIFRlanVuLA0KDQpPbmUgcG9zc2liaWxpdHkgaXMgdG8g
Y2hlY2sgdGhlIGNvdW50IGZvciB0aGUgbG9jYWwgQ1BVIG9mIHEtPnFfdXNhZ2VfY291bnRlcg0K
YXQgcnVudGltZSwgZS5nLiB1c2luZyBXQVJOX09OX09OQ0UoKS4gSG93ZXZlciwgdGhhdCBtaWdo
dCBoYXZlIGEgcnVudGltZQ0Kb3ZlcmhlYWQuIEFub3RoZXIgcG9zc2liaWxpdHkgaXMgdG8gZm9s
bG93IHRoZSBleGFtcGxlIG9mIGtlcm5mcyBhbmQgdG8gdXNlDQphIGxvY2tkZXAgbWFwIGFuZCBs
b2NrZGVwX2Fzc2VydF9oZWxkKCkuIFRoZXJlIG1pZ2h0IGJlIG90aGVyIGFsdGVybmF0aXZlcyBJ
DQpoYXZlIG5vdCB0aG91Z2h0IG9mLg0KDQpCYXJ0Lg0KDQoNCg0K

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

Hello,

On Thu, Apr 12, 2018 at 06:56:26PM +0000, Bart Van Assche wrote:
> On Thu, 2018-04-12 at 11:11 -0700, tj@kernel.org wrote:
> > * Right now, blk_queue_enter/exit() doesn't have any annotations.
> >   IOW, there's no way for paths which need enter locked to actually
> >   assert that.  If we're gonna protect more things with queue
> >   enter/exit, it gotta be annotated.
> 
> Hello Tejun,
> 
> One possibility is to check the count for the local CPU of q->q_usage_counter
> at runtime, e.g. using WARN_ON_ONCE(). However, that might have a runtime
> overhead. Another possibility is to follow the example of kernfs and to use
> a lockdep map and lockdep_assert_held(). There might be other alternatives I
> have not thought of.

Oh, I'd just do straight-forward lockdep annotation on
queue_enter/exit functions and provide the necessary assert helpers.

Thanks.

-- 
tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[Bart Van Assche]

T24gVGh1LCAyMDE4LTA0LTEyIGF0IDEyOjA5IC0wNzAwLCB0akBrZXJuZWwub3JnIHdyb3RlOg0K
PiBPbiBUaHUsIEFwciAxMiwgMjAxOCBhdCAwNjo1NjoyNlBNICswMDAwLCBCYXJ0IFZhbiBBc3Nj
aGUgd3JvdGU6DQo+ID4gT24gVGh1LCAyMDE4LTA0LTEyIGF0IDExOjExIC0wNzAwLCB0akBrZXJu
ZWwub3JnIHdyb3RlOg0KPiA+ID4gKiBSaWdodCBub3csIGJsa19xdWV1ZV9lbnRlci9leGl0KCkg
ZG9lc24ndCBoYXZlIGFueSBhbm5vdGF0aW9ucy4NCj4gPiA+ICAgSU9XLCB0aGVyZSdzIG5vIHdh
eSBmb3IgcGF0aHMgd2hpY2ggbmVlZCBlbnRlciBsb2NrZWQgdG8gYWN0dWFsbHkNCj4gPiA+ICAg
YXNzZXJ0IHRoYXQuICBJZiB3ZSdyZSBnb25uYSBwcm90ZWN0IG1vcmUgdGhpbmdzIHdpdGggcXVl
dWUNCj4gPiA+ICAgZW50ZXIvZXhpdCwgaXQgZ290dGEgYmUgYW5ub3RhdGVkLg0KPiA+IA0KPiA+
IEhlbGxvIFRlanVuLA0KPiA+IA0KPiA+IE9uZSBwb3NzaWJpbGl0eSBpcyB0byBjaGVjayB0aGUg
Y291bnQgZm9yIHRoZSBsb2NhbCBDUFUgb2YgcS0+cV91c2FnZV9jb3VudGVyDQo+ID4gYXQgcnVu
dGltZSwgZS5nLiB1c2luZyBXQVJOX09OX09OQ0UoKS4gSG93ZXZlciwgdGhhdCBtaWdodCBoYXZl
IGEgcnVudGltZQ0KPiA+IG92ZXJoZWFkLiBBbm90aGVyIHBvc3NpYmlsaXR5IGlzIHRvIGZvbGxv
dyB0aGUgZXhhbXBsZSBvZiBrZXJuZnMgYW5kIHRvIHVzZQ0KPiA+IGEgbG9ja2RlcCBtYXAgYW5k
IGxvY2tkZXBfYXNzZXJ0X2hlbGQoKS4gVGhlcmUgbWlnaHQgYmUgb3RoZXIgYWx0ZXJuYXRpdmVz
IEkNCj4gPiBoYXZlIG5vdCB0aG91Z2h0IG9mLg0KPiANCj4gT2gsIEknZCBqdXN0IGRvIHN0cmFp
Z2h0LWZvcndhcmQgbG9ja2RlcCBhbm5vdGF0aW9uIG9uDQo+IHF1ZXVlX2VudGVyL2V4aXQgZnVu
Y3Rpb25zIGFuZCBwcm92aWRlIHRoZSBuZWNlc3NhcnkgYXNzZXJ0IGhlbHBlcnMuDQoNCkhlbGxv
IFRlanVuLA0KDQpJcyBzb21ldGhpbmcgbGlrZSB0aGUgcGF0Y2ggYmVsb3cgcGVyaGFwcyB3aGF0
IHlvdSBoYWQgaW4gbWluZD8NCg0KVGhhbmtzLA0KDQpCYXJ0Lg0KDQpTdWJqZWN0OiBbUEFUQ0hd
IGJsb2NrOiBVc2UgbG9ja2RlcCB0byB2ZXJpZnkgd2hldGhlciBibGtfcXVldWVfZW50ZXIoKSBp
cw0KIHVzZWQgd2hlbiBuZWNlc3NhcnkNCg0KU3VnZ2VzdGVkLWJ5OiBUZWp1biBIZW8gPHRqQGtl
cm5lbC5vcmc+DQpTaWduZWQtb2ZmLWJ5OiBCYXJ0IFZhbiBBc3NjaGUgPGJhcnQudmFuYXNzY2hl
QHdkYy5jb20+DQpDYzogVGVqdW4gSGVvIDx0akBrZXJuZWwub3JnPg0KLS0tDQogYmxvY2svYmxr
LWNncm91cC5jICAgICAgICAgfCAgMiArKw0KIGJsb2NrL2Jsay1jb3JlLmMgICAgICAgICAgIHwg
MTMgKysrKysrKysrKysrLQ0KIGJsb2NrL2Jsay5oICAgICAgICAgICAgICAgIHwgIDEgKw0KIGlu
Y2x1ZGUvbGludXgvYmxrLWNncm91cC5oIHwgIDIgKysNCiBpbmNsdWRlL2xpbnV4L2Jsa2Rldi5o
ICAgICB8ICAxICsNCiA1IGZpbGVzIGNoYW5nZWQsIDE4IGluc2VydGlvbnMoKyksIDEgZGVsZXRp
b24oLSkNCg0KZGlmZiAtLWdpdCBhL2Jsb2NrL2Jsay1jZ3JvdXAuYyBiL2Jsb2NrL2Jsay1jZ3Jv
dXAuYw0KaW5kZXggMWMxNjY5NGFlMTQ1Li5jMzRlMTNlNzZmOTAgMTAwNjQ0DQotLS0gYS9ibG9j
ay9ibGstY2dyb3VwLmMNCisrKyBiL2Jsb2NrL2Jsay1jZ3JvdXAuYw0KQEAgLTE0NSw2ICsxNDUs
OCBAQCBzdHJ1Y3QgYmxrY2dfZ3EgKmJsa2dfbG9va3VwX3Nsb3dwYXRoKHN0cnVjdCBibGtjZyAq
YmxrY2csDQogew0KIAlzdHJ1Y3QgYmxrY2dfZ3EgKmJsa2c7DQogDQorCWxvY2tfaXNfaGVsZCgm
cS0+cV9lbnRlcl9tYXApOw0KKw0KIAkvKg0KIAkgKiBIaW50IGRpZG4ndCBtYXRjaC4gIExvb2sg
dXAgZnJvbSB0aGUgcmFkaXggdHJlZS4gIE5vdGUgdGhhdCB0aGUNCiAJICogaGludCBjYW4gb25s
eSBiZSB1cGRhdGVkIHVuZGVyIHF1ZXVlX2xvY2sgYXMgb3RoZXJ3aXNlIEBibGtnDQpkaWZmIC0t
Z2l0IGEvYmxvY2svYmxrLWNvcmUuYyBiL2Jsb2NrL2Jsay1jb3JlLmMNCmluZGV4IDM5MzA4ZTg3
NGZmYS4uYTYxZGJlN2YyNGE1IDEwMDY0NA0KLS0tIGEvYmxvY2svYmxrLWNvcmUuYw0KKysrIGIv
YmxvY2svYmxrLWNvcmUuYw0KQEAgLTkzMSw4ICs5MzEsMTMgQEAgaW50IGJsa19xdWV1ZV9lbnRl
cihzdHJ1Y3QgcmVxdWVzdF9xdWV1ZSAqcSwgYmxrX21xX3JlcV9mbGFnc190IGZsYWdzKQ0KIAkJ
fQ0KIAkJcmN1X3JlYWRfdW5sb2NrKCk7DQogDQotCQlpZiAoc3VjY2VzcykNCisJCWlmIChzdWNj
ZXNzKSB7DQorCQkJbXV0ZXhfYWNxdWlyZV9uZXN0KCZxLT5xX2VudGVyX21hcCwgMCwgMCwNCisJ
CQkJCSAgIGxvY2tfaXNfaGVsZCgmcS0+cV9lbnRlcl9tYXApID8NCisJCQkJCSAgICZxLT5xX2Vu
dGVyX21hcCA6IE5VTEwsDQorCQkJCQkgICBfUkVUX0lQXyk7DQogCQkJcmV0dXJuIDA7DQorCQl9
DQogDQogCQlpZiAoZmxhZ3MgJiBCTEtfTVFfUkVRX05PV0FJVCkNCiAJCQlyZXR1cm4gLUVCVVNZ
Ow0KQEAgLTk1OSw2ICs5NjQsNyBAQCBpbnQgYmxrX3F1ZXVlX2VudGVyKHN0cnVjdCByZXF1ZXN0
X3F1ZXVlICpxLCBibGtfbXFfcmVxX2ZsYWdzX3QgZmxhZ3MpDQogDQogdm9pZCBibGtfcXVldWVf
ZXhpdChzdHJ1Y3QgcmVxdWVzdF9xdWV1ZSAqcSkNCiB7DQorCW11dGV4X3JlbGVhc2UoJnEtPnFf
ZW50ZXJfbWFwLCAwLCBfUkVUX0lQXyk7DQogCXBlcmNwdV9yZWZfcHV0KCZxLT5xX3VzYWdlX2Nv
dW50ZXIpOw0KIH0NCiANCkBAIC05OTQsMTIgKzEwMDAsMTUgQEAgc3RydWN0IHJlcXVlc3RfcXVl
dWUgKmJsa19hbGxvY19xdWV1ZV9ub2RlKGdmcF90IGdmcF9tYXNrLCBpbnQgbm9kZV9pZCwNCiAJ
CQkJCSAgIHNwaW5sb2NrX3QgKmxvY2spDQogew0KIAlzdHJ1Y3QgcmVxdWVzdF9xdWV1ZSAqcTsN
CisJc3RhdGljIHN0cnVjdCBsb2NrX2NsYXNzX2tleSBfX2tleTsNCiANCiAJcSA9IGttZW1fY2Fj
aGVfYWxsb2Nfbm9kZShibGtfcmVxdWVzdHFfY2FjaGVwLA0KIAkJCQlnZnBfbWFzayB8IF9fR0ZQ
X1pFUk8sIG5vZGVfaWQpOw0KIAlpZiAoIXEpDQogCQlyZXR1cm4gTlVMTDsNCiANCisJbG9ja2Rl
cF9pbml0X21hcCgmcS0+cV9lbnRlcl9tYXAsICJxX2VudGVyX21hcCIsICZfX2tleSwgMCk7DQor
DQogCXEtPmlkID0gaWRhX3NpbXBsZV9nZXQoJmJsa19xdWV1ZV9pZGEsIDAsIDAsIGdmcF9tYXNr
KTsNCiAJaWYgKHEtPmlkIDwgMCkNCiAJCWdvdG8gZmFpbF9xOw0KQEAgLTIyNjQsNiArMjI3Myw4
IEBAIGdlbmVyaWNfbWFrZV9yZXF1ZXN0X2NoZWNrcyhzdHJ1Y3QgYmlvICpiaW8pDQogCQlnb3Rv
IGVuZF9pbzsNCiAJfQ0KIA0KKwlsb2NrX2lzX2hlbGQoJnEtPnFfZW50ZXJfbWFwKTsNCisNCiAJ
LyoNCiAJICogRm9yIGEgUkVRX05PV0FJVCBiYXNlZCByZXF1ZXN0LCByZXR1cm4gLUVPUE5PVFNV
UFANCiAJICogaWYgcXVldWUgaXMgbm90IGEgcmVxdWVzdCBiYXNlZCBxdWV1ZS4NCmRpZmYgLS1n
aXQgYS9ibG9jay9ibGsuaCBiL2Jsb2NrL2Jsay5oDQppbmRleCA3Y2Q2NGY1MzNhNDYuLjI2ZjMx
MzMzMWIxMyAxMDA2NDQNCi0tLSBhL2Jsb2NrL2Jsay5oDQorKysgYi9ibG9jay9ibGsuaA0KQEAg
LTE0NSw2ICsxNDUsNyBAQCBzdGF0aWMgaW5saW5lIHZvaWQgYmxrX3F1ZXVlX2VudGVyX2xpdmUo
c3RydWN0IHJlcXVlc3RfcXVldWUgKnEpDQogCSAqIGJlZW4gZXN0YWJsaXNoZWQsIGZ1cnRoZXIg
cmVmZXJlbmNlcyB1bmRlciB0aGF0IHNhbWUgY29udGV4dA0KIAkgKiBuZWVkIG5vdCBjaGVjayB0
aGF0IHRoZSBxdWV1ZSBoYXMgYmVlbiBmcm96ZW4gKG1hcmtlZCBkZWFkKS4NCiAJICovDQorCW11
dGV4X2FjcXVpcmVfbmVzdCgmcS0+cV9lbnRlcl9tYXAsIDAsIDAsICZxLT5xX2VudGVyX21hcCwg
X1JFVF9JUF8pOw0KIAlwZXJjcHVfcmVmX2dldCgmcS0+cV91c2FnZV9jb3VudGVyKTsNCiB9DQog
DQpkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9ibGstY2dyb3VwLmggYi9pbmNsdWRlL2xpbnV4
L2Jsay1jZ3JvdXAuaA0KaW5kZXggNmM2NjZmZDdkZTNjLi41MmUyZTJkOTk3MWUgMTAwNjQ0DQot
LS0gYS9pbmNsdWRlL2xpbnV4L2Jsay1jZ3JvdXAuaA0KKysrIGIvaW5jbHVkZS9saW51eC9ibGst
Y2dyb3VwLmgNCkBAIC0yNjYsNiArMjY2LDggQEAgc3RhdGljIGlubGluZSBzdHJ1Y3QgYmxrY2df
Z3EgKl9fYmxrZ19sb29rdXAoc3RydWN0IGJsa2NnICpibGtjZywNCiB7DQogCXN0cnVjdCBibGtj
Z19ncSAqYmxrZzsNCiANCisJbG9ja19pc19oZWxkKCZxLT5xX2VudGVyX21hcCk7DQorDQogCWlm
IChibGtjZyA9PSAmYmxrY2dfcm9vdCkNCiAJCXJldHVybiBxLT5yb290X2Jsa2c7DQogDQpkaWZm
IC0tZ2l0IGEvaW5jbHVkZS9saW51eC9ibGtkZXYuaCBiL2luY2x1ZGUvbGludXgvYmxrZGV2LmgN
CmluZGV4IDBkMjJmMzUxYTc0Yi4uYTBiMWFkYmQ0NDA2IDEwMDY0NA0KLS0tIGEvaW5jbHVkZS9s
aW51eC9ibGtkZXYuaA0KKysrIGIvaW5jbHVkZS9saW51eC9ibGtkZXYuaA0KQEAgLTYzMSw2ICs2
MzEsNyBAQCBzdHJ1Y3QgcmVxdWVzdF9xdWV1ZSB7DQogCXN0cnVjdCByY3VfaGVhZAkJcmN1X2hl
YWQ7DQogCXdhaXRfcXVldWVfaGVhZF90CW1xX2ZyZWV6ZV93cTsNCiAJc3RydWN0IHBlcmNwdV9y
ZWYJcV91c2FnZV9jb3VudGVyOw0KKwlzdHJ1Y3QgbG9ja2RlcF9tYXAJcV9lbnRlcl9tYXA7DQog
CXN0cnVjdCBsaXN0X2hlYWQJYWxsX3Ffbm9kZTsNCiANCiAJc3RydWN0IGJsa19tcV90YWdfc2V0
CSp0YWdfc2V0Ow0K

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

[tj]

Hello, Bart.

On Thu, Apr 12, 2018 at 10:40:52PM +0000, Bart Van Assche wrote:
> Is something like the patch below perhaps what you had in mind?

Yeah, exactly.  It'd be really great to have the lockdep asserts add
to the right places.

Thanks.

-- 
tejun