[PATCH] Use queuedata accessors for device handler

[PATCH] Use queuedata accessors for device handler

[Hannes Reinecke]

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 dereference the
scsi device; however, for other (non-SCSI) devices this
points to a totally different structure.
So we should rather be using accessors here to make
sure ->queuedata points to a valid sdev.

Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 0119b81..bc96ca6 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -441,7 +441,7 @@ int scsi_dh_set_params(struct request_queue *q, const char *params)
 	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 && sdev->scsi_dh_data)
 		scsi_dh = sdev->scsi_dh_data->scsi_dh;
 	if (scsi_dh && scsi_dh->set_params && get_device(&sdev->sdev_gendev))
@@ -484,7 +484,7 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 		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 *q)
 	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);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index fc3f168..a96fe54 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1614,6 +1614,24 @@ out:
 	spin_lock_irq(q->queue_lock);
 }
 
+/**
+ * scsi_device_from_queue - return sdev associated with a request_queue
+ * @q: The request queue to return the sdev from
+ *
+ * Return the sdev associated with a request queue or NULL if the
+ * request_queue does not reference a SCSI device.
+ */
+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;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d371c3c..6712446 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -301,6 +301,7 @@ extern void starget_for_each_device(struct scsi_target *, void *,
 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 *,

Re: [PATCH] Use queuedata accessors for device handler

[Christoph Hellwig]

On Mon, Aug 22, 2011 at 03:35:43PM +0200, Hannes Reinecke wrote:
> 
> 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 dereference the
> scsi device; however, for other (non-SCSI) devices this
> points to a totally different structure.
> So we should rather be using accessors here to make
> sure ->queuedata points to a valid sdev.

How do we match to attach a scsi device handler to a non-scsi queue?
I suspect that is the fundamental issue that needs addressing.

Re: [PATCH] Use queuedata accessors for device handler

[Hannes Reinecke]

On 08/22/2011 04:45 PM, Christoph Hellwig wrote:
> On Mon, Aug 22, 2011 at 03:35:43PM +0200, Hannes Reinecke wrote:
>>
>> 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 dereference the
>> scsi device; however, for other (non-SCSI) devices this
>> points to a totally different structure.
>> So we should rather be using accessors here to make
>> sure ->queuedata points to a valid sdev.
>
> How do we match to attach a scsi device handler to a non-scsi queue?
> I suspect that is the fundamental issue that needs addressing.
>
We don't. SCSI device handler are only for SCSI devices, not block 
devices. But scsi_dh_attach() and scsi_dh_detach() is called from 
dm-mpath.c:parse_path(), and this doesn't have any idea about the 
underlying device type.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html