[Stgt-devel] iSER

Pete Wyckoff pw
Thu Oct 11 20:22:53 CEST 2007


fujita.tomonori at lab.ntt.co.jp wrote on Thu, 11 Oct 2007 14:57 +0900:
> Pete, the iSER patchset is ready for re-submission?

I think so.  There are no major outstanding issues that I know about
now.  Let me know if you want to see another round of emails on
this.  Or we can just iterate on your questions as you view the git.

> BTW, can you elaborate on the following commit?
> 
> http://git.osc.edu/?p=tgt.git;a=commit;h=8d9eae7acd041fc10a7cfe560c1c280dcc290fa1
> 
> What type of commands hit this bug?

First, the original commit referred to in that log fixes an issue
where the target would spin with nothing to do.  A request comes in,
gets handed off to bs_sync, but the conn->fd was still EPOLLIN |
EPOLLOUT.  tgtd main thread would keep trying to send something,
looping.  But now, looking at that old tree, I can't convince myself
how this was ever a problem.  The only bad thing that happens
without this old patch, now, is one extra trip through
iscsi_task_tx_start() which always prints "no more data", but that
doesn't seem like a huge deal.  Maybe I'm missing something today,
or was missing something back then.  Or fixed some real bug in the
interim.

The new commit you ask about essentially reverts the original
commit.  The problem I was seeing is that with multiple commands
outstanding, turning off EPOLLOUT on conn->fd would prohibit any
task responses from being sent on the connection.  That epoll
setting is for the entire connection, not for the individual task
that is just beginning execution.  So you could have 10 tasks on
task->c_list, waiting to send responses back to the initiator.  Then
one new request is received and is submitted via
iscsi_scsi_cmd_execute().  This disables EPOLLOUT, and the waiting
10 tasks do not get sent until some other task happens to complete
in bs_sync and re-enables EPOLLOUT polling on the connection fd.

The net effect of the two patches is zero.  Just some minor code
path rewrite with no logic change.  Sorry for the churn.

		-- Pete



More information about the stgt mailing list