On Wed, Mar 12 2008 at 18:03 +0200, Michael Reed <mdr@xxxxxxx> wrote:
>
> 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.
>
>
Will not compile here is a cleaner one
---
From: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
Date: Wed, 12 Mar 2008 13:18:49 +0200
Subject: [PATCH] scsi: Always complete BLOCK_PC commands
Current code, in some IO combinations could wrongly retry
a BLOCK_PC command in chunks. This was never intended.
Fix it that BLOCK_PC will always complete atomically
Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
---
drivers/scsi/scsi_lib.c | 24 +++++++++++++-----------
1 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ba21d97..4b8b57c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -784,14 +784,20 @@ 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));
@@ -866,15 +872,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);
/*
--
1.5.3.3
--
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
