* [PATCH v2 1/3] block: warn if sharing request queue across gendisks
@ 2017-03-28 23:12 Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 2/3] blk-mq: fix leak of q->stats Omar Sandoval
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Omar Sandoval @ 2017-03-28 23:12 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
Now that the remaining drivers have been converted to one request queue
per gendisk, let's warn if a request queue gets registered more than
once. This will catch future drivers which might do it inadvertently or
any old drivers that I may have missed.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
block/blk-sysfs.c | 7 +++++++
include/linux/blkdev.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 7f090dd15ca6..833fb7f9ce9d 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -871,6 +871,11 @@ int blk_register_queue(struct gendisk *disk)
if (WARN_ON(!q))
return -ENXIO;
+ WARN_ONCE(test_bit(QUEUE_FLAG_REGISTERED, &q->queue_flags),
+ "%s is registering an already registered queue\n",
+ kobject_name(&dev->kobj));
+ queue_flag_set_unlocked(QUEUE_FLAG_REGISTERED, q);
+
/*
* SCSI probing may synchronously create and destroy a lot of
* request_queues for non-existent devices. Shutting down a fully
@@ -931,6 +936,8 @@ void blk_unregister_queue(struct gendisk *disk)
if (WARN_ON(!q))
return;
+ queue_flag_clear_unlocked(QUEUE_FLAG_REGISTERED, q);
+
if (q->mq_ops)
blk_mq_unregister_dev(disk_to_dev(disk), q);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1a7dc42a8918..a2dc6b390d48 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -617,6 +617,7 @@ struct request_queue {
#define QUEUE_FLAG_STATS 27 /* track rq completion times */
#define QUEUE_FLAG_RESTART 28 /* queue needs restart at completion */
#define QUEUE_FLAG_POLL_STATS 29 /* collecting stats for hybrid polling */
+#define QUEUE_FLAG_REGISTERED 30 /* queue has been registered to a disk */
#define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
(1 << QUEUE_FLAG_STACKABLE) | \
--
2.12.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/3] blk-mq: fix leak of q->stats
2017-03-28 23:12 [PATCH v2 1/3] block: warn if sharing request queue across gendisks Omar Sandoval
@ 2017-03-28 23:12 ` Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 3/3] block: fix leak of q->rq_wb Omar Sandoval
2017-03-29 14:10 ` [PATCH v2 1/3] block: warn if sharing request queue across gendisks Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: Omar Sandoval @ 2017-03-28 23:12 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
blk_alloc_queue_node() already allocates q->stats, so
blk_mq_init_allocated_queue() is overwriting it with a new allocation.
Fixes: a83b576c9c25 ("block: fix stacked driver stats init and free")
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
block/blk-mq.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7df9dbfab022..00eeef31db30 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2212,10 +2212,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
/* mark the queue as mq asap */
q->mq_ops = set->ops;
- q->stats = blk_alloc_queue_stats();
- if (!q->stats)
- goto err_exit;
-
q->poll_cb = blk_stat_alloc_callback(blk_mq_poll_stats_fn,
blk_stat_rq_ddir, 2, q);
if (!q->poll_cb)
--
2.12.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/3] block: fix leak of q->rq_wb
2017-03-28 23:12 [PATCH v2 1/3] block: warn if sharing request queue across gendisks Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 2/3] blk-mq: fix leak of q->stats Omar Sandoval
@ 2017-03-28 23:12 ` Omar Sandoval
2017-03-29 14:10 ` [PATCH v2 1/3] block: warn if sharing request queue across gendisks Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: Omar Sandoval @ 2017-03-28 23:12 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
CONFIG_DEBUG_TEST_DRIVER_REMOVE found a possible leak of q->rq_wb when a
request queue is reregistered. This has been a problem since wbt was
introduced, but the WARN_ON(!list_empty(&stats->callbacks)) in the
blk-stat rework exposed it. Fix it by cleaning up wbt when we unregister
the queue.
Fixes: 87760e5eef35 ("block: hook up writeback throttling")
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
block/blk-sysfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 833fb7f9ce9d..45854266e398 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -795,7 +795,6 @@ static void blk_release_queue(struct kobject *kobj)
struct request_queue *q =
container_of(kobj, struct request_queue, kobj);
- wbt_exit(q);
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);
@@ -938,6 +937,9 @@ void blk_unregister_queue(struct gendisk *disk)
queue_flag_clear_unlocked(QUEUE_FLAG_REGISTERED, q);
+ wbt_exit(q);
+
+
if (q->mq_ops)
blk_mq_unregister_dev(disk_to_dev(disk), q);
--
2.12.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] block: warn if sharing request queue across gendisks
2017-03-28 23:12 [PATCH v2 1/3] block: warn if sharing request queue across gendisks Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 2/3] blk-mq: fix leak of q->stats Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 3/3] block: fix leak of q->rq_wb Omar Sandoval
@ 2017-03-29 14:10 ` Jens Axboe
2017-03-29 17:06 ` Omar Sandoval
2 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2017-03-29 14:10 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, kernel-team
On Tue, Mar 28 2017, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Now that the remaining drivers have been converted to one request queue
> per gendisk, let's warn if a request queue gets registered more than
> once. This will catch future drivers which might do it inadvertently or
> any old drivers that I may have missed.
Added 1-3 for 4.12, thanks. Final nail in the coffin for shared queues.
BTW, can you include a cover letter when it's more than one patch? Makes
the flow a bit more straight forward.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] block: warn if sharing request queue across gendisks
2017-03-29 14:10 ` [PATCH v2 1/3] block: warn if sharing request queue across gendisks Jens Axboe
@ 2017-03-29 17:06 ` Omar Sandoval
0 siblings, 0 replies; 5+ messages in thread
From: Omar Sandoval @ 2017-03-29 17:06 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, kernel-team
On Wed, Mar 29, 2017 at 08:10:35AM -0600, Jens Axboe wrote:
> On Tue, Mar 28 2017, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > Now that the remaining drivers have been converted to one request queue
> > per gendisk, let's warn if a request queue gets registered more than
> > once. This will catch future drivers which might do it inadvertently or
> > any old drivers that I may have missed.
>
> Added 1-3 for 4.12, thanks. Final nail in the coffin for shared queues.
Thanks!
> BTW, can you include a cover letter when it's more than one patch? Makes
> the flow a bit more straight forward.
Yeah, I'll make sure to do that in the future.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-29 17:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 23:12 [PATCH v2 1/3] block: warn if sharing request queue across gendisks Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 2/3] blk-mq: fix leak of q->stats Omar Sandoval
2017-03-28 23:12 ` [PATCH v2 3/3] block: fix leak of q->rq_wb Omar Sandoval
2017-03-29 14:10 ` [PATCH v2 1/3] block: warn if sharing request queue across gendisks Jens Axboe
2017-03-29 17:06 ` Omar Sandoval
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).