Mike Christie wrote:
> On 07/22/2009 12:00 PM, Mike Christie wrote:
>>> No, wrong. The check in queuecommand is by no means relevant
>>> to the actual window.
>>> We're checking the target window at the time queuecommand is run,
>>> but we're _generating_ the CmdSN only much later after we've
>>> dequeued the command.
>>
>> The check in queuecommand is for scsi cmd pdus and checks the
>> session->queued_cmdsn against the MaxCmdSn. The queued_cmdsn is just a
>> preallocation of CmdSn values. So if the scsi layer sends us X commands,
>> the initiator checks if the window has X spaces open. If it does, then
>> we add the task to the cmdqueue, and increase queued_cmdsn. This is
>> supposed to prevent the scsi layer from sending us too many commands and
>> them sitting in the driver cmdqueue and timing out.
>>
>> The cmdsn is then allocated when we put the cmd on the wire. So the
>
> Maybe this is causing some confusion. We do not increment the cmdsn
> value for immediate commands (how we send a tmf or nop) and for data-out
> pdus. We only increment it for scsi cmd pdus, so the only place I check
> for the window having space is in queuecommand. If you are worried about
> a nop or data out increasing the cmdsn value in one thread and a scsi
> cmd pdu increasing it in another, then it will not happen. We only this
> one path to worry about.
>
If it were so easy.
I have this patch:
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 716cc34..90905f2 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1212,6 +1212,21 @@ static int iscsi_xmit_task(struct iscsi_conn *conn)
struct iscsi_task *task = conn->task;
int rc;
+ /* Check for target CmdSN window */
+ if (conn->session->state == ISCSI_STATE_LOGGED_IN &&
+ iscsi_sna_lt(conn->session->max_cmdsn,
+ conn->session->cmdsn)) {
+ iscsi_conn_printk(KERN_INFO, conn,
+ "target window closed cmd %u max %u",
+ conn->session->cmdsn,
+ conn->session->max_cmdsn);
+
+#if 0
+ /* Window closed, wait for CmdSN update */
+ return -EPROTO;
+#endif
+ }
+
__iscsi_get_task(task);
spin_unlock_bh(&conn->session->lock);
rc = conn->session->tt->xmit_task(task);
against the current linux-2.6-iscsi tree.
When running my test harness (10 concurrent bonnie runs over 10
iscsi LUNs), I'm seeing these messages:
connection2:0: target window closed cmd 61225 max 61224
connection2:0: target window closed cmd 61258 max 61257
connection2:0: target window closed cmd 61291 max 61290
connection2:0: target window closed cmd 61324 max 61323
connection2:0: target window closed cmd 61357 max 61356
connection2:0: target window closed cmd 61413 max 61412
connection1:0: target window closed cmd 61321 max 61320
connection1:0: target window closed cmd 61361 max 61360
connection1:0: target window closed cmd 61401 max 61400
connection1:0: target window closed cmd 61449 max 61448
connection1:0: target window closed cmd 61482 max 61481
connection1:0: target window closed cmd 61531 max 61530
connection1:0: target window closed cmd 61579 max 61578
connection1:0: target window closed cmd 61626 max 61625
connection1:0: target window closed cmd 61674 max 61673
connection1:0: target window closed cmd 61717 max 61716
connection1:0: target window closed cmd 61754 max 61753
connection1:0: target window closed cmd 61802 max 61801
connection1:0: target window closed cmd 61837 max 61836
connection1:0: target window closed cmd 61874 max 61873
connection2:0: target window closed cmd 62205 max 62204
connection2:0: target window closed cmd 62326 max 62325
connection2:0: target window closed cmd 62390 max 62389
connection2:0: target window closed cmd 62486 max 62485
connection2:0: target window closed cmd 62522 max 62521
connection2:0: target window closed cmd 62686 max 62685
connection2:0: target window closed cmd 62813 max 62812
connection2:0: target window closed cmd 62881 max 62880
connection2:0: target window closed cmd 62914 max 62913
So there _is_ a race window. It might be that my
patch is not entirely correct (in the best of worlds
we would have a correct queued_cmdsn counter and this
wouldn't happen), but unless we've found that one
my patch fixes the issue.
I know it's slightly irritating to have found a race
without knowing where exactly it is, but I for one
feel happy enough to have the race window fixed.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
