Hi Alan,
On Mon, 29 Sep 2008 17:12:28 -0400 (EDT), Alan Stern wrote:
> The patch also changes the "failure" path; now instead of calling
> scsi_end_request() it directly calls end_dequeued_request() followed
> by scsi_next_command().
...
> @@ -908,7 +907,6 @@ void scsi_io_completion(struct scsi_cmnd
> */
> if (scsi_end_request(cmd, error, good_bytes, result == 0) == NULL)
> return;
> - this_count = blk_rq_bytes(req);
>
> if (sense_valid && !sense_deferred) {
> switch (sshdr.sense_key) {
...
> @@ -1002,14 +1000,10 @@ void scsi_io_completion(struct scsi_cmnd
> if (driver_byte(result) & DRIVER_SENSE)
> scsi_print_sense("", cmd);
> }
> - goto fail;
> }
> -
> - retry2:
> - scsi_end_request(cmd, -EIO, this_count, 1);
> - return;
> fail:
> - scsi_end_request(cmd, -EIO, this_count, 0);
> + end_dequeued_request(req, -EIO);
> + scsi_next_command(cmd);
> return;
> requeue:
> scsi_requeue_command(q, cmd);
I think you should use blk_end_request(req, -EIO, blk_rq_bytes(rq))
here instead of end_dequeued_request(req, -EIO).
end_dequeued_request() needs to be called with queue lock held,
but I can't see any queue lock in your patches.
Also, if you add the queue lock and still use end_dequeued_request(),
__end_that_request_first(), which completes BIOs in the request,
is called with the queue lock held, though it doesn't require queue
lock actually. So that might cause some performance regressions.
Thanks,
Kiyoshi Ueda
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
