* [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer
@ 2020-10-15 13:36 Kashyap Desai
2020-11-13 11:51 ` John Garry
2020-11-20 7:16 ` Hannes Reinecke
0 siblings, 2 replies; 5+ messages in thread
From: Kashyap Desai @ 2020-10-15 13:36 UTC (permalink / raw)
To: linux-scsi; +Cc: Kashyap Desai, sumit.saxena, chandrakanth.patil, linux-block
[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]
io_uring with IOPOLL is not currently supported in scsi mid layer.
Outside of that everything else should work and no extra support in the driver is needed.
Currently io_uring with IOPOLL support is only available in block layer.
This patch is to extend support of mq_poll in scsi layer.
Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: sumit.saxena@broadcom.com
Cc: chandrakanth.patil@broadcom.com
Cc: linux-block@vger.kernel.org
---
drivers/scsi/scsi_lib.c | 16 ++++++++++++++++
include/scsi/scsi_cmnd.h | 1 +
include/scsi/scsi_host.h | 11 +++++++++++
3 files changed, 28 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 72b12102f777..5a3c383a2bb3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1766,6 +1766,19 @@ static void scsi_mq_exit_request(struct blk_mq_tag_set *set, struct request *rq,
cmd->sense_buffer);
}
+
+static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx)
+{
+ struct request_queue *q = hctx->queue;
+ struct scsi_device *sdev = q->queuedata;
+ struct Scsi_Host *shost = sdev->host;
+
+ if (shost->hostt->mq_poll)
+ return shost->hostt->mq_poll(shost, hctx->queue_num);
+
+ return 0;
+}
+
static int scsi_map_queues(struct blk_mq_tag_set *set)
{
struct Scsi_Host *shost = container_of(set, struct Scsi_Host, tag_set);
@@ -1833,6 +1846,7 @@ static const struct blk_mq_ops scsi_mq_ops_no_commit = {
.cleanup_rq = scsi_cleanup_rq,
.busy = scsi_mq_lld_busy,
.map_queues = scsi_map_queues,
+ .poll = scsi_mq_poll,
};
@@ -1861,6 +1875,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
.cleanup_rq = scsi_cleanup_rq,
.busy = scsi_mq_lld_busy,
.map_queues = scsi_map_queues,
+ .poll = scsi_mq_poll,
};
struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
@@ -1893,6 +1908,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
else
tag_set->ops = &scsi_mq_ops_no_commit;
tag_set->nr_hw_queues = shost->nr_hw_queues ? : 1;
+ tag_set->nr_maps = shost->nr_maps ? : 1;
tag_set->queue_depth = shost->can_queue;
tag_set->cmd_size = cmd_size;
tag_set->numa_node = NUMA_NO_NODE;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index e76bac4d14c5..5844374a85b1 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -9,6 +9,7 @@
#include <linux/types.h>
#include <linux/timer.h>
#include <linux/scatterlist.h>
+#include <scsi/scsi_host.h>
#include <scsi/scsi_device.h>
#include <scsi/scsi_request.h>
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 701f178b20ae..905ee6b00c55 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -270,6 +270,16 @@ struct scsi_host_template {
*/
int (* map_queues)(struct Scsi_Host *shost);
+ /*
+ * SCSI interface of blk_poll - poll for IO completions.
+ * Possible interface only if scsi LLD expose multiple h/w queues.
+ *
+ * Return values: Number of completed entries found.
+ *
+ * Status: OPTIONAL
+ */
+ int (* mq_poll)(struct Scsi_Host *shost, unsigned int queue_num);
+
/*
* Check if scatterlists need to be padded for DMA draining.
*
@@ -610,6 +620,7 @@ struct Scsi_Host {
* the total queue depth is can_queue.
*/
unsigned nr_hw_queues;
+ unsigned nr_maps;
unsigned active_mode:2;
unsigned unchecked_isa_dma:1;
--
2.18.1
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4169 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer
2020-10-15 13:36 [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer Kashyap Desai
@ 2020-11-13 11:51 ` John Garry
2020-11-30 7:41 ` Kashyap Desai
2020-11-20 7:16 ` Hannes Reinecke
1 sibling, 1 reply; 5+ messages in thread
From: John Garry @ 2020-11-13 11:51 UTC (permalink / raw)
To: Kashyap Desai, linux-scsi; +Cc: sumit.saxena, chandrakanth.patil, linux-block
On 15/10/2020 14:36, Kashyap Desai wrote:
> io_uring with IOPOLL is not currently supported in scsi mid layer.
> Outside of that everything else should work and no extra support in the driver is needed.
> Currently io_uring with IOPOLL support is only available in block layer.
> This patch is to extend support of mq_poll in scsi layer. >
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Cc: sumit.saxena@broadcom.com
> Cc: chandrakanth.patil@broadcom.com
> Cc: linux-block@vger.kernel.org
>
> ---
> drivers/scsi/scsi_lib.c | 16 ++++++++++++++++
> include/scsi/scsi_cmnd.h | 1 +
> include/scsi/scsi_host.h | 11 +++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 72b12102f777..5a3c383a2bb3 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1766,6 +1766,19 @@ static void scsi_mq_exit_request(struct blk_mq_tag_set *set, struct request *rq,
> cmd->sense_buffer);
> }
>
> +
> +static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx)
> +{
> + struct request_queue *q = hctx->queue;
> + struct scsi_device *sdev = q->queuedata;
> + struct Scsi_Host *shost = sdev->host;
could we separately set hctx->driver_data = shost or similar for a
quicker lookup? I don't see hctx->driver_data set for SCSI currently.
Going through the scsi_device looks strange - I know that it is done in
scsi_commit_rqs.
> +
> + if (shost->hostt->mq_poll)
to avoid this check, could we reject if .mq_poll is not set and
HCTX_TYPE_POLL is?
> + return shost->hostt->mq_poll(shost, hctx->queue_num);
> +
> + return 0;
> +}
> +
> static int scsi_map_queues(struct blk_mq_tag_set *set)
> {
> struct Scsi_Host *shost = container_of(set, struct Scsi_Host, tag_set);
> @@ -1833,6 +1846,7 @@ static const struct blk_mq_ops scsi_mq_ops_no_commit = {
> .cleanup_rq = scsi_cleanup_rq,
> .busy = scsi_mq_lld_busy,
> .map_queues = scsi_map_queues,
> + .poll = scsi_mq_poll,
> };
>
>
> @@ -1861,6 +1875,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
> .cleanup_rq = scsi_cleanup_rq,
> .busy = scsi_mq_lld_busy,
> .map_queues = scsi_map_queues,
> + .poll = scsi_mq_poll,
> };
>
> struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
> @@ -1893,6 +1908,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
> else
> tag_set->ops = &scsi_mq_ops_no_commit;
> tag_set->nr_hw_queues = shost->nr_hw_queues ? : 1;
> + tag_set->nr_maps = shost->nr_maps ? : 1;
> tag_set->queue_depth = shost->can_queue;
> tag_set->cmd_size = cmd_size;
> tag_set->numa_node = NUMA_NO_NODE;
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index e76bac4d14c5..5844374a85b1 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -9,6 +9,7 @@
> #include <linux/types.h>
> #include <linux/timer.h>
> #include <linux/scatterlist.h>
> +#include <scsi/scsi_host.h>
can we maintain alphabetic ordering?
> #include <scsi/scsi_device.h>
> #include <scsi/scsi_request.h>
>
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 701f178b20ae..905ee6b00c55 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -270,6 +270,16 @@ struct scsi_host_template {
> */
> int (* map_queues)(struct Scsi_Host *shost);
>
> + /*
> + * SCSI interface of blk_poll - poll for IO completions.
> + * Possible interface only if scsi LLD expose multiple h/w queues.
> + *
> + * Return values: Number of completed entries found.
/s/values/value/
> + *
> + * Status: OPTIONAL
> + */
> + int (* mq_poll)(struct Scsi_Host *shost, unsigned int queue_num);
> +
> /*
> * Check if scatterlists need to be padded for DMA draining.
> *
> @@ -610,6 +620,7 @@ struct Scsi_Host {
> * the total queue depth is can_queue.
> */
> unsigned nr_hw_queues;
> + unsigned nr_maps; > unsigned active_mode:2;
> unsigned unchecked_isa_dma:1;
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer
2020-11-13 11:51 ` John Garry
@ 2020-11-30 7:41 ` Kashyap Desai
2020-12-01 9:49 ` John Garry
0 siblings, 1 reply; 5+ messages in thread
From: Kashyap Desai @ 2020-11-30 7:41 UTC (permalink / raw)
To: John Garry, linux-scsi; +Cc: Sumit Saxena, Chandrakanth Patil, linux-block
[-- Attachment #1: Type: text/plain, Size: 3929 bytes --]
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index
> > 72b12102f777..5a3c383a2bb3 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -1766,6 +1766,19 @@ static void scsi_mq_exit_request(struct
> blk_mq_tag_set *set, struct request *rq,
> > cmd->sense_buffer);
> > }
> >
> > +
> > +static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx) {
> > + struct request_queue *q = hctx->queue;
> > + struct scsi_device *sdev = q->queuedata;
> > + struct Scsi_Host *shost = sdev->host;
>
> could we separately set hctx->driver_data = shost or similar for a quicker
> lookup? I don't see hctx->driver_data set for SCSI currently.
> Going through the scsi_device looks strange - I know that it is done in
> scsi_commit_rqs.
John - I have included your comments. Below is add-on patch which handles
all your comment except one.
Below is just compiled (not tested patch). Please let me know if you like to
handle "scsi_init_hctx" in this patch or shall we do it as a separate patch
(out of this patch series.) ?
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1769,9 +1769,7 @@ static void scsi_mq_exit_request(struct blk_mq_tag_set
*set, struct request *rq,
static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx)
{
- struct request_queue *q = hctx->queue;
- struct scsi_device *sdev = q->queuedata;
- struct Scsi_Host *shost = sdev->host;
+ struct Scsi_Host *shost = hctx->driver_data;
if (shost->hostt->mq_poll)
return shost->hostt->mq_poll(shost, hctx->queue_num);
@@ -1779,6 +1777,14 @@ static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx)
return 0;
}
+static int scsi_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
+ unsigned int hctx_idx)
+{
+ struct Scsi_Host *shost = data;
+ hctx->driver_data = shost;
+ return 0;
+}
+
static int scsi_map_queues(struct blk_mq_tag_set *set)
{
struct Scsi_Host *shost = container_of(set, struct Scsi_Host,
tag_set);
@@ -1846,6 +1852,7 @@ static const struct blk_mq_ops scsi_mq_ops_no_commit =
{
.cleanup_rq = scsi_cleanup_rq,
.busy = scsi_mq_lld_busy,
.map_queues = scsi_map_queues,
+ .init_hctx = scsi_init_hctx,
.poll = scsi_mq_poll,
};
@@ -1875,6 +1882,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
.cleanup_rq = scsi_cleanup_rq,
.busy = scsi_mq_lld_busy,
.map_queues = scsi_map_queues,
+ .init_hctx = scsi_init_hctx,
.poll = scsi_mq_poll,
};
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 5844374a85b1..cc30df96f5f7 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -9,8 +9,8 @@
#include <linux/types.h>
#include <linux/timer.h>
#include <linux/scatterlist.h>
-#include <scsi/scsi_host.h>
#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
#include <scsi/scsi_request.h>
struct Scsi_Host;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 905ee6b00c55..a0cda0f66b84 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -274,7 +274,7 @@ struct scsi_host_template {
* SCSI interface of blk_poll - poll for IO completions.
* Possible interface only if scsi LLD expose multiple h/w queues.
*
- * Return values: Number of completed entries found.
+ * Return value: Number of completed entries found.
*
* Status: OPTIONAL
*/
>
> > +
> > + if (shost->hostt->mq_poll)
>
> to avoid this check, could we reject if .mq_poll is not set and
> HCTX_TYPE_POLL is?
Is this urgent or shall we improve later ? I am not able to figure out how
you want to manage this ? Can you explain little bit ?
Kashyap
>
> > + return shost->hostt->mq_poll(shost, hctx->queue_num);
> > +
> > + return 0;
> > +}
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4169 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer
2020-11-30 7:41 ` Kashyap Desai
@ 2020-12-01 9:49 ` John Garry
0 siblings, 0 replies; 5+ messages in thread
From: John Garry @ 2020-12-01 9:49 UTC (permalink / raw)
To: Kashyap Desai, linux-scsi; +Cc: Sumit Saxena, Chandrakanth Patil, linux-block
On 30/11/2020 07:41, Kashyap Desai wrote:
>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index
>>> 72b12102f777..5a3c383a2bb3 100644
>>> --- a/drivers/scsi/scsi_lib.c
>>> +++ b/drivers/scsi/scsi_lib.c
>>> @@ -1766,6 +1766,19 @@ static void scsi_mq_exit_request(struct
>> blk_mq_tag_set *set, struct request *rq,
>>> cmd->sense_buffer);
>>> }
>>>
>>> +
>>> +static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx) {
>>> + struct request_queue *q = hctx->queue;
>>> + struct scsi_device *sdev = q->queuedata;
>>> + struct Scsi_Host *shost = sdev->host;
>> could we separately set hctx->driver_data = shost or similar for a quicker
>> lookup? I don't see hctx->driver_data set for SCSI currently.
>> Going through the scsi_device looks strange - I know that it is done in
>> scsi_commit_rqs.
> John - I have included your comments. Below is add-on patch which handles
> all your comment except one.
> Below is just compiled (not tested patch). Please let me know if you like to
> handle "scsi_init_hctx" in this patch or shall we do it as a separate patch
> (out of this patch series.) ?
It might be better as a separate patch if you also change
scsi_commit_rqs() to use hctx->driver_data, which you are currently not
doing (or showing here).
>
>
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1769,9 +1769,7 @@ static void scsi_mq_exit_request(struct blk_mq_tag_set
> *set, struct request *rq,
>
> static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx)
> {
> - struct request_queue *q = hctx->queue;
> - struct scsi_device *sdev = q->queuedata;
> - struct Scsi_Host *shost = sdev->host;
> + struct Scsi_Host *shost = hctx->driver_data;
>
> if (shost->hostt->mq_poll)
> return shost->hostt->mq_poll(shost, hctx->queue_num);
> @@ -1779,6 +1777,14 @@ static int scsi_mq_poll(struct blk_mq_hw_ctx *hctx)
> return 0;
> }
>
> +static int scsi_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
> + unsigned int hctx_idx)
> +{
> + struct Scsi_Host *shost = data;
> + hctx->driver_data = shost;
> + return 0;
> +}
> +
> static int scsi_map_queues(struct blk_mq_tag_set *set)
> {
> struct Scsi_Host *shost = container_of(set, struct Scsi_Host,
> tag_set);
> @@ -1846,6 +1852,7 @@ static const struct blk_mq_ops scsi_mq_ops_no_commit =
> {
> .cleanup_rq = scsi_cleanup_rq,
> .busy = scsi_mq_lld_busy,
> .map_queues = scsi_map_queues,
> + .init_hctx = scsi_init_hctx,
> .poll = scsi_mq_poll,
> };
>
> @@ -1875,6 +1882,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
> .cleanup_rq = scsi_cleanup_rq,
> .busy = scsi_mq_lld_busy,
> .map_queues = scsi_map_queues,
> + .init_hctx = scsi_init_hctx,
> .poll = scsi_mq_poll,
> };
>
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 5844374a85b1..cc30df96f5f7 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -9,8 +9,8 @@
> #include <linux/types.h>
> #include <linux/timer.h>
> #include <linux/scatterlist.h>
> -#include <scsi/scsi_host.h>
> #include <scsi/scsi_device.h>
> +#include <scsi/scsi_host.h>
> #include <scsi/scsi_request.h>
>
> struct Scsi_Host;
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 905ee6b00c55..a0cda0f66b84 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -274,7 +274,7 @@ struct scsi_host_template {
> * SCSI interface of blk_poll - poll for IO completions.
> * Possible interface only if scsi LLD expose multiple h/w queues.
> *
> - * Return values: Number of completed entries found.
> + * Return value: Number of completed entries found.
> *
> * Status: OPTIONAL
> */
>
>>> +
>>> + if (shost->hostt->mq_poll)
>> to avoid this check, could we reject if .mq_poll is not set and
>> HCTX_TYPE_POLL is?
> Is this urgent or shall we improve later ? I am not able to figure out how
> you want to manage this ? Can you explain little bit ?
I don't think that it will make much overhead difference, so ok to omit
for now if more trouble than it's worth to implement.
Thanks,
John
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer
2020-10-15 13:36 [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer Kashyap Desai
2020-11-13 11:51 ` John Garry
@ 2020-11-20 7:16 ` Hannes Reinecke
1 sibling, 0 replies; 5+ messages in thread
From: Hannes Reinecke @ 2020-11-20 7:16 UTC (permalink / raw)
To: Kashyap Desai, linux-scsi; +Cc: sumit.saxena, chandrakanth.patil, linux-block
On 10/15/20 3:36 PM, Kashyap Desai wrote:
> io_uring with IOPOLL is not currently supported in scsi mid layer.
> Outside of that everything else should work and no extra support in the driver is needed.
> Currently io_uring with IOPOLL support is only available in block layer.
> This patch is to extend support of mq_poll in scsi layer.
>
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Cc: sumit.saxena@broadcom.com
> Cc: chandrakanth.patil@broadcom.com
> Cc: linux-block@vger.kernel.org
>
> ---
> drivers/scsi/scsi_lib.c | 16 ++++++++++++++++
> include/scsi/scsi_cmnd.h | 1 +
> include/scsi/scsi_host.h | 11 +++++++++++
> 3 files changed, 28 insertions(+)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-01 9:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 13:36 [PATCH v1 1/3] add io_uring with IOPOLL support in scsi layer Kashyap Desai
2020-11-13 11:51 ` John Garry
2020-11-30 7:41 ` Kashyap Desai
2020-12-01 9:49 ` John Garry
2020-11-20 7:16 ` Hannes Reinecke
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).