Mike Christie wrote:
> Hannes Reinecke wrote:
>> 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 
> 
> This is expected like I said in the other mail, because before
> iscsi_xmit_task is called the session->cmdsn is incremented by
> iscsi_prep_scsi_cmd_pdu when sending scsi cmd pdus. For data-outs and
> nops, then it will probably be higher too, but like I said in the other
> mail that is ok.
> 
> 
> iscsi_data_xmit->iscsi_prep_scsi_cmd_pdu
> 
> iscsi_prep_scsi_cmd_pdu()
> {
> hdr->cmdsn = session->cmdsn;
> session->cmdsn++;
> }
> 
> Then we call iscsi_xmit_task() and your check is spitting out and
> testing the new/updated session->cmdsn. The scsi cmd pdu that gets sent
> out is going to have a cmdsn that is less than or equal to maxcmdsn.
> 
> Try printing out the task->cmdsn for scsi cmd pdus.
> 
Fsck. You are correct.

Testing.

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
-~----------~----~----~----~------~----~------~--~---

Reply via email to