On 08/25/2013 08:01 AM, Or Gerlitz wrote:
>> Is the max_cmdsn use correct? If we set it under the back_log but read
>> it under the frwd_lock will that work? We really just need it to be
>> updated on the send side when it is updated on the recv side.
>
> session->max_cmdsn is not updated on the send side, it is only updated
> on the recv
> side and it tells us to which value session->cmdsn can grow to. It is
> read under the
> forward lock indeed, but this isn't actually needed. Since the
> upcall/response flow
> is the only writer of the value (which for itself is protected with the
> back lock),
> and the reader can look without locking, in worst case the sender will
> see a closed
> window and will re-try again later.
I know you guys sent a new version of the patch, but I am commenting on
just the specific issue below because we started the discussion here.
This chunk in __iscsi_update_cmdsn:
if (max_cmdsn != session->max_cmdsn &&
!iscsi_sna_lt(max_cmdsn, session->max_cmdsn)) {
session->max_cmdsn = max_cmdsn;
/*
* if the window closed with IO queued, then kick the
* xmit thread
*/
if (!list_empty(&session->leadconn->cmdqueue) ||
!list_empty(&session->leadconn->mgmtqueue))
iscsi_conn_queue_work(session->leadconn);
looks not needed. We never will have a closed window and something on
one of those lists. If we did we could miss a case we need to wake up
the xmit thread because they are using different locks.
On the next sending of your patchset could you send a patch to delete
that chunk above or delete that chunk in the main patch.
Will send other comments in other thread where the new version of the
patch is on.
--
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.
