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.
