mirrors/scst
1
0
Fork 0
mirror of https://github.com/SCST-project/scst.git synced 2026-05-22 21:21:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

- Cleanups and fixes in transfer length and direction processing.

Browse Source 
git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@210 d57e44dd-8a1f-0410-8b47-8ef2f437770f
This commit is contained in:
Vladislav Bolkhovitin
2007-10-25 09:54:00 +00:00
parent ec6f11c514
commit 3e3ff59ca8
8 changed files with 173 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files

118
scst/README
View File

@@ -172,34 +172,66 @@ Compilation options
There are the following compilation options, that could be commented
in/out in Makefile:
- DEBUG - turns on some debugging code, including some logging. Makes
the driver considerably bigger and slower, producing large amount of
log data.
- DEBUG - if defined, turns on some debugging code, including some
logging. Makes the driver considerably bigger and slower, producing
large amount of log data.
- TRACING - turns on ability to log events. Makes the driver considerably
bigger and lead to some performance loss.
- TRACING - if defined, turns on ability to log events. Makes the
driver considerably bigger and lead to some performance loss.
- EXTRACHECKS - adds extra validity checks in the various places.
- EXTRACHECKS - if defined, adds extra validity checks in the various
places.
- DEBUG_TM - turns on task management functions debugging, when on
LUN 0 in the default access control group some of the commands will
be delayed for about 60 sec., so making the remote initiator send TM
functions, eg ABORT TASK and TARGET RESET. Also set TM_DBG_GO_OFFLINE
symbol in the Makefile to 1 if you want that the device eventually
become completely unresponsive, or to 0 otherwise to circle around
ABORTs and RESETs code. Needs DEBUG turned on.
- USE_EXPECTED_VALUES - if not defined (default), initiator supplied
expected data transfer length and direction will be used only for
verification purposes to return error or warn in case if one of them
is invalid. Instead, locally decoded from SCSI command values will be
used. This is necessary for security reasons, because otherwise a
faulty initiator can crash target by supplying invalid value in one
of those parameters. This is especially important in case of
pass-through mode. If USE_EXPECTED_VALUES is defined, initiator
supplied expected data transfer length and direction will override
the locally decoded values. This might be necessary if internal SCST
commands translation table doesn't contain SCSI command, which is
used in your environment. You can know that if you have messages like
"Unknown opcode XX for YY. Should you update scst_scsi_op_table?" in
your kernel log and your initiator returns an error. Also report
those messages in the SCST mailing list
scst-devel@lists.sourceforge.net. Note, that not all SCSI transports
support supplying expected values.
- STRICT_SERIALIZING - makes SCST send all commands to underlying SCSI
device synchronously, one after one. This makes task management more
reliable, with cost of some performance penalty. This is mostly
actual for stateful SCSI devices like tapes, where the result of
command's execution depends from device's settings set by previous
commands. Disk and RAID devices are stateless in the most cases. The
current SCSI core in Linux doesn't allow to abort all commands
reliably if they sent asynchronously to a stateful device. Turned off
by default, turn it on if you use stateful device(s) and need as much
error recovery reliability as possible. As a side effect, no kernel
patching is necessary.
- DEBUG_TM - if defined, turns on task management functions debugging,
when on LUN 0 in the default access control group some of the
commands will be delayed for about 60 sec., so making the remote
initiator send TM functions, eg ABORT TASK and TARGET RESET. Also
define TM_DBG_GO_OFFLINE symbol in the Makefile to 1 if you want that
the device eventually become completely unresponsive, or to 0
otherwise to circle around ABORTs and RESETs code. Needs DEBUG turned
on.
- STRICT_SERIALIZING - if defined, makes SCST send all commands to
underlying SCSI device synchronously, one after one. This makes task
management more reliable, with cost of some performance penalty. This
is mostly actual for stateful SCSI devices like tapes, where the
result of command's execution depends from device's settings defined
by previous commands. Disk and RAID devices are stateless in the most
cases. The current SCSI core in Linux doesn't allow to abort all
commands reliably if they sent asynchronously to a stateful device.
Turned off by default, turn it on if you use stateful device(s) and
need as much error recovery reliability as possible. As a side
effect, no kernel patching is necessary.
- ALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ - if defined, it will be allowed
to submit pass-through commands to real SCSI devices via the SCSI
middle layer using scsi_execute_async() function from soft IRQ
context (tasklets). This used to be the default, but currently it
seems the SCSI middle layer starts expecting only thread context on
the IO submit path, so it is disabled now by default. Enabling it
will decrease amount of context switches and improve performance. It
is more or less safe, in the worst case, if in your configuration the
SCSI middle layer really doesn't expect SIRQ context in
scsi_execute_async() function, you will get a warning message in the
kernel log.
- SCST_HIGHMEM - if defined on HIGHMEM systems with 2.6 kernels, it
allows SCST to use HIGHMEM. This is very experimental feature, which
@@ -216,8 +248,8 @@ in/out in Makefile:
HIGHMEM kernel configurations are fully supported, but not recommended
for performance reasons, except for scst_user, where they are not
supported, because this module deals with user supplied memory on a
zero-copy manner. Consider change VMSPLIT option or use 64-bit system
configuration instead.
zero-copy manner. If you need to use it, consider change VMSPLIT option
or use 64-bit system configuration instead.
For changing VMSPLIT option (CONFIG_VMSPLIT to be precise) you should in
"make menuconfig" command set the following variables:
@@ -437,15 +469,23 @@ IMPORTANT: By default for performance reasons VDISK FILEIO devices use write
workloads write through caching might perform better, than
write back one with the barrier protection turned on.
IMPORTANT: Many disk and partition table management utilities don't support
IMPORTANT: Some disk and partition table management utilities don't support
========= block sizes >512 bytes, therefore make sure that your favorite one
supports it. Also, if you export disk file or device with
some block size, different from one, with which it was
already divided on partitions, you could get various weird
supports it. Currently only cfdisk is known to work only with
512 bytes blocks, other utilities like fdisk on Linux or
standard disk manager on Windows are proved to work well with
non-512 bytes blocks. Note, if you export a disk file or
device with some block size, different from one, with which
it was already partitioned, you could get various weird
things like utilities hang up or other unexpected behavior.
Hence, to be sure, zero the exported file or device before the
first access to it from the remote initiator with another
block size.
Hence, to be sure, zero the exported file or device before
the first access to it from the remote initiator with another
block size. On Window initiator make sure you "Set Signature"
in the disk manager on the imported from the target drive
before doing any other partitioning on it. After you
successfully mounted a file system over non-512 bytes block
size device, the block size stops matter, any program will
work with files on such file system.
BLOCKIO VDISK mode
------------------
@@ -465,9 +505,10 @@ which prefer or demand direct IO access and, because of the nature of
their data access, can actually see worse performance with
non-discriminate caching.
4) Multiple layers of targets were the secondary/triary layers need to
have a consistent view of the primary targets in order to preserve data
integrity which a page cache backed IO type might not provide reliably.
4) Multiple layers of targets were the secondary and above layers need
to have a consistent view of the primary targets in order to preserve
data integrity which a page cache backed IO type might not provide
reliably.
Also it has an advantage over FILEIO that it doesn't copy data between
the system cache and the commands data buffers, so it saves a
@@ -533,6 +574,8 @@ II. In order to get the maximum performance you should:
- Disable in Makefile STRICT_SERIALIZING, EXTRACHECKS, TRACING, DEBUG*,
SCST_STRICT_SECURITY, SCST_HIGHMEM
- For pass-through devices enable ALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ.
2. For target drivers:
- Disable in Makefiles EXTRACHECKS, TRACING, DEBUG*
@@ -552,7 +595,8 @@ IMPORTANT: Some of the compilation options enabled by default, i.e. SCST
than for performance.
If you use SCST version taken directly from its SVN repository, you can
set the above options using debug2perf script file.
set the above options, except ALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ, using
debug2perf script file.
4. For kernel:

