Hannes Reinecke wrote:
> Hi Mike,
> I got fed up with the large number of rejects I've seen from queuecommand
> due to the CmdSN window closing on us.

Does this affect performance or is the major problem debug printks?

> So I put together a patch for dynamically adjusting shost->can_queue
> depending on the number of rejects.
> Purely as a proof-on-concept, but works out quite nicely.
> I did on purpose _not_ calling the change_queue_depth callback
> as this would induce quite some overhead here with very
> little gain, as shost->can_queue seems to be more appropriate
> here as we're deducing the number of commands for the host/target,
> not the LUN.
> Interesting though, both NetApp and MSA level out at around
> 64 commands. Maybe we should consider changing the default ...

I think a netapp we have here can go to 256 or 128.

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 0356e3d..a4d6eda 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1592,7 +1743,7 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, void 
(*done)(struct scsi_cmnd *))
        struct iscsi_cls_session *cls_session;
        struct Scsi_Host *host;
        struct iscsi_host *ihost;
-       int reason = 0;
+       int reason = 0, adjust_qdepth = 0;
        struct iscsi_session *session;
        struct iscsi_conn *conn;
        struct iscsi_task *task = NULL;
@@ -1684,27 +1835,53 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, 
void (*done)(struct scsi_cmnd *))

+       if (session->reject_count < 20) {
+               adjust_qdepth = 1;
+               session->reject_count = 100;
+       } else {
+               session->reject_count--;
+       }
+       if (adjust_qdepth == 1) {
+               host->can_queue++;

I think this should be starget->can_queue, because bnx2i and cxgb3i can 
have multiple sessions per host.

You also want to add some code so you do not go over 
starget->can_queue, becuase we run out of preallocated cmds, and you 
will hit the same reject printks. Also we do not want to go over this 
value in some cases, because users might have set this value lower than 
the target cmdns window because the network could be slow and the target 
just tells us about what it can handle in a normal setup or wrt its 

We were discussing this on the fcoe list, and I mentioned it here on the 
scsi cmd timer patch. What I thought we might want to do is instead of 
doing it based on cmdsn window rejects which can sometimes be static, we 
can take into account how often commands are timing while they are still 
pending. For example if a cmd is still pending and the scsi command 
timer fires, then can_queue is probably too high at that time.

But we probably also need to check the cmdsn window rejects, because I 
think that can also change on some targets. So it can be separate 
patches, and I am not against what you are trying to do.

We could also then add some code to try and dynamically increase it like 
Vasu is doing for the device queue_depth. This can_queue rampup would be 
for the target port resources/limitations instead of Vasu's device one.

+               iscsi_session_printk(KERN_INFO, session,
+                                    "adjusted queue depth to %u\n",
+                                    host->can_queue);

For a real patch you probably want to make this a debug printk. We could 
end up with a lot of printks.

+       }
        return 0;

        sc->scsi_done = NULL;
        iscsi_complete_task(task, ISCSI_TASK_COMPLETED);
-       spin_unlock(&session->lock);
        ISCSI_DBG_SESSION(session, "cmd 0x%x rejected (%d)\n",
                          sc->cmnd[0], reason);
+       if (session->reject_count > 200) {
+               adjust_qdepth = -1;
+               session->reject_count = 100;
+       } else {
+               session->reject_count++;
+       }
+       spin_unlock(&session->lock);
+       if (adjust_qdepth == -1) {
+               host->can_queue--;
+               iscsi_session_printk(KERN_INFO, session,
+                                    "adjusted queue depth to %u\n",
+                                    host->can_queue);
+       }

        sc->scsi_done = NULL;
        iscsi_complete_task(task, ISCSI_TASK_COMPLETED);
-       spin_unlock(&session->lock);
+       session->fault_count++;
        ISCSI_DBG_SESSION(session, "iscsi: cmd 0x%x is not queued (%d)\n",
                          sc->cmnd[0], reason);
+       spin_unlock(&session->lock);
        if (!scsi_bidi_cmnd(sc))
                scsi_set_resid(sc, scsi_bufflen(sc));
        else {
@@ -2793,6 +2982,7 @@ int iscsi_conn_start(struct iscsi_cls_conn *cls_conn)

        conn->last_recv = jiffies;
        conn->last_ping = jiffies;
+       session->reject_count = 100;
        if (conn->recv_timeout && conn->ping_timeout)
                          jiffies + (conn->recv_timeout * HZ));
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 61afeb5..b006a32 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -298,6 +303,7 @@ struct iscsi_session {
                                                 * - r2tpool               */
        int                     state;          /* session state           */
        int                     age;            /* counts session re-opens */
+       unsigned long           reject_count;   /* counts cmd rejects */

        int                     scsi_cmds_max;  /* max scsi commands */
        int                     cmds_max;       /* size of cmds array */

You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
For more options, visit this group at http://groups.google.com/group/open-iscsi

Reply via email to