[Stgt-devel] iSER

[Pete Wyckoff]

fujita.tomonori at lab.ntt.co.jp wrote on Fri, 12 Oct 2007 10:48 +0900:
> Can you please submit an updated patchset to the mailing list?
> 
> It's easier to review and comment on it. And there might be other
> people who are interested in them.

Will do.

> There is one logic change. The original iscsi_scsi_cmd_execute()
> always calls:
> 
> static int iscsi_scsi_cmd_execute(struct iscsi_task *task)
> {
> 	struct iscsi_connection *conn = task->conn;
> 	struct iscsi_cmd *req = (struct iscsi_cmd *) &task->req;
> 	uint8_t rw = req->flags & ISCSI_FLAG_CMD_WRITE;
> 	int err = 0;
> 
> 	if (rw && task->r2t_count) {
> 		if (!task->unsol_count)
> 			list_add_tail(&task->c_list, &task->conn->tx_clist);
> 		goto no_queuing;
> 	}
> 
> 	task->offset = 0;  /* for use as transmit pointer for data-ins */
> 	err = iscsi_target_cmd_queue(task);
> no_queuing:
> 	tgt_event_modify(conn->fd, EPOLLIN|EPOLLOUT);
> 
> 	return err;
> }
> 
> Do you think that we need this? I guess that we don't need it but it's
> a safer option.

Hrm.  We've decided to execute the task.  But maybe all the data
hasn't arrived yet.  If unsolicited data has all shown up, and we
still need to generate more R2Ts, put task on tx_clist and turn
on EPOLLOUT.  That's pretty straightforward.

But if all the write data is in, and we queue the task to run, there
is no way it will be ready to transmit again.  Until target calls
iscsi_scsi_cmd_done(), which it always does, both for sync and async
backing stores.

So the only way we could need this extra EPOLLOUT line is if there
were a bug elsewhere, in my opinion.  For a sync target, it will
just duplicate the call at the end of cmd_done().  For an async
target, it will force a trip through iscsi_task_tx_start() that
will turn it off as soon as it sees that tx_clist is empty.

I tried it with and without.  Didn't seem to matter.  Extra call
is very fast, sub-microsecond.  You can do a clean revert or take
the patch when I send it---your call.

		-- Pete