Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 23/10/2013 19:34, Mike Christie wrote: I am not sure what you are saying. I think when I wrote code in the first comment I meant code and comments. I just want you to change the comments in the above functions so instead of having something like: /** * iscsi_free_task - free a task * @task: iscsi cmd task * * Must be called with session lock. It now says the new lock that needs to be grabbed. In the case for iscsi_free_task it seems either one can be taken depending on the context. Oh, got it -- Shlomo - Mike is referring to the documentation changes we need to do and say which lock need to be taken (fwd, back) when function X is called! -- 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 open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 10/23/2013 11:33 AM, Or Gerlitz wrote: > On 23/10/2013 19:29, Shlomo Pongratz wrote: >> O.K. I'll rebase to Jame's "misc" branch (used to work on his >> "for-next" branch). > > > > Mike, aren't we expected to rebase against for-next re code that goes to > the next kernel? anyway, the be2iscsi patches are on both branches.. so > either would be fine for-next is fine as it has the misc changes. -- 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 open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 10/23/2013 11:29 AM, Shlomo Pongratz wrote:
> On 10/23/2013 7:12 PM, Mike Christie wrote:
>> 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:micha...@cs.wisc.edu]
>>>>> Sent: Monday, October 21, 2013 7:06 PM
>>>>> To: Or Gerlitz
>>>>> Cc: open-iscsi@googlegroups.com; 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_tlock;/* protects session state, *
>>>>>> - * sequence numbers, *
>>>>>> +spinlock_tfrwd_lock;/* protects session state, *
>>>>>> + * cmdsn, queued_cmdsn *
>>>>>>* session resources: *
>>>>>> - * - cmdpool, *
>>>>>> + * - cmdpool kfifo_out , *
>>>>>>* - mgmtpool, *
>>>>>>* - r2tpool */
>>>>>> +spinlock_tback_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
>>
> Hi Mike,
>
> O.K. I'll rebase to Jame's "misc" branch (used to work on his "for-next"
> branch).
>
> All the "session->lock" were the session was of type "struct
> iscsi_session" were replaced by frwd and back lock.
I am not sure what you are saying. I think when I wrote code in the
first comment I meant code and comments. I just want you to change the
comments in the above functions so instead of having something like:
/**
* iscsi_free_task - free a task
* @task: iscsi cmd task
*
* Must be called with session lock.
It now says the new lock that needs to be grabbed. In the case for
iscsi_free_task it seems either one can be taken depending on the context.
--
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 23/10/2013 19:29, Shlomo Pongratz wrote: O.K. I'll rebase to Jame's "misc" branch (used to work on his "for-next" branch). Mike, aren't we expected to rebase against for-next re code that goes to the next kernel? anyway, the be2iscsi patches are on both branches.. so either would be fine -- 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 open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 23/10/2013 19:12, Mike Christie wrote:
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:micha...@cs.wisc.edu]
Sent: Monday, October 21, 2013 7:06 PM
To: Or Gerlitz
Cc: open-iscsi@googlegroups.com; 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
Mike, something is weird, the patch removes the session lock field and
after being applied the code is not only working but also gets to build
OK, so I don't get it how come there are places in the code that ref
it... I'm away from the office on LinuxCon so can't check it out to the
bit level myself now...
--
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 10/23/2013 7:12 PM, Mike Christie wrote:
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:micha...@cs.wisc.edu]
Sent: Monday, October 21, 2013 7:06 PM
To: Or Gerlitz
Cc: open-iscsi@googlegroups.com; 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
Hi Mike,
O.K. I'll rebase to Jame's "misc" branch (used to work on his "for-next"
branch).
All the "session->lock" were the session was of type "struct
iscsi_session" were replaced by frwd and back lock.
All "session->lock" one can see with grep are for session of type
"struct iscsi_cls_session".
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
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:micha...@cs.wisc.edu]
>>> Sent: Monday, October 21, 2013 7:06 PM
>>> To: Or Gerlitz
>>> Cc: open-iscsi@googlegroups.com; 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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 10/23/2013 06:16 AM, Shlomo Pongratz wrote:
>> -Original Message-
>> From: Mike Christie [mailto:micha...@cs.wisc.edu]
>> Sent: Monday, October 21, 2013 7:06 PM
>> To: Or Gerlitz
>> Cc: open-iscsi@googlegroups.com; 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
Also make sure you refresh James's scsi misc tree when you make the new
patchset. Some be2iscsi patches where merged to that driver that
conflict with you patches.
--
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
RE: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
> -Original Message-
> From: Mike Christie [mailto:micha...@cs.wisc.edu]
> Sent: Monday, October 21, 2013 7:06 PM
> To: Or Gerlitz
> Cc: open-iscsi@googlegroups.com; 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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
Re: [PATCH RFC V1 2/3] SCSI/libiscsi: Reduce locking contention in fast path
On 10/21/2013 7:06 PM, Mike Christie wrote:
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,
Will do, thank you.
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
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.
--
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/groups/opt_out.
