scsi: use 'scsi_device_from_queue()' for scsi_dh

PATCH In review Hannes Reinecke 5 participants 5 messages Feb 17, 2017 – Feb 21, 2017 SCSI

Share & tools

Hannes Reinecke

author

2017-02-17 08:02 UTC #0 lore raw

PATCH v3 1/1 scsi: use 'scsi_device_from_queue()' for scsi_dh

The device handler needs to check if a given queue belongs to a scsi device; only then does it make sense to attach a device handler.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/scsi/scsi_dh.c     | 22 ++++------------------
 drivers/scsi/scsi_lib.c    | 24 ++++++++++++++++++++++++
 include/scsi/scsi_device.h |  1 +
 3 files changed, 29 insertions(+), 18 deletions(-)

drivers/scsi/scsi_dh.c +4 ~~-18~~

index b8d3b97..84addee 100644
@@ -219,20 +219,6 @@ int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh)
 }
 EXPORT_SYMBOL_GPL(scsi_unregister_device_handler);
 
-static struct scsi_device *get_sdev_from_queue(struct request_queue *q)
-{
-	struct scsi_device *sdev;
-	unsigned long flags;
-
-	spin_lock_irqsave(q->queue_lock, flags);
-	sdev = q->queuedata;
-	if (!sdev || !get_device(&sdev->sdev_gendev))
-		sdev = NULL;
-	spin_unlock_irqrestore(q->queue_lock, flags);
-
-	return sdev;
-}
-
 /*
  * scsi_dh_activate - activate the path associated with the scsi_device
  *      corresponding to the given request queue.

@@ -251,7 +237,7 @@ int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
 	struct scsi_device *sdev;
 	int err = SCSI_DH_NOSYS;
 
-	sdev = get_sdev_from_queue(q);
+	sdev = scsi_device_from_queue(q);
 	if (!sdev) {
 		if (fn)
 			fn(data, err);

@@ -298,7 +284,7 @@ int scsi_dh_set_params(struct request_queue *q, const char *params)
 	struct scsi_device *sdev;
 	int err = -SCSI_DH_NOSYS;
 
-	sdev = get_sdev_from_queue(q);
+	sdev = scsi_device_from_queue(q);
 	if (!sdev)
 		return err;

@@ -321,7 +307,7 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 	struct scsi_device_handler *scsi_dh;
 	int err = 0;
 
-	sdev = get_sdev_from_queue(q);
+	sdev = scsi_device_from_queue(q);
 	if (!sdev)
 		return -ENODEV;

@@ -359,7 +345,7 @@ const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
 	struct scsi_device *sdev;
 	const char *handler_name = NULL;
 
-	sdev = get_sdev_from_queue(q);
+	sdev = scsi_device_from_queue(q);
 	if (!sdev)
 		return NULL;

drivers/scsi/scsi_lib.c +24 -0

index 9d6aed5..1756cd8 100644
@@ -2145,6 +2145,30 @@ void scsi_mq_destroy_tags(struct Scsi_Host *shost)
 	blk_mq_free_tag_set(&shost->tag_set);
 }
 
+/**
+ * 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;
+	unsigned long flags;
+
+	if (q->mq_ops) {
+		if (q->mq_ops == &scsi_mq_ops)
+			sdev = q->queuedata;
+	} else if (q->request_fn == scsi_request_fn)
+		sdev = q->queuedata;
+	if (!sdev || !get_device(&sdev->sdev_gendev))
+		sdev = NULL;
+
+	return sdev;
+}
+EXPORT_SYMBOL_GPL(scsi_device_from_queue);
+
 /*
  * Function:    scsi_block_requests()
  *

include/scsi/scsi_device.h +1 -0

index 8990e58..be41c76 100644
@@ -315,6 +315,7 @@ extern int scsi_add_device(struct Scsi_Host *host, uint channel,
 extern int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh);
 void scsi_attach_vpd(struct scsi_device *sdev);
 
+extern struct scsi_device *scsi_device_from_queue(struct request_queue *q);
 extern int scsi_device_get(struct scsi_device *);
 extern void scsi_device_put(struct scsi_device *);
 extern struct scsi_device *scsi_device_lookup(struct Scsi_Host *,

Signature · 1 line

1.8.5.6

Christoph Hellwig

Reviewed-by on v3 1/1

2017-02-17 08:33 UTC #1 lore raw

Looks fine,

Reviewed-by Christoph Hellwig <[email protected]>

kbuild test robot

build bot

2017-02-17 09:47 UTC #2 lore raw

In reply to Hannes Reinecke

reproduce:

Hi Hannes,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.10-rc8 next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-use-scsi_device_from_queue-for-scsi_dh/20170217-161522
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-randconfig-a0-02171706 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

All warnings (new ones prefixed by >>):

drivers/scsi/scsi_lib.c: In function 'scsi_device_from_queue':

>> drivers/scsi/scsi_lib.c:2157: warning: unused variable 'flags'

vim +/flags +2157 drivers/scsi/scsi_lib.c

2141
2142 void scsi_mq_destroy_tags(struct Scsi_Host *shost)
2143 {
2144 blk_mq_free_tag_set(&shost->tag_set);
2145 }
2146
2147 /**
2148 * scsi_device_from_queue - return sdev associated with a request_queue
2149 * @q: The request queue to return the sdev from
2150 *
2151 * Return the sdev associated with a request queue or NULL if the
2152 * request_queue does not reference a SCSI device.
2153 */
2154 struct scsi_device *scsi_device_from_queue(struct request_queue *q)
2155 {
2156 struct scsi_device *sdev = NULL;

> 2157		unsigned long flags;

2158
2159 if (q->mq_ops) {
2160 if (q->mq_ops == &scsi_mq_ops)
2161 sdev = q->queuedata;
2162 } else if (q->request_fn == scsi_request_fn)
2163 sdev = q->queuedata;
2164 if (!sdev || !get_device(&sdev->sdev_gendev))
2165 sdev = NULL;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Martin K. Petersen

2017-02-21 03:22 UTC #3 lore raw

In reply to Hannes Reinecke

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hannes" == Hannes Reinecke <[email protected]> writes:

Hannes> The device handler needs to check if a given queue belongs to a Hannes> scsi device; only then does it make sense to attach a device Hannes> handler.

Fixed kbuild warning and applied to 4.11/scsi-queue.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Mike Snitzer

2017-02-21 16:36 UTC #4 lore raw

Quoted 7 lines · Martin K. Petersen

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hannes" == Hannes Reinecke <[email protected]> writes:
Hannes> The device handler needs to check if a given queue belongs to a Hannes> scsi device; only then does it make sense to attach a device Hannes> handler.
Fixed kbuild warning and applied to 4.11/scsi-queue.

Thanks for picking this up but Keith's approach is slightly more efficient and opens a way forward to expose support for both scsi_dh and whatever Christoph is calling his NVMe device handler. But we can revisit once that latter lands.