On 10/23/2013 11:07 AM, Mike Christie wrote: > On 10/23/2013 06:16 AM, Shlomo Pongratz wrote: >>> -----Original Message----- >>> From: Mike Christie [mailto:[email protected]] >>> Sent: Monday, October 21, 2013 7:06 PM >>> To: Or Gerlitz >>> Cc: [email protected]; Roi Dayan; Shlomo Pongratz >>> Subject: Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in >>> fast path >>> >>> On 09/17/2013 05:43 AM, Or Gerlitz wrote: >>>> diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h index >>>> 6ac9e17..783f031 100644 >>>> --- a/include/scsi/libiscsi.h >>>> +++ b/include/scsi/libiscsi.h >>>> @@ -326,12 +326,15 @@ struct iscsi_session { >>>> struct iscsi_transport *tt; >>>> struct Scsi_Host *host; >>>> struct iscsi_conn *leadconn; /* leading connection */ >>>> - spinlock_t lock; /* protects session state, * >>>> - * sequence numbers, * >>>> + spinlock_t frwd_lock; /* protects session state, * >>>> + * cmdsn, queued_cmdsn * >>>> * session resources: * >>>> - * - cmdpool, * >>>> + * - cmdpool kfifo_out , * >>>> * - mgmtpool, * >>>> * - r2tpool */ >>>> + spinlock_t back_lock; /* protects cmdsn_exp * >>>> + * cmdsn_max, * >>>> + * cmdpool kfifo_in */ >>> >>> 1. fix up the comment names like above. cmdsn_exp should be exp_cmdsn. >>> Also review that we even need a lock for max/exp cmdsns. >>> >>> 2. There are references to the session->lock in the code still. Fix those >>> up. >>> >>> 3. Add some comments above about locking hierarchy. For example we grab >>> the frwd_lock then call fail_scsi_task which grabs the back_lock when >>> calling >>> fail_scsi_task*. >>> >>> 4. Handle review comment from other mail about removing useless chunk of >>> code. >>> >>> >>> That's all looks good. Thanks for working on it and being patient with >>> reviews >>> and also staying on top of getting me to review it. >>> >>> The iscsi tcp r2t changes are ok too. I want to fix them a different way, >>> once >>> we actually implement multiple r2t support for that driver. >>> Today all that code does not seem useful for just single r2t support. >> >> Hi Mike, >> >> Regarding the reference to "session->lock" I've found such references only >> in "drivers/scsi/scsi_transport_iscsi.c". >> Please note that the so called "session" there, is of type >> "iscsi_cls_session" and not of type "iscsi_session". >> Since it is not part of the data path I don't think we should bother with >> this lock. >> > > Check for "session lock" too: > > iscsi_free_task > iscsi_complete_scsi_task > fail_scsi_task > iscsi_itt_to_task > __iscsi_complete_pdu > iscsi_itt_to_ctask > iscsi_requeue_task > fail_scsi_tasks > iscsi_suspend_queue
One more iscsi_tcp_cleanup_task -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/groups/opt_out.
