On Fri, 12 Oct 2007 12:34:58 -0400
Pete Wyckoff <[EMAIL PROTECTED]> wrote:
> [EMAIL PROTECTED] 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.
Thanks.
> > 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.
Yeah, I think so.
> 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.
I did a clean revert just because the original code have been tested
for a long time in my lab.
As you said, I think that we don't need to set EPOLLIN|EPOLLOUT there
so I'm happy to change it later on (after lots of testings).
_______________________________________________
Stgt-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/stgt-devel