1
scst/include/scst_const.h
View File

@@ -121,6 +121,7 @@ enum scst_cmd_queue_type
#define scst_sense_read_error MEDIUM_ERROR, 0x11, 0
#define scst_sense_write_error MEDIUM_ERROR, 0x03, 0
#define scst_sense_not_ready NOT_READY, 0x04, 0x10
#define scst_sense_invalid_message ILLEGAL_REQUEST, 0x49, 0
/*************************************************************
* SCSI opcodes not listed anywhere else

3
scst/src/Makefile
View File

@@ -116,6 +116,9 @@ EXTRA_CFLAGS += -I$(SCST_INC_DIR) -Wextra -Wno-unused-parameter
EXTRA_CFLAGS += -DEXTRACHECKS
#EXTRA_CFLAGS += -DUSE_EXPECTED_VALUES
#EXTRA_CFLAGS += -DALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ
#EXTRA_CFLAGS += -fno-inline
#EXTRA_CFLAGS += -DTRACING

18
scst/src/scst_cdbprobe.h
View File

@@ -249,16 +249,6 @@ static const struct scst_sdbops scst_scsi_op_table[] = {
SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 7, get_trans_len_2},
{0x2F, "O OO O ", "VERIFY(10)",
SCST_DATA_WRITE, SCST_TRANSFER_LEN_TYPE_FIXED, 7, get_trans_len_2},
/*
{0x30, "O OO O ", "SEARCH DATA HIGH(10)",
SCST_DATA_NONE, SCST_UNKNOWN_LENGTH, 0, get_trans_len_none},
{0x31, " O ", "OBJECT POSITION",
SCST_DATA_NONE, SCST_UNKNOWN_LENGTH, 0, get_trans_len_none},
{0x31, "O OO O ", "SEARCH DATA EQUAL(10)",
SCST_DATA_NONE, SCST_UNKNOWN_LENGTH, 0, get_trans_len_none},
{0x32, "O OO O ", "SEARCH DATA LOW(10)",
SCST_DATA_NONE, SCST_UNKNOWN_LENGTH, 0, get_trans_len_none},
*/
{0x33, "O OO O ", "SET LIMITS(10)",
SCST_DATA_NONE, FLAG_NONE, 0, get_trans_len_none},
{0x34, " O ", "READ POSITION",
@@ -402,7 +392,7 @@ static const struct scst_sdbops scst_scsi_op_table[] = {
{0x93, " M ", "ERASE(16)",
SCST_DATA_NONE, SCST_LONG_TIMEOUT, 0, get_trans_len_none},
{0x9E, "O ", "SERVICE ACTION IN",
SCST_DATA_READ, FLAG_NONE, 0, get_trans_len_none},
SCST_DATA_READ, SCST_UNKNOWN_LENGTH, 0, get_trans_len_none},
/* 12-bytes length CDB */
{0xA0, "VVVVVVVVVV M ", "REPORT LUN",
@@ -443,7 +433,7 @@ static const struct scst_sdbops scst_scsi_op_table[] = {
SCST_DATA_NONE, FLAG_NONE, 0, get_trans_len_none},
{0xAC, " O ", "ERASE(12)",
SCST_DATA_NONE, FLAG_NONE, 0, get_trans_len_none},
{0xAC, " O ", "GET PERFORMANCE",
{0xAC, " M ", "GET PERFORMANCE",
SCST_DATA_READ, SCST_UNKNOWN_LENGTH, 0, get_trans_len_none},
{0xAD, " O ", "READ DVD STRUCTURE",
SCST_DATA_READ, FLAG_NONE, 8, get_trans_len_2},
@@ -467,8 +457,8 @@ static const struct scst_sdbops scst_scsi_op_table[] = {
SCST_DATA_READ, FLAG_NONE, 9, get_trans_len_1},
{0xB6, " O ", "SEND VOLUME TAG",
SCST_DATA_WRITE, FLAG_NONE, 9, get_trans_len_1},
{0xB6, " O ", "SET STREAMING",
SCST_DATA_WRITE, FLAG_NONE, 0, get_trans_len_none},
{0xB6, " M ", "SET STREAMING",
SCST_DATA_WRITE, FLAG_NONE, 9, get_trans_len_2},
{0xB7, " O ", "READ DEFECT DATA(12)",
SCST_DATA_READ, FLAG_NONE, 9, get_trans_len_1},
{0xB8, " O ", "READ ELEMENT STATUS",

32
scst/src/scst_lib.c
View File

@@ -1626,7 +1626,7 @@ static uint32_t get_trans_len_block_limit(const uint8_t *cdb, uint8_t off)
static uint32_t get_trans_len_read_capacity(const uint8_t *cdb, uint8_t off)
{
return 8;
return READ_CAP_LEN;
}
static uint32_t get_trans_len_single(const uint8_t *cdb, uint8_t off)
@@ -1698,7 +1698,7 @@ int scst_get_cdb_info(const uint8_t *cdb_p, int dev_type,
#ifdef EXTRACHECKS
if (unlikely((info_p->transfer_len == 0) &&
(info_p->direction != SCST_DATA_NONE))) {
TRACE_DBG("Warning! transfer_len 0, direction %d change on %d",
TRACE_MGMT_DBG("Warning! transfer_len 0, direction %d change on %d",
info_p->direction, SCST_DATA_NONE);
info_p->direction = SCST_DATA_NONE;
}
@@ -1846,10 +1846,6 @@ int scst_sbc_generic_parse(struct scst_cmd *cmd,
info_cdb->direction, info_cdb->flags, info_cdb->transfer_len);
switch (cmd->cdb[0]) {
case READ_CAPACITY:
cmd->bufflen = READ_CAP_LEN;
cmd->data_direction = SCST_DATA_READ;
break;
case SERVICE_ACTION_IN:
if ((cmd->cdb[1] & 0x1f) == SAI_READ_CAPACITY_16) {
cmd->bufflen = READ_CAP16_LEN;
@@ -1911,18 +1907,6 @@ int scst_cdrom_generic_parse(struct scst_cmd *cmd,
cmd->cdb[1] &= 0x1f;
switch (cmd->cdb[0]) {
case READ_CAPACITY:
cmd->bufflen = READ_CAP_LEN;
cmd->data_direction = SCST_DATA_READ;
break;
case GPCMD_SET_STREAMING:
cmd->bufflen = (((*(cmd->cdb + 9)) & 0xff) << 8) +
((*(cmd->cdb + 10)) & 0xff);
cmd->bufflen &= 0xffff;
break;
case GPCMD_READ_CD:
cmd->bufflen = cmd->bufflen >> 8;
break;
case VERIFY_6:
case VERIFY:
case VERIFY_12:
@@ -1972,18 +1956,6 @@ int scst_modisk_generic_parse(struct scst_cmd *cmd,
cmd->cdb[1] &= 0x1f;
switch (cmd->cdb[0]) {
case READ_CAPACITY:
cmd->bufflen = READ_CAP_LEN;
cmd->data_direction = SCST_DATA_READ;
break;
case 0xB6 /* SET_STREAMING */ :
cmd->bufflen = (((*(cmd->cdb + 9)) & 0xff) << 8) +
((*(cmd->cdb + 10)) & 0xff);
cmd->bufflen &= 0xffff;
break;
case 0xBE /* READ_CD */ :
cmd->bufflen = cmd->bufflen >> 8;
break;
case VERIFY_6:
case VERIFY:
case VERIFY_12:

7
scst/src/scst_main.c
View File

@@ -606,8 +606,13 @@ static int scst_dev_handler_check(struct scst_dev_type *dev_handler)
goto out;
}
if (dev_handler->exec == NULL)
if (dev_handler->exec == NULL) {
#ifdef ALLOW_PASSTHROUGH_IO_SUBMIT_IN_SIRQ
dev_handler->exec_atomic = 1;
#else
dev_handler->exec_atomic = 0;
#endif
}
if (dev_handler->dev_done == NULL)
dev_handler->dev_done_atomic = 1;

2
scst/src/scst_priv.h
View File

@@ -126,6 +126,8 @@ static inline int scst_get_context(void) {
return SCST_CONTEXT_DIRECT;
}
extern unsigned long scst_max_cmd_mem;
#define SCST_MGMT_CMD_CACHE_STRING "scst_mgmt_cmd"
extern struct kmem_cache *scst_mgmt_cachep;
extern mempool_t *scst_mgmt_mempool;

108
scst/src/scst_targ.c
View File

@@ -305,23 +305,20 @@ static int scst_parse_cmd(struct scst_cmd *cmd)
*/
if (unlikely(scst_get_cdb_info(cmd->cdb, dev->handler->type,
&cdb_info) != 0))
{
static int t;
if (t < 10) {
t++;
PRINT_INFO_PR("Unknown opcode 0x%02x for %s. "
"Should you update scst_scsi_op_table?",
cmd->cdb[0], dev->handler->name);
}
&cdb_info) != 0)) {
PRINT_ERROR_PR("Unknown opcode 0x%02x for %s. "
"Should you update scst_scsi_op_table?",
cmd->cdb[0], dev->handler->name);
#ifdef USE_EXPECTED_VALUES
if (scst_cmd_is_expected_set(cmd)) {
TRACE(TRACE_SCSI, "Using initiator supplied values: "
"direction %d, transfer_len %d",
cmd->expected_data_direction,
cmd->expected_transfer_len);
cmd->data_direction = cmd->expected_data_direction;
cmd->bufflen = cmd->expected_transfer_len;
/* Restore (most probably) lost CDB length */
/* Restore (likely) lost CDB length */
cmd->cdb_len = scst_get_cdb_len(cmd->cdb);
if (cmd->cdb_len == -1) {
PRINT_ERROR_PR("Unable to get CDB length for "
@@ -333,13 +330,17 @@ static int scst_parse_cmd(struct scst_cmd *cmd)
}
} else {
PRINT_ERROR_PR("Unknown opcode 0x%02x for %s and "
"target %s not supplied expected values. "
"Returning INVALID OPCODE.", cmd->cdb[0],
dev->handler->name, cmd->tgtt->name);
"target %s not supplied expected values",
cmd->cdb[0], dev->handler->name, cmd->tgtt->name);
scst_set_cmd_error(cmd,
SCST_LOAD_SENSE(scst_sense_invalid_opcode));
goto out_xmit;
}
#else
scst_set_cmd_error(cmd,
SCST_LOAD_SENSE(scst_sense_invalid_opcode));
goto out_xmit;
#endif
} else {
TRACE(TRACE_SCSI, "op_name <%s>, direction=%d (expected %d, "
"set %s), transfer_len=%d (expected len %d), flags=%d",
@@ -349,13 +350,25 @@ static int scst_parse_cmd(struct scst_cmd *cmd)
cdb_info.transfer_len, cmd->expected_transfer_len,
cdb_info.flags);
/* Restore (most probably) lost CDB length */
cmd->cdb_len = cdb_info.cdb_len;
cmd->data_direction = cdb_info.direction;
if (!(cdb_info.flags & SCST_UNKNOWN_LENGTH))
if (unlikely((cdb_info.flags & SCST_UNKNOWN_LENGTH) != 0)) {
if (scst_cmd_is_expected_set(cmd)) {
/*
* Command data length can't be easily
* determined from the CDB. Get it from
* the supplied expected value, but
* limit it to some reasonable value (50MB).
*/
cmd->bufflen = min(cmd->expected_transfer_len,
50*1024*1024);
} else
cmd->bufflen = 0;
} else
cmd->bufflen = cdb_info.transfer_len;
/* else cmd->bufflen remained as it was inited in 0 */
/* Restore (likely) lost CDB length */
cmd->cdb_len = cdb_info.cdb_len;
}
if (unlikely(cmd->cdb[cmd->cdb_len - 1] & CONTROL_BYTE_NACA_BIT)) {
@@ -402,25 +415,14 @@ call_parse:
if (state == SCST_CMD_STATE_DEFAULT)
state = SCST_CMD_STATE_PREPARE_SPACE;
}
else
} else
state = SCST_CMD_STATE_PREPARE_SPACE;
if (scst_cmd_is_expected_set(cmd)) {
if (cmd->expected_transfer_len < cmd->bufflen) {
TRACE(TRACE_SCSI, "cmd->expected_transfer_len(%d) < "
"cmd->bufflen(%d), using expected_transfer_len "
"instead", cmd->expected_transfer_len,
cmd->bufflen);
cmd->bufflen = cmd->expected_transfer_len;
}
}
if (cmd->data_len == -1)
cmd->data_len = cmd->bufflen;
if (cmd->data_buf_alloced && (orig_bufflen > cmd->bufflen)) {
PRINT_ERROR_PR("Target driver supplied data buffer (size %d), "
if (cmd->data_buf_alloced && unlikely((orig_bufflen > cmd->bufflen))) {
PRINT_ERROR_PR("Dev handler supplied data buffer (size %d), "
"is less, than required (size %d)", cmd->bufflen,
orig_bufflen);
goto out_error;
@@ -448,6 +450,42 @@ call_parse:
}
#endif
if (scst_cmd_is_expected_set(cmd)) {
#ifdef USE_EXPECTED_VALUES
# ifdef EXTRACHECKS
if ((cmd->data_direction != cmd->expected_data_direction) ||
(cmd->bufflen != cmd->expected_transfer_len)) {
PRINT_ERROR_PR("Expected values don't match decoded ones: "
"data_direction %d, expected_data_direction %d, "
"bufflen %d, expected_transfer_len %d",
cmd->data_direction, cmd->expected_data_direction,
cmd->bufflen, cmd->expected_transfer_len);
}
# endif
cmd->data_direction = cmd->expected_data_direction;
cmd->bufflen = cmd->expected_transfer_len;
#else
if (unlikely(cmd->data_direction != cdb_info.direction)) {
PRINT_ERROR_PR("Expected data direction %d for opcode "
"0x%02x (handler %s, target %s) doesn't match "
"decoded value %d", cmd->data_direction,
cmd->cdb[0], dev->handler->name,
cmd->tgtt->name, cdb_info.direction);
scst_set_cmd_error(cmd,
SCST_LOAD_SENSE(scst_sense_invalid_message));
goto out_dev_done;
}
if (unlikely(cmd->bufflen != cmd->expected_transfer_len)) {
PRINT_INFO_PR("Warning: expected transfer length %d for "
"opcode 0x%02x (handler %s, target %s) doesn't "
"match decoded value %d. Faulty initiator?",
cmd->expected_transfer_len, cmd->cdb[0],
dev->handler->name, cmd->tgtt->name,
cmd->bufflen);
}
#endif
}
switch (state) {
case SCST_CMD_STATE_PREPARE_SPACE:
case SCST_CMD_STATE_DEV_PARSE:
@@ -488,6 +526,10 @@ out:
out_error:
/* dev_done() will be called as part of the regular cmd's finish */
scst_set_cmd_error(cmd, SCST_LOAD_SENSE(scst_sense_hardw_error));
#ifndef USE_EXPECTED_VALUES
out_dev_done:
#endif
cmd->state = SCST_CMD_STATE_DEV_DONE;
res = SCST_CMD_STATE_RES_CONT_SAME;
goto out;
@@ -498,8 +540,6 @@ out_xmit:
goto out;
}
static int scst_prepare_space(struct scst_cmd *cmd)
{
int r = 0, res = SCST_CMD_STATE_RES_CONT_SAME;