On 10-06-02 10:07 AM, James Bottomley wrote:
On Wed, 2010-06-02 at 16:33 +0900, FUJITA Tomonori wrote:
This fixes scsi_get_lba() helper function for PC commands.
Only the block layer is supposed to touch rq->__sector. We could
create a new accessor for it. But it seems overdoing a bit? Jens?
=
From: FUJITA Tomonori<fujita.tomonori@xxxxxxxxxxxxx>
Subject: [PATCH] scsi: fix scsi_get_lba helper function for pc command
scsi_get_lba() can be used to get the lba info from
scsi_cmnd. scsi_get_lba() gets the lba info from rq->__sector so
scsi_get_lba() returns a bogus value for PC commands (we don't use
rq->__sector for PC commands).
This patch sets rq->__sector in scsi_setup_blk_pc_cmnd() so that
scsi_get_lba() works with PC commands.
scsi_get_data_transfer_info() is taken from
scsi_debug. scsi_get_data_transfer_info() looks useful for some
(scsi_debug, scsi_trace, libata, etc). So I export it. I'll convert
them to use scsi_get_data_transfer_info().
Signed-off-by: FUJITA Tomonori<fujita.tomonori@xxxxxxxxxxxxx>
---
drivers/scsi/scsi_lib.c | 54 +++++++++++++++++++++++++++++++++++++++++++++-
include/scsi/scsi_cmnd.h | 4 +++
2 files changed, 57 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1646fe7..b9b7f40 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -19,6 +19,7 @@
#include<linux/delay.h>
#include<linux/hardirq.h>
#include<linux/scatterlist.h>
+#include<asm/unaligned.h>
#include<scsi/scsi.h>
#include<scsi/scsi_cmnd.h>
@@ -69,6 +70,52 @@ struct kmem_cache *scsi_sdb_cache;
static void scsi_run_queue(struct request_queue *q);
+int scsi_get_data_transfer_info(unsigned char *cmd, unsigned long long *lba,
+ unsigned int *num, unsigned int *ei_lba)
+{
+ int ret = 0;
+
+ switch (*cmd) {
+ case VARIABLE_LENGTH_CMD:
+ *lba = get_unaligned_be64(&cmd[12]);
+ *num = get_unaligned_be32(&cmd[28]);
+ *ei_lba = get_unaligned_be32(&cmd[20]);
+ break;
You can't do this ... unless you know the format of the command. For
instance, if you knew this was a 32 byte CDB, then SPC does define where
the LBA is.
+ case WRITE_16:
+ case READ_16:
+ case VERIFY_16:
+ case WRITE_SAME_16:
+ *lba = get_unaligned_be64(&cmd[2]);
+ *num = get_unaligned_be32(&cmd[10]);
+ break;
+ case WRITE_12:
+ case READ_12:
+ case VERIFY_12:
+ *lba = get_unaligned_be32(&cmd[2]);
+ *num = get_unaligned_be32(&cmd[6]);
+ break;
+ case WRITE_10:
+ case READ_10:
+ case XDWRITEREAD_10:
+ case VERIFY:
+ case WRITE_SAME:
+ *lba = get_unaligned_be32(&cmd[2]);
+ *num = get_unaligned_be16(&cmd[7]);
+ break;
+ case WRITE_6:
+ case READ_6:
+ *lba = (u32)cmd[3] | (u32)cmd[2]<< 8 |
+ (u32)(cmd[1]& 0x1f)<< 16;
+ *num = (0 == cmd[4]) ? 256 : cmd[4];
+ break;
I really wouldn't do it like this because there are going to be
unexpected gaps. What about using the CDB groups instead? Each CDB
group has a well defined location for the LBA. The only problem is the
16 byte commands group because they have two forms: ordinary and long
LBA.
The 16 byte SCSI command might be ATA PASS THROUGH (16)!
Is it so important to have scsi_get_lba() for "PC" commands?
Why not just return 0 or MAX_UINT and be done with it.
A "PC" command might be lots of things. For example:
- a tunnelled ATA command
- a non SCSI protocol
- a SCSI vendor specific or obsolete command
(e.g. some SCSI PRINTER commands share opcodes with
READ and WRITE)
- from a lesser used command set (e.g. OSD or SSC)
A LLD may well be able to narrow the field if it has a
reason to decode the command to find a LBA or other info.
Doug Gilbert
--
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