On 6/24/2014 9:54 AM, Mike Christie wrote:
On 06/11/2014 04:09 AM, Sagi Grimberg wrote:
In case protection information exists on the wire
scsi transports should include it in the transfer
byte count (even if protection information does not
exist in the host memory space). This helper will
compute the total transfer length from the scsi
command data length and protection attributes.
Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
---
include/scsi/scsi_cmnd.h | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index dd7c998..a100c6e 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -7,6 +7,7 @@
#include <linux/types.h>
#include <linux/timer.h>
#include <linux/scatterlist.h>
+#include <scsi/scsi_device.h>
struct Scsi_Host;
struct scsi_device;
@@ -306,4 +307,20 @@ static inline void set_driver_byte(struct scsi_cmnd *cmd, char status)
cmd->result = (cmd->result & 0x00ffffff) | (status << 24);
}
+static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
+{
+ unsigned int xfer_len = blk_rq_bytes(scmd->request);
+ unsigned int prot_op = scsi_get_prot_op(scmd);
+ unsigned int sector_size = scmd->device->sector_size;
+
+ switch (prot_op) {
+ case SCSI_PROT_NORMAL:
+ case SCSI_PROT_WRITE_STRIP:
+ case SCSI_PROT_READ_INSERT:
+ return xfer_len;
+ }
+
+ return xfer_len + (xfer_len >> ilog2(sector_size)) * 8;
+}
+
#endif /* _SCSI_SCSI_CMND_H */
I found the issue Christoph is hitting in the other thread.
The problem is WRITE_SAME requests are setup so that req->__data_len is
the value of the entire request when the setup is completed but during
the setup process it's value changes
So __data_len could be thousands of bytes but
scsi_out(scsi_cmnd)->length for this case was only returning 512 which
is the sector size. This is because sd_setup_-write_same_cmnd does:
rq->__data_len = sdp->sector_size;
....
scsi_setup_blk_pc_cmnd()
....
rq->__data_len = nr_bytes;
and scsi_setup_blk_pc_cmnd does scsi_init_io() -> scsi_init_sgtable()
and that does
sdb->length = blk_rq_bytes(req);
and at this time because before we called scsi_setup_blk_pc_cmnd we set
the __data_len to sector size, the sdb length is going to be only 512
but the final request->__data_len is the total size of the operation.
Hey Christoph, Mike &MKP,
So just got first look in this thread, didn't have time to reproduce it yet.
Mike's analysis makes sense to me, I think the below change should
resolve this one.
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index a100c6e..2afd9c2 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -309,7 +309,7 @@ static inline void set_driver_byte(struct scsi_cmnd
*cmd, char status)
static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
{
- unsigned int xfer_len = blk_rq_bytes(scmd->request);
+ unsigned int xfer_len = scsi_bufflen(scmd);
unsigned int prot_op = scsi_get_prot_op(scmd);
unsigned int sector_size = scmd->device->sector_size;
Moreover, since bidi and dif are adjacent, this will also needed:
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 3f46234..abf0c3e 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -386,12 +386,14 @@ static int iscsi_prep_scsi_cmd_pdu(struct
iscsi_task *task)
rc = iscsi_prep_bidi_ahs(task);
if (rc)
return rc;
+ transfer_length = scsi_in(sc)->length;
+ } else {
+ transfer_length = scsi_transfer_length(sc);
}
if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
task->protected = true;
- transfer_length = scsi_transfer_length(sc);
hdr->data_length = cpu_to_be32(transfer_length);
if (sc->sc_data_direction == DMA_TO_DEVICE) {
struct iscsi_r2t_info *r2t = &task->unsol_r2t;
Let me test and queue it up.
Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
