Boaz Harrosh wrote:
<snip>
>
> All these patches do not solve the root cause of the problem and until
> solved, (easily) they will keep popping up.
I will ask that the correctness of the sg driver's construction of
a scatter list also be considered here. Is it ever correct for bi_size
to wind up larger than the request size? Why is rounding needed in one
case (sg_build_indirect) but not the other (sg_link_reserve)?
> The root cause is that BLOCK_PC commands *can not* be retried in chunks.
> The midlayer has no way to know how to retry them. and any attempt to do so
> will result in an illegal scsi command.
>
> I'm really sorry for not seeing this before. I have seen all these emails
> and patches back and forth but they addressed such remote locations, that
> I have not paid attention to them.
>
> So lets try to touch base. BLOCK_PC command can only be completed in full
> always. In case of error or residual this should be properly reported and
> that is all the midlayer can do. The Initiator That knows what command was
> sent will then decide on a retry or not. In above case that will be user
> mode.
Those who truly understand this should comment. It makes sense to this
novice.
>
> Just to demonstrate what I mean a patch is attached. Just as an RFC, totally
> untested.
I can try this out and see what happens.
Mike
> ---
> git-diff --stat -p
> drivers/scsi/scsi_lib.c | 29 ++++++++++++++++-------------
> 1 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index ba21d97..e5d05c6 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -784,17 +784,24 @@ EXPORT_SYMBOL(scsi_release_buffers);
> * in req->data_len and req->next_rq->data_len. The upper-layer driver can
> * decide what to do with this information.
> */
> -void scsi_end_bidi_request(struct scsi_cmnd *cmd)
> +static void scsi_end_blk_request(struct scsi_cmnd *cmd)
> {
> struct request *req = cmd->request;
> unsigned int dlen = req->data_len;
> - unsigned int next_dlen = req->next_rq->data_len;
> + unsigned int next_dlen;
>
> - req->data_len = scsi_out(cmd)->resid;
> - req->next_rq->data_len = scsi_in(cmd)->resid;
> + if (blk_bidi_rq(req)) {
> + next_dlen = req->next_rq->data_len;
> + req->data_len = scsi_out(cmd)->resid;
> + req->next_rq->data_len = scsi_in(cmd)->resid;
> + } else {
> + next_dlen = 0;
> + req->data_len = scsi_get_resid(cmd);
> + }
>
> - /* The req and req->next_rq have not been completed */
> - BUG_ON(blk_end_bidi_request(req, 0, dlen, next_dlen));
> + if (blk_end_bidi_request(req, 0, dlen, next_dlen))
> + /* The req and req->next_rq have not been completed */
> + BUG_ON();
>
> scsi_release_buffers(cmd);
>
> @@ -866,15 +873,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
> req->sense_len = len;
> }
> }
> - if (scsi_bidi_cmnd(cmd)) {
> - /* will also release_buffers */
> - scsi_end_bidi_request(cmd);
> - return;
> - }
> - req->data_len = scsi_get_resid(cmd);
> + /* will also release_buffers */
> + scsi_end_blk_request(cmd);
> + return;
> }
>
> - BUG_ON(blk_bidi_rq(req)); /* bidi not support for !blk_pc_request yet */
> scsi_release_buffers(cmd);
>
> /*
>
>
>
>
>
>
>
> --
> 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
--
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
