Re: [PATCH 2/4] block layer varlen-cdb
On Nov. 01, 2007, 20:40 +0200, Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Thu, Nov 01, 2007 at 08:05:06PM +0200, Boaz Harrosh wrote:
>> @@ -287,8 +287,13 @@ struct request {
>> /*
>> * when request is used as a packet command carrier
>> */
>> -unsigned int cmd_len;
>> +unsigned short cmd_len;
>> +unsigned short varlen_cdb_len; /* length of varlen_cdb buffer */
>> unsigned char cmd[BLK_MAX_CDB];
>> +unsigned char *varlen_cdb; /* an optional variable-length cdb.
>> + * points to a user buffer that must be
>> + * valid until end of request
>> + */
>>
>
> Try this instead:
>
> unsigned int cmd_len;
> - unsigned char cmd[BLK_MAX_CDB];
> + unsigned char _cmd[BLK_MAX_CDB];
> + unsigned char *cmd;
>
> Then initialise cmd to the address of _cmd. If you need to override it
> later (ie patch 3), you can.
>
I agree this is probably the cleanest implementation but when Boaz and I
initially discussed this approach he convinced me that LL block devices assume
that req->cmd_len <= BLK_MAX_CDB and it is unsafe at the moment to expose them
potentially larger commands.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] [v2] ibmvscsi: Prevent IO during partner login
The prior version of this patch introduced a problem for the error
handler routines. Calling ibmvscsi_eh_reset_device during a
initialization/login would result in the reset failing without need.
To avoid failing the eh_* functions while re-attaching to the server
adapter this will retry for a period of time while
ibmvscsi_send_srp_event
returns SCSI_MLQUEUE_HOST_BUSY.
By setting the request_limit in send_srp_login to 1 we allowed login
requests to be sent to the server adapter. If this was not an initial
login, but was a login after a disconnect with the server, other I/O
requests could attempt to be processed before the login occured.
To address this we can set the request_limit to 0 while doing the login
and add an exception where login requests, along with task management
events, are always passed to the server.
There is a case where the request_limit had already reached 0 would
result
in all events being sent rather than returning SCSI_MLQUEUE_HOST_BUSY;
this
has also been fixed by this patch.
Signed-off-by: Robert Jennings <[EMAIL PROTECTED]>
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 59 ---
1 file changed, 48 insertions(+), 11 deletions(-)
Index: b/drivers/scsi/ibmvscsi/ibmvscsi.c
===
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -556,7 +556,7 @@ static int ibmvscsi_send_srp_event(struc
unsigned long timeout)
{
u64 *crq_as_u64 = (u64 *) &evt_struct->crq;
- int request_status;
+ int request_status = 0;
int rc;
/* If we have exhausted our request limit, just fail this request,
@@ -574,6 +574,13 @@ static int ibmvscsi_send_srp_event(struc
if (request_status < -1)
goto send_error;
/* Otherwise, we may have run out of requests. */
+ /* If request limit was 0 when we started the adapter is in the
+* process of performing a login with the server adapter, or
+* we may have run out of requests.
+*/
+ else if (request_status == -1 &&
+evt_struct->iu.srp.login_req.opcode != SRP_LOGIN_REQ)
+ goto send_busy;
/* Abort and reset calls should make it through.
* Nothing except abort and reset should use the last two
* slots unless we had two or less to begin with.
@@ -633,7 +640,8 @@ static int ibmvscsi_send_srp_event(struc
unmap_cmd_data(&evt_struct->iu.srp.cmd, evt_struct, hostdata->dev);
free_event_struct(&hostdata->pool, evt_struct);
- atomic_inc(&hostdata->request_limit);
+ if (request_status != -1)
+ atomic_inc(&hostdata->request_limit);
return SCSI_MLQUEUE_HOST_BUSY;
send_error:
@@ -927,10 +935,11 @@ static int send_srp_login(struct ibmvscs
login->req_buf_fmt = SRP_BUF_FORMAT_DIRECT | SRP_BUF_FORMAT_INDIRECT;
spin_lock_irqsave(hostdata->host->host_lock, flags);
- /* Start out with a request limit of 1, since this is negotiated in
-* the login request we are just sending
+ /* Start out with a request limit of 0, since this is negotiated in
+* the login request we are just sending and login requests always
+* get sent by the driver regardless of request_limit.
*/
- atomic_set(&hostdata->request_limit, 1);
+ atomic_set(&hostdata->request_limit, 0);
rc = ibmvscsi_send_srp_event(evt_struct, hostdata, init_timeout * 2);
spin_unlock_irqrestore(hostdata->host->host_lock, flags);
@@ -967,6 +976,7 @@ static int ibmvscsi_eh_abort_handler(str
int rsp_rc;
unsigned long flags;
u16 lun = lun_from_dev(cmd->device);
+ unsigned long wait_switch = 0;
/* First, find this command in our sent list so we can figure
* out the correct tag
@@ -1010,15 +1020,30 @@ static int ibmvscsi_eh_abort_handler(str
tsk_mgmt->lun, tsk_mgmt->task_tag);
evt->sync_srp = &srp_rsp;
- init_completion(&evt->comp);
- rsp_rc = ibmvscsi_send_srp_event(evt, hostdata, init_timeout * 2);
- spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+
+ wait_switch = jiffies + (init_timeout * HZ);
+ do {
+ init_completion(&evt->comp);
+ rsp_rc = ibmvscsi_send_srp_event(evt, hostdata, init_timeout *
2);
+
+ if (rsp_rc != SCSI_MLQUEUE_HOST_BUSY)
+ break;
+
+ spin_unlock_irqrestore(hostdata->host->host_lock, flags);
+ msleep(10);
+ spin_lock_irqsave(hostdata->host->host_lock, flags);
+ } while (time_before(jiffies, wait_switch));
+
if (rsp_rc != 0) {
+ free_event_struct(&found_evt->hos
Re: [PATCH 3/3] debloat aic7xxx and aic79xx drivers
On Thu, Nov 01, 2007 at 03:23:28PM -0700, Randy Dunlap wrote: > when attempting to apply to scsi-misc or linus-git or ... ? scsi-misc -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] debloat aic7xxx and aic79xx drivers
On Thu, 1 Nov 2007 16:16:04 -0600 Matthew Wilcox wrote: > On Sun, Oct 14, 2007 at 04:02:15PM +0100, Denys Vlasenko wrote: > > Adds more consts > > error: patch failed: drivers/scsi/aic7xxx/aic79xx.h:1328 > error: drivers/scsi/aic7xxx/aic79xx.h: patch does not apply > error: patch failed: drivers/scsi/aic7xxx/aic7xxx.h:1145 > error: drivers/scsi/aic7xxx/aic7xxx.h: patch does not apply when attempting to apply to scsi-misc or linus-git or ... ? Thanks, --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] debloat aic7xxx and aic79xx drivers
On Sun, Oct 14, 2007 at 04:02:15PM +0100, Denys Vlasenko wrote: > Adds more consts error: patch failed: drivers/scsi/aic7xxx/aic79xx.h:1328 error: drivers/scsi/aic7xxx/aic79xx.h: patch does not apply error: patch failed: drivers/scsi/aic7xxx/aic7xxx.h:1145 error: drivers/scsi/aic7xxx/aic7xxx.h: patch does not apply -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] debloat aic7xxx and aic79xx drivers
On Sun, Oct 14, 2007 at 04:00:10PM +0100, Denys Vlasenko wrote:
> Deinlines and moves big functions from .h to .c files.
> Adds prototypes for ahc_lookup_scb and ahd_lookup_scb to .h files.
Adds trailing whitespace.
.dotest/patch:216: * We also set the full residual flag which the
Adds trailing whitespace.
.dotest/patch:2383:{
warning: 2 lines add trailing whitespaces.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] block layer varlen-cdb
On Thu, Nov 01, 2007 at 08:05:06PM +0200, Boaz Harrosh wrote:
> @@ -287,8 +287,13 @@ struct request {
> /*
>* when request is used as a packet command carrier
>*/
> - unsigned int cmd_len;
> + unsigned short cmd_len;
> + unsigned short varlen_cdb_len; /* length of varlen_cdb buffer */
> unsigned char cmd[BLK_MAX_CDB];
> + unsigned char *varlen_cdb; /* an optional variable-length cdb.
> + * points to a user buffer that must be
> + * valid until end of request
> + */
>
Try this instead:
unsigned int cmd_len;
- unsigned char cmd[BLK_MAX_CDB];
+ unsigned char _cmd[BLK_MAX_CDB];
+ unsigned char *cmd;
Then initialise cmd to the address of _cmd. If you need to override it
later (ie patch 3), you can.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] iscsi: extended cdb support
Once varlen cdbs are supported by the block and scsi-ml layers
we can apply this patch to support extended CDBs in iscsi.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/scsi/iscsi_tcp.h |3 +-
drivers/scsi/libiscsi.c| 56
include/scsi/iscsi_proto.h |6 +++-
3 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
index ec4e5cf..30077fa 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -23,6 +23,7 @@
#define ISCSI_TCP_H
#include
+#include
/* Socket's Receive state machine */
#define IN_PROGRESS_WAIT_HEADER0x0
@@ -49,7 +50,7 @@
#define XMSTATE_SOL_HDR_INIT 0x2000
#define ISCSI_SG_TABLESIZE SG_ALL
-#define ISCSI_TCP_MAX_CMD_LEN 16
+#define ISCSI_TCP_MAX_CMD_LEN SCSI_MAX_VARLEN_CDB_SIZE
struct crypto_hash;
struct socket;
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 6a747cc..e57963c 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -139,6 +139,45 @@ static int iscsi_add_hdr(struct iscsi_cmd_task *ctask,
unsigned len)
return 0;
}
+/*
+ * make an extended cdb AHS
+ */
+static int iscsi_prep_ecdb_ahs(struct iscsi_cmd_task *ctask)
+{
+ struct scsi_cmnd *cmd = ctask->sc;
+ unsigned rlen, pad_len;
+ unsigned short ahslength;
+ struct iscsi_ecdb_ahdr *ecdb_ahdr;
+ int rc;
+
+ ecdb_ahdr = iscsi_next_hdr(ctask);
+ rlen = cmd->cmd_len - ISCSI_CDB_SIZE;
+
+ BUG_ON(rlen > sizeof(ecdb_ahdr->ecdb));
+ ahslength = rlen + sizeof(ecdb_ahdr->reserved);
+
+ pad_len = iscsi_padding(rlen);
+
+ rc = iscsi_add_hdr(ctask, sizeof(ecdb_ahdr->ahslength) +
+ sizeof(ecdb_ahdr->ahstype) + ahslength + pad_len);
+ if (rc)
+ return rc;
+
+ if (pad_len)
+ memset(&ecdb_ahdr->ecdb[rlen], 0, pad_len);
+
+ ecdb_ahdr->ahslength = cpu_to_be16(ahslength);
+ ecdb_ahdr->ahstype = ISCSI_AHSTYPE_CDB;
+ ecdb_ahdr->reserved = 0;
+ memcpy(ecdb_ahdr->ecdb, cmd->cmnd + ISCSI_CDB_SIZE, rlen);
+
+ debug_scsi("iscsi_prep_ecdb_ahs: varlen_cdb_len %d "
+ "rlen %d pad_len %d ahs_length %d iscsi_headers_size %u\n",
+ cmd->cmd_len, rlen, pad_len, ahslength, ctask->hdr_len);
+
+ return 0;
+}
+
/**
* iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
* @ctask: iscsi cmd task
@@ -152,7 +191,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
struct iscsi_session *session = conn->session;
struct iscsi_cmd *hdr = ctask->hdr;
struct scsi_cmnd *sc = ctask->sc;
- unsigned hdrlength;
+ unsigned hdrlength, cmd_len;
int rc;
ctask->hdr_len = 0;
@@ -167,10 +206,17 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_cmd_task
*ctask)
hdr->cmdsn = cpu_to_be32(session->cmdsn);
session->cmdsn++;
hdr->exp_statsn = cpu_to_be32(conn->exp_statsn);
-memcpy(hdr->cdb, sc->cmnd, sc->cmd_len);
- if (sc->cmd_len < MAX_COMMAND_SIZE)
- memset(&hdr->cdb[sc->cmd_len], 0,
- MAX_COMMAND_SIZE - sc->cmd_len);
+ cmd_len = sc->cmd_len;
+ if (cmd_len < ISCSI_CDB_SIZE)
+ memset(&hdr->cdb[cmd_len], 0,
+ ISCSI_CDB_SIZE - cmd_len);
+ else if (cmd_len > ISCSI_CDB_SIZE) {
+ rc = iscsi_prep_ecdb_ahs(ctask);
+ if (rc)
+ return rc;
+ cmd_len = ISCSI_CDB_SIZE;
+ }
+ memcpy(hdr->cdb, sc->cmnd, cmd_len);
ctask->data_count = 0;
ctask->imm_count = 0;
diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
index 4b2dce7..9dff9cc 100644
--- a/include/scsi/iscsi_proto.h
+++ b/include/scsi/iscsi_proto.h
@@ -110,6 +110,7 @@ struct iscsi_ahs_hdr {
#define ISCSI_AHSTYPE_CDB 1
#define ISCSI_AHSTYPE_RLENGTH 2
+#define ISCSI_CDB_SIZE 16
/* iSCSI PDU Header */
struct iscsi_cmd {
@@ -123,7 +124,7 @@ struct iscsi_cmd {
__be32 data_length;
__be32 cmdsn;
__be32 exp_statsn;
- uint8_t cdb[16];/* SCSI Command Block */
+ uint8_t cdb[ISCSI_CDB_SIZE];/* SCSI Command Block */
/* Additional Data (Command Dependent) */
};
@@ -152,7 +153,8 @@ struct iscsi_ecdb_ahdr {
__be16 ahslength; /* CDB length - 15, including reserved byte */
uint8_t ahstype;
uint8_t reserved;
- uint8_t ecdb[260 - 16]; /* 4-byte aligned extended CDB spillover */
+ /* 4-byte aligned extended CDB spillover */
+ uint8_t ecdb[260 - ISCSI_CDB_SIZE];
};
/* SCSI Response Header */
--
1.5.3.1
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vge
[PATCH 3/4] scsi: varlen extended and vendor-specific cdbs
Add support for variable-length, extended, and vendor specific
CDBs to scsi-ml. It is now possible for initiators and ULD's
to issue these types of commands. LLDs need not change much.
All they need is to raise the .max_cmd_len to the longest command
they support (see iscsi patches).
- clean-up some code paths that did not expect commands to be
larger than 16, and change cmd_len members' type to short as
char is not enough.
- Add support for varlen_cdb in scsi_execute.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
Signed-off-by: Benny Halevy <[EMAIL PROTECTED]>
---
block/scsi_ioctl.c |4 ++--
drivers/scsi/constants.c | 10 +++---
drivers/scsi/scsi.c | 22 +++---
drivers/scsi/scsi_lib.c | 25 +
include/scsi/scsi.h | 40 +---
include/scsi/scsi_cmnd.h |2 +-
include/scsi/scsi_host.h |8 +++-
7 files changed, 74 insertions(+), 37 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 91c7322..f08e196 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -33,13 +33,13 @@
#include
/* Command group 3 is reserved and should never be used. */
-const unsigned char scsi_command_size[8] =
+const unsigned char scsi_command_size_tbl[8] =
{
6, 10, 10, 12,
16, 12, 10, 10
};
-EXPORT_SYMBOL(scsi_command_size);
+EXPORT_SYMBOL(scsi_command_size_tbl);
#include
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 024553f..5edfe0f 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -28,7 +28,6 @@
#define SERVICE_ACTION_OUT_12 0xa9
#define SERVICE_ACTION_IN_16 0x9e
#define SERVICE_ACTION_OUT_16 0x9f
-#define VARIABLE_LENGTH_CMD 0x7f
@@ -210,7 +209,7 @@ static void print_opcode_name(unsigned char * cdbp, int
cdb_len)
cdb0 = cdbp[0];
switch(cdb0) {
case VARIABLE_LENGTH_CMD:
- len = cdbp[7] + 8;
+ len = scsi_varlen_cdb_length(cdbp);
if (len < 10) {
printk("short variable length command, "
"len=%d ext_len=%d", len, cdb_len);
@@ -300,7 +299,7 @@ static void print_opcode_name(unsigned char * cdbp, int
cdb_len)
cdb0 = cdbp[0];
switch(cdb0) {
case VARIABLE_LENGTH_CMD:
- len = cdbp[7] + 8;
+ len = scsi_varlen_cdb_length(cdbp);
if (len < 10) {
printk("short opcode=0x%x command, len=%d "
"ext_len=%d", cdb0, len, cdb_len);
@@ -335,10 +334,7 @@ void __scsi_print_command(unsigned char *cdb)
int k, len;
print_opcode_name(cdb, 0);
- if (VARIABLE_LENGTH_CMD == cdb[0])
- len = cdb[7] + 8;
- else
- len = COMMAND_SIZE(cdb[0]);
+ len = scsi_command_size(cdb);
/* print out all bytes in cdb */
for (k = 0; k < len; ++k)
printk(" %02x", cdb[k]);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index e7dd171..6bbc053 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -79,15 +79,6 @@ static void scsi_done(struct scsi_cmnd *cmd);
#define MIN_RESET_PERIOD (15*HZ)
/*
- * Macro to determine the size of SCSI command. This macro takes vendor
- * unique commands into account. SCSI commands in groups 6 and 7 are
- * vendor unique and we will depend upon the command length being
- * supplied correctly in cmd_len.
- */
-#define CDB_SIZE(cmd) (cmd)->cmnd[0] >> 5) & 7) < 6) ? \
- COMMAND_SIZE((cmd)->cmnd[0]) : (cmd)->cmd_len)
-
-/*
* Note - the initial logging level can be set here to log events at boot time.
* After the system is up, you may enable logging via the /proc interface.
*/
@@ -467,6 +458,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
unsigned long flags = 0;
unsigned long timeout;
int rtn = 0;
+ unsigned cmd_len;
/* check if the device is still usable */
if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
@@ -548,9 +540,17 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
* Before we queue this command, check if the command
* length exceeds what the host adapter can handle.
*/
- if (CDB_SIZE(cmd) > cmd->device->host->max_cmd_len) {
+ cmd_len = cmd->cmd_len;
+ if (!cmd_len) {
+ BUG_ON(cmd->cmnd[0] == VARIABLE_LENGTH_CMD);
+ cmd_len = COMMAND_SIZE((cmd)->cmnd[0]);
+ }
+
+ if (cmd_len > cmd->device->host->max_cmd_len) {
SCSI_LOG_MLQUEUE(3,
- printk("queuecommand : command too long.\n"));
+ printk("queuecommand : command too long. "
+ "cdb_size=%d host->max_cmd_len=%d\n",
+ cmd->cmd_len, cmd->device->host->max_cmd_len));
cmd->result = (DID_AB
[PATCH 2/4] block layer varlen-cdb
- add varlen_cdb and varlen_cdb_len to hold a large user cdb
if needed. They start as empty. Allocation of buffer must
be done by user and held until request execution is done.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
block/ll_rw_blk.c |2 ++
include/linux/blkdev.h |7 ++-
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/block/ll_rw_blk.c b/block/ll_rw_blk.c
index b01dee3..df478f2 100644
--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -261,6 +261,8 @@ static void rq_init(struct request_queue *q, struct request
*rq)
rq->end_io_data = NULL;
rq->completion_data = NULL;
rq->next_rq = NULL;
+ rq->varlen_cdb_len = 0;
+ rq->varlen_cdb = NULL;
}
/**
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index bbf906a..990643d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -287,8 +287,13 @@ struct request {
/*
* when request is used as a packet command carrier
*/
- unsigned int cmd_len;
+ unsigned short cmd_len;
+ unsigned short varlen_cdb_len; /* length of varlen_cdb buffer */
unsigned char cmd[BLK_MAX_CDB];
+ unsigned char *varlen_cdb; /* an optional variable-length cdb.
+* points to a user buffer that must be
+* valid until end of request
+*/
unsigned int data_len;
unsigned int sense_len;
--
1.5.3.1
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] Let scsi_cmnd->cmnd use request->cmd[] buffer
- struct scsi_cmnd had a 16 bytes command buffer of its own.
This is an unnecessary duplication and copy of request's
cmd. It is probably left overs from the time that scsi_cmnd
could function without a request attached. So clean that up.
- Once above is done, few places, apart from scsi-ml, needed
adjustments due to changing the data type of scsi_cmnd->cmnd.
- Lots of drivers still use MAX_COMMAND_SIZE. So I have left
that #define but equate it to BLK_MAX_CDB. The way I see it
and is reflected in the patch below is.
MAX_COMMAND_SIZE - means: The longest fixed-length (*) SCSI CDB
as per the SCSI standard and is not related
to the implementation.
BLK_MAX_CDB. - The allocated space at the request level
(*)fixed-length here means commands that their size can be determined
by their opcode and the CDB does not carry a length specifier, like
the VARIABLE_LENGTH_CMD(0x7f) command. This is actually not exactly
true and the SCSI standard also defines extended commands and
vendor specific commands that can be bigger than 16 bytes. The kernel
will support these using the same infrastructure used for VARLEN CDB's.
So in effect MAX_COMMAND_SIZE means the maximum size command
scsi-ml supports without specifying a cmd_len by ULD's
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
---
drivers/firewire/fw-sbp2.c |2 +-
drivers/s390/scsi/zfcp_dbf.c |2 +-
drivers/scsi/53c700.c|6 +++---
drivers/scsi/ibmvscsi/ibmvscsi.c |2 +-
drivers/scsi/scsi_error.c| 14 --
drivers/scsi/scsi_lib.c |5 +++--
drivers/scsi/scsi_tgt_lib.c |2 ++
drivers/usb/storage/isd200.c |2 ++
include/scsi/scsi_cmnd.h | 10 +++---
include/scsi/scsi_eh.h |6 --
10 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index 5596df6..151d02c 100644
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -1212,7 +1212,7 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd,
scsi_done_fn_t done)
memset(orb->request.command_block,
0, sizeof(orb->request.command_block));
- memcpy(orb->request.command_block, cmd->cmnd, COMMAND_SIZE(*cmd->cmnd));
+ memcpy(orb->request.command_block, cmd->cmnd, cmd->cmd_len);
orb->base.callback = complete_command_orb;
orb->base.request_bus =
diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index ffa3bf7..2d43c10 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -730,7 +730,7 @@ _zfcp_scsi_dbf_event_common(const char *tag, const char
*tag2, int level,
rec->scsi_result = scsi_cmnd->result;
rec->scsi_cmnd = (unsigned long)scsi_cmnd;
rec->scsi_serial = scsi_cmnd->serial_number;
- memcpy(rec->scsi_opcode, &scsi_cmnd->cmnd,
+ memcpy(rec->scsi_opcode, scsi_cmnd->cmnd,
min((int)scsi_cmnd->cmd_len,
ZFCP_DBF_SCSI_OPCODE));
rec->scsi_retries = scsi_cmnd->retries;
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 71ff3fb..85460f8 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -599,7 +599,7 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata,
(struct NCR_700_command_slot *)SCp->host_scribble;
dma_unmap_single(hostdata->dev, slot->pCmd,
-sizeof(SCp->cmnd), DMA_TO_DEVICE);
+MAX_COMMAND_SIZE, DMA_TO_DEVICE);
if (slot->flags == NCR_700_FLAG_AUTOSENSE) {
char *cmnd = NCR_700_get_sense_cmnd(SCp->device);
#ifdef NCR_700_DEBUG
@@ -1003,7 +1003,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct
scsi_cmnd *SCp,
* here */
NCR_700_unmap(hostdata, SCp, slot);
dma_unmap_single(hostdata->dev, slot->pCmd,
-sizeof(SCp->cmnd),
+MAX_COMMAND_SIZE,
DMA_TO_DEVICE);
cmnd[0] = REQUEST_SENSE;
@@ -1900,7 +1900,7 @@ NCR_700_queuecommand(struct scsi_cmnd *SCp, void
(*done)(struct scsi_cmnd *))
}
slot->resume_offset = 0;
slot->pCmd = dma_map_single(hostdata->dev, SCp->cmnd,
- sizeof(SCp->cmnd), DMA_TO_DEVICE);
+ MAX_COMMAND_SIZE, DMA_TO_DEVICE);
NCR_700_start_command(SCp);
return 0;
}
diff -
[RFC 0/4] varlen extended and vendor-specific cdbs
This is a proposal for adding support for variable-length, extended, and vendor specific CDBs. It should now cover the entire range of the SCSI standard. This patchset extends my original submission (over a year old) in that it starts with cleaning up scsi_cmnd, hence the first patch. The other three are similar to the ones submitted before. This effort is orthogonal to the bidi and scsi_data_buffer effort and can be accepted now. The patchset, however, is presented in this RFC on top of the scsi_data_buffer patches, as they sit in my tree. They can easily be rebased to current scsi-misc. The iscsi patch is on top of the iscsi branch of the iscsi-git-tree. (Matthew, these patches will conflict with your scsi_cmnd cleanup patches I promise to rebase them before submission) A complete git-tree based on scsi-misc which includes a complete bidi and varlen work can be fetched from: - git://git.bhalevy.com/open-osd bidi branch (varlen branch is this work) - http://www.bhalevy.com/git - has the gitweb GUI [1/4] Let scsi_cmnd->cmnd use request->cmd buffer Here I let scsi_cmnd->cmnd point to the space allocated by request->cmd, instead of copying the data. The scsi_cmnd->cmd_len is guaranteed to contain the right length of the command. I have tried to go over every single place in the kernel that uses scsi_cmnd->cmnd and make sure it looks sane. Surprisingly to me, that was not at all bad. I hope I did not miss anything. I've tested on an x86_64 machine booting from a sata disk and ran the iscsi regression tests as well as my bidi and varlen tests on top of the complete patchset and all tests passed. [2/4] block layer varlen-cdb Unlike the scsi approach (see below), I did not want to unify cmd[]/cmd_len and the *varlen_cdb and varlen_cdb_len members of struct request. This is because unlike scsi, block devices do not have a .max_cmd_len parameter to protect them from unexpected large commands. In the case varlen_cdb and varlen_cdb_len are used the cmd[] buffer is ignored and the cmd_len will be set to zero. [3/4] scsi: varlen extended and vendor-specific cdbs Adds support for variable length, extended, and vendor-specific cdbs at the scsi mid-layer. [4/4] iscsi: extended cdb support This is on top of the iscsi branch. In the URL above there are the three missing patches for iscsi, that add support for AHSs. Boaz - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] aha152x: Use scsi_eh API for REQUEST_SENSE invocation
- Use new scsi_eh_prep/restor_cmnd() for synchronous
REQUEST_SENSE invocation.
Signed-off-by: Boaz Harrosh <[EMAIL PROTECTED]>
drivers/scsi/aha152x.c | 38 --
1 files changed, 8 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index ea8c699..6ccdc96 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -260,6 +260,7 @@
#include
#include
#include
+#include
#include "aha152x.h"
static LIST_HEAD(aha152x_host_list);
@@ -558,9 +559,7 @@ struct aha152x_hostdata {
struct aha152x_scdata {
Scsi_Cmnd *next;/* next sc in queue */
struct completion *done;/* semaphore to block on */
- unsigned char aha_orig_cmd_len;
- unsigned char aha_orig_cmnd[MAX_COMMAND_SIZE];
- int aha_orig_resid;
+ struct scsi_eh_save ses;
};
/* access macros for hostdata */
@@ -1017,16 +1016,10 @@ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt,
struct completion *complete,
SCp.buffers_residual : left buffers in list
SCp.phase: current state of the command */
- if ((phase & (check_condition|resetting)) || !scsi_sglist(SCpnt)) {
- if (phase & check_condition) {
- SCpnt->SCp.ptr = SCpnt->sense_buffer;
- SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
- scsi_set_resid(SCpnt, sizeof(SCpnt->sense_buffer));
- } else {
- SCpnt->SCp.ptr = NULL;
- SCpnt->SCp.this_residual = 0;
- scsi_set_resid(SCpnt, 0);
- }
+ if ((phase & resetting) || !scsi_sglist(SCpnt)) {
+ SCpnt->SCp.ptr = NULL;
+ SCpnt->SCp.this_residual = 0;
+ scsi_set_resid(SCpnt, 0);
SCpnt->SCp.buffer = NULL;
SCpnt->SCp.buffers_residual = 0;
} else {
@@ -1561,10 +1554,7 @@ static void busfree_run(struct Scsi_Host *shpnt)
}
#endif
- /* restore old command */
- memcpy(cmd->cmnd, sc->aha_orig_cmnd, sizeof(cmd->cmnd));
- cmd->cmd_len = sc->aha_orig_cmd_len;
- scsi_set_resid(cmd, sc->aha_orig_resid);
+ scsi_eh_restore_cmnd(cmd, &sc->ses);
cmd->SCp.Status = SAM_STAT_CHECK_CONDITION;
@@ -1587,22 +1577,10 @@ static void busfree_run(struct Scsi_Host *shpnt)
DPRINTK(debug_eh, ERR_LEAD "requesting
sense\n", CMDINFO(ptr));
#endif
- /* save old command */
sc = SCDATA(ptr);
/* It was allocated in aha152x_internal_queue?
*/
BUG_ON(!sc);
- memcpy(sc->aha_orig_cmnd, ptr->cmnd,
- sizeof(ptr->cmnd));
- sc->aha_orig_cmd_len = ptr->cmd_len;
- sc->aha_orig_resid = scsi_get_resid(ptr);
-
- ptr->cmnd[0] = REQUEST_SENSE;
- ptr->cmnd[1] = 0;
- ptr->cmnd[2] = 0;
- ptr->cmnd[3] = 0;
- ptr->cmnd[4] =
sizeof(ptr->sense_buffer);
- ptr->cmnd[5] = 0;
- ptr->cmd_len = 6;
+ scsi_eh_prep_cmnd(ptr, &sc->ses, NULL, 0, ~0);
DO_UNLOCK(flags);
aha152x_internal_queue(ptr, NULL,
check_condition, ptr->scsi_done);
--
1.5.3.1
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DMA API Re: Remove dma_coherent_mem interface
* ian <[EMAIL PROTECTED]> [2007-11-01 16:33]: > On Thu, 2007-11-01 at 19:07 +0900, Paul Mundt wrote: > > Indeed. The SH case for OHCI on MFDs is the same. So that's at least 2 > > users. > > three if you count James SCSI card. Four if you count http://most4linux.sourceforge.net/. Thanks, Bernhard - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DMA API Re: Remove dma_coherent_mem interface
On Thu, 2007-11-01 at 19:07 +0900, Paul Mundt wrote: > Indeed. The SH case for OHCI on MFDs is the same. So that's at least 2 > users. three if you count James SCSI card. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8
Stephen Rothwell <[EMAIL PROTECTED]> writes: > diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c > index 240a0bb..3c2dbc0 100644 > --- a/drivers/scsi/aacraid/commsup.c > +++ b/drivers/scsi/aacraid/commsup.c > @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac) > aif = (struct aac_aifcmd *)hw_fib->data; > aif->command = cpu_to_le32(AifCmdEventNotify); > aif->seqnum = cpu_to_le32(0x); > - aif->data[0] = cpu_to_le32(AifEnExpEvent); > - aif->data[1] = cpu_to_le32(AifExeFirmwarePanic); > - aif->data[2] = cpu_to_le32(AifHighPriority); > + aif->data[0] = AifEnExpEvent; > + aif->data[1] = AifExeFirmwarePanic; > + aif->data[2] = AifHighPriority; > aif->data[3] = cpu_to_le32(BlinkLED); What about the last line? Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8
ACK v2 Sincerely -- Mark Salyzyn > -Original Message- > From: Stephen Rothwell [mailto:[EMAIL PROTECTED] > Sent: Thursday, November 01, 2007 2:32 AM > To: AACRAID > Cc: linux-scsi@vger.kernel.org; LKML > Subject: [PATCHv2] aacraid: don't assign cpu_to_le32(constant) to u8 > > Noticed on PowerPC allmod config build: > > drivers/scsi/aacraid/commsup.c:1342: warning: large integer > implicitly truncated to unsigned type > drivers/scsi/aacraid/commsup.c:1343: warning: large integer > implicitly truncated to unsigned type > drivers/scsi/aacraid/commsup.c:1344: warning: large integer > implicitly truncated to unsigned type > > Also fix some whitespace on the changed lines. > > Signed-off-by: Stephen Rothwell <[EMAIL PROTECTED]> > --- > drivers/scsi/aacraid/commsup.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > This version just fixes a couple of whitespace anomolies on > the lines I > changed. > > -- > Cheers, > Stephen Rothwell[EMAIL PROTECTED] > > diff --git a/drivers/scsi/aacraid/commsup.c > b/drivers/scsi/aacraid/commsup.c > index 240a0bb..3c2dbc0 100644 > --- a/drivers/scsi/aacraid/commsup.c > +++ b/drivers/scsi/aacraid/commsup.c > @@ -1339,9 +1339,9 @@ int aac_check_health(struct aac_dev * aac) > aif = (struct aac_aifcmd *)hw_fib->data; > aif->command = cpu_to_le32(AifCmdEventNotify); > aif->seqnum = cpu_to_le32(0x); > - aif->data[0] = cpu_to_le32(AifEnExpEvent); > - aif->data[1] = cpu_to_le32(AifExeFirmwarePanic); > - aif->data[2] = cpu_to_le32(AifHighPriority); > + aif->data[0] = AifEnExpEvent; > + aif->data[1] = AifExeFirmwarePanic; > + aif->data[2] = AifHighPriority; > aif->data[3] = cpu_to_le32(BlinkLED); > > /* > -- > 1.5.3.4 > - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DMA API Re: Remove dma_coherent_mem interface
On Thu, Nov 01, 2007 at 09:48:46AM +, ian wrote: > On Wed, 2007-10-31 at 15:34 -0600, Matthew Wilcox wrote: > > It may not be significant, so it would be an easy decision to keep > > the interface. But if the decision is going to be to keep the > > interface despite the lack of users and no matter what the > > performance win, then I don't need to finish this work. > > I dont mind what the interface is as long as we can support devices that > can only DMA from their own local RAM as described above... > Indeed. The SH case for OHCI on MFDs is the same. So that's at least 2 users. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DMA API Re: Remove dma_coherent_mem interface
On Wed, 2007-10-31 at 15:34 -0600, Matthew Wilcox wrote: > On Wed, Oct 31, 2007 at 09:03:24PM +, ian wrote: > It's been three years since this API was introduced, and the only > *merged* user is the one that James wrote. Yes I appreciate that. > Now it's in the way of some > performance improvements I want to make. I can't possibly go round every > little platform tree looking for users Agreed - hence why Im taking the trouble to writ e to you now to let you know the code is in use. > -- you need to get your drivers > into mainline. Yes, it's not necessarily a quick process, but equally, > you've had three years. I've been quite ill, which I admit is not anyone elses fault. Anyhow, to business... > Now, what driver is it that's going to consume this coherent memory? all the TMIO based ASICs that have USB will need this support - thats three different chips alone. I believe the mediaQ chipset has similar constraints on its USB and (IIRC) other devices. Part of the problem is that these chips are multi-function devices and as such it takes a lot longer to get all the drivers polished up to a good state than it would for a single-function chip. I suppose individual functions could be submitted seperately though. > Does it use the dma_pool interface, or does it use dma_alloc_coherent > directly? At this point, I'd have to check - which I will be doing shortly - I've jusrt started work on my toshiba devices again, although I started with AC97, so it wont be right away. > If the latter, I can retain the dma_declare_coherent_memory > interface. If the former, then we have to come to a decision between > supporting some drivers that are so unimportant that they haven't been > merged in three years and making a performance win for drivers that are > used every day. As its the OHCI driver doing the allocation, I'd imagine the dma_pool interface is the one in use. > Admittedly, I haven't finished the work to make dma_pool use slub, thus > I don't know what the performance win will turn out to be. The specific problem for these devices is that they can only DMA to their internal memory, and cant decode the 32 bit addresses of the bus they sit on - so whilst they might be mapped at 0x800x , they see their address space as being 0x0 - 0xf (for example). What might work would be if the dma pool interface could use a device-specified allocator to service its requests, thus most stuff would use slub (by default) and our devices could specify their own allocator > It may not > be significant, so it would be an easy decision to keep the interface. > But if the decision is going to be to keep the interface despite the > lack of users and no matter what the performance win, then I don't need > to finish this work. I dont mind what the interface is as long as we can support devices that can only DMA from their own local RAM as described above... :-) - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
