diff mbox

ISER blk-mq tags

Message ID e9e358d6-5962-7c07-df33-857082f9f821@grimberg.me (mailing list archive)
State RFC
Headers show

Commit Message

Sagi Grimberg June 19, 2018, 2:59 p.m. UTC
> Hi,

Hi Karan,

> I have run in to a problem where it seems like an ISER initiator cannot
> submit more than 128 commands to the target (per scsi host). I tried to
> debug this and found that when iscsi_host_add() is called,
> Scsi_Host>can_queue is unset. This causes iscsi_host_add() to use the
> default qdepth (128) when creating the scsi host's blk-mq in
> scsi_add_host(). After the call to iscsi_host_add() shost->can_queue is
> set to the correct value, however by then the blk-mq is already created
> with the wrong nuber of tags.
> 
> This causes the queue depth not to exceed 128.
> 
> Please advise if this is the expected behaviour. Have I missed
> something? It appears that other drivers that call scsi_add_host() set
> can_queue before the function call.

Thanks for reporting, you are correct, this can be easily fixed, does
the below patch fix your issue:
--
                 if (iser_conn->state != ISER_CONN_UP) {
@@ -660,6 +660,7 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
                 }
                 mutex_unlock(&iser_conn->state_mutex);
         } else {
+               shost->can_queue = min_t(u16, cmds_max, 
ISER_DEF_XMIT_CMDS_MAX);
                 max_cmds = ISER_DEF_XMIT_CMDS_MAX;
                 if (iscsi_host_add(shost, NULL))
                         goto free_host;
@@ -678,12 +679,6 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
                  iser_conn, shost->sg_tablesize,
                  shost->max_sectors);

-       if (cmds_max > max_cmds) {
-               iser_info("cmds_max changed from %u to %u\n",
-                         cmds_max, max_cmds);
-               cmds_max = max_cmds;
-       }
-
         cls_session = iscsi_session_setup(&iscsi_iser_transport, shost,
                                           cmds_max, 0,
                                           sizeof(struct iscsi_iser_task),
@@ -692,7 +687,6 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
                 goto remove_host;
         session = cls_session->dd_data;

-       shost->can_queue = session->scsi_cmds_max;
         return cls_session;

  remove_host:
--
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Karandeep Chahal June 20, 2018, 4:15 p.m. UTC | #1
Hi Sagi,

Thanks. I will try to test the patch sometime this week. Sorry about the delay.

Thanks
-Karan


On Tue, 2018-06-19 at 17:59 +0300, Sagi Grimberg wrote:
> > Hi,
> 
> Hi Karan,
> 
> > I have run in to a problem where it seems like an ISER initiator cannot
> > submit more than 128 commands to the target (per scsi host). I tried to
> > debug this and found that when iscsi_host_add() is called,
> > Scsi_Host>can_queue is unset. This causes iscsi_host_add() to use the
> > default qdepth (128) when creating the scsi host's blk-mq in
> > scsi_add_host(). After the call to iscsi_host_add() shost->can_queue is
> > set to the correct value, however by then the blk-mq is already created
> > with the wrong nuber of tags.
> > 
> > This causes the queue depth not to exceed 128.
> > 
> > Please advise if this is the expected behaviour. Have I missed
> > something? It appears that other drivers that call scsi_add_host() set
> > can_queue before the function call.
> 
> Thanks for reporting, you are correct, this can be easily fixed, does
> the below patch fix your issue:
> --
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
> b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 0336643c2ed6..434ae88ff623 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -633,8 +633,8 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
>           */
>          if (ep) {
>                  iser_conn = ep->dd_data;
> -               max_cmds = iser_conn->max_cmds;
>                  shost->sg_tablesize = iser_conn->scsi_sg_tablesize;
> +               shost->can_queue = min_t(u16, cmds_max, 
> iser_conn->max_cmds);
> 
>                  mutex_lock(&iser_conn->state_mutex);
>                  if (iser_conn->state != ISER_CONN_UP) {
> @@ -660,6 +660,7 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
>                  }
>                  mutex_unlock(&iser_conn->state_mutex);
>          } else {
> +               shost->can_queue = min_t(u16, cmds_max, 
> ISER_DEF_XMIT_CMDS_MAX);
>                  max_cmds = ISER_DEF_XMIT_CMDS_MAX;
>                  if (iscsi_host_add(shost, NULL))
>                          goto free_host;
> @@ -678,12 +679,6 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
>                   iser_conn, shost->sg_tablesize,
>                   shost->max_sectors);
> 
> -       if (cmds_max > max_cmds) {
> -               iser_info("cmds_max changed from %u to %u\n",
> -                         cmds_max, max_cmds);
> -               cmds_max = max_cmds;
> -       }
> -
>          cls_session = iscsi_session_setup(&iscsi_iser_transport, shost,
>                                            cmds_max, 0,
>                                            sizeof(struct iscsi_iser_task),
> @@ -692,7 +687,6 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
>                  goto remove_host;
>          session = cls_session->dd_data;
> 
> -       shost->can_queue = session->scsi_cmds_max;
>          return cls_session;
> 
>   remove_host:
> --
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 0336643c2ed6..434ae88ff623 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -633,8 +633,8 @@  iscsi_iser_session_create(struct iscsi_endpoint *ep,
          */
         if (ep) {
                 iser_conn = ep->dd_data;
-               max_cmds = iser_conn->max_cmds;
                 shost->sg_tablesize = iser_conn->scsi_sg_tablesize;
+               shost->can_queue = min_t(u16, cmds_max, 
iser_conn->max_cmds);

                 mutex_lock(&iser_conn->state_mutex);