[v2,1/3] block: warn if sharing request queue across gendisks

Commit Message

Omar Sandoval March 28, 2017, 11:12 p.m. UTC

From: Omar Sandoval <[e-mail]>

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 <[e-mail]>
---
 block/blk-sysfs.c      | 7 +++++++
 include/linux/blkdev.h | 1 +
 2 files changed, 8 insertions(+)

Comments

Jens Axboe March 29, 2017, 2:10 p.m. UTC | #1

On Tue, Mar 28 2017, Omar Sandoval wrote:
> From: Omar Sandoval <[e-mail]>
> 
> 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.

Omar Sandoval March 29, 2017, 5:06 p.m. UTC | #2

On Wed, Mar 29, 2017 at 08:10:35AM -0600, Jens Axboe wrote:
> On Tue, Mar 28 2017, Omar Sandoval wrote:
> > From: Omar Sandoval <[e-mail]>
> > 
> > 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.

Patch

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)	|	\