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

Best regards,

S.P.

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

Reply via email to