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 *))
}
session->queued_cmdsn++;
+ if (session->reject_count < 20) {
+ adjust_qdepth = 1;
+ session->reject_count = 100;
+ } else {
+ session->reject_count--;
+ }
spin_unlock(&session->lock);
spin_lock(host->host_lock);
+ 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
resources.
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;
prepd_reject:
sc->scsi_done = NULL;
iscsi_complete_task(task, ISCSI_TASK_COMPLETED);
reject:
- 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);
spin_lock(host->host_lock);
+ if (adjust_qdepth == -1) {
+ host->can_queue--;
+ iscsi_session_printk(KERN_INFO, session,
+ "adjusted queue depth to %u\n",
+ host->can_queue);
+ }
return SCSI_MLQUEUE_TARGET_BUSY;
prepd_fault:
sc->scsi_done = NULL;
iscsi_complete_task(task, ISCSI_TASK_COMPLETED);
fault:
- 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)
mod_timer(&conn->transport_timer,
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
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---
