Re: [PATCH 1/1] block: rework flush sequencing for blk-mq

[Christoph Hellwig]

On Fri, Mar 14, 2014 at 02:25:19AM -0700, Christoph Hellwig wrote:
> b) is a bit harder, but we should think hard about it when rewriting the
> multipath code to support blk-mq.  Talking about which I think trying to
> use dm-multipath on any blk-mq device will go horribly crash and boom at
> the moment.

Talking abnout crashing and burning.. Hannes, did you run this patch
past dm-devel and linux-scsi yet?  Don't quite like it but the problem
seems real..


From: Hannes Reinecke <hare@xxxxxxx>
Subject: Kernel bug triggered in multipath
References: bnc#486001
Patch-mainline: not yet

Starting multipath on a cciss device will cause a kernel
warning to be triggered. Problem is that we're using the
->queuedata field of the request_queue to derefence the
scsi device; however, for other (non-SCSI) devices this
points to a totally different structure.
So we should rather be using accessors here which make
sure we're only returning valid SCSI device structures.

Signed-off-by: Hannes Reinecke <hare@xxxxxxx>

---
 drivers/scsi/device_handler/scsi_dh.c |   10 +++++-----
 drivers/scsi/scsi_lib.c               |   11 +++++++++++
 include/scsi/scsi_device.h            |    1 +
 3 files changed, 17 insertions(+), 5 deletions(-)

--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -397,7 +397,7 @@ int scsi_dh_activate(struct request_queu
 	struct device *dev = NULL;
 
 	spin_lock_irqsave(q->queue_lock, flags);
-	sdev = q->queuedata;
+	sdev = scsi_device_from_queue(q);
 	if (!sdev) {
		spin_unlock_irqrestore(q->queue_lock, flags);
		err = SCSI_DH_NOSYS;
@@ -484,7 +484,7 @@ int scsi_dh_attach(struct request_queue
 		return -EINVAL;
 
 	spin_lock_irqsave(q->queue_lock, flags);
-	sdev = q->queuedata;
+	sdev = scsi_device_from_queue(q);
 	if (!sdev || !get_device(&sdev->sdev_gendev))
 		err = -ENODEV;
 	spin_unlock_irqrestore(q->queue_lock, flags);
@@ -512,7 +512,7 @@ void scsi_dh_detach(struct request_queue
 	struct scsi_device_handler *scsi_dh = NULL;
 
 	spin_lock_irqsave(q->queue_lock, flags);
-	sdev = q->queuedata;
+	sdev = scsi_device_from_queue(q);
 	if (!sdev || !get_device(&sdev->sdev_gendev))
 		sdev = NULL;
 	spin_unlock_irqrestore(q->queue_lock, flags);
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1594,6 +1594,17 @@ out:
 	spin_lock_irq(q->queue_lock);
 }
 
+struct scsi_device *scsi_device_from_queue(struct request_queue *q)
+{
+	struct scsi_device *sdev = NULL;
+
+	if (q->request_fn == scsi_request_fn)
+		sdev = q->queuedata;
+
+	return sdev;
+}
+EXPORT_SYMBOL_GPL(scsi_device_from_queue);
+
 u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 {
 	struct device *host_dev;
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -300,6 +300,7 @@ extern void starget_for_each_device(stru
 extern void __starget_for_each_device(struct scsi_target *, void *,
 				      void (*fn)(struct scsi_device *,
 						 void *));
+extern struct scsi_device *scsi_device_from_queue(struct request_queue *);
 
 /* only exposed to implement shost_for_each_device */
 extern struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/