On Mon, 2014-11-24 at 23:05 -0500, Douglas Gilbert wrote:
> From: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
> Date: Mon, 24 Nov 2014 20:46:29 -0500
> Subject: [PATCH 4/5] change SCSI command parser to table driven
>
> The existing 'big switch' parser in queuecommand() is changed to
> a table driven parser. The old and new queuecommand() were moved
> in the source so diff would not shuffle them. Apart from the new
> tables most other changes are refactoring existing response code
> to be more easily called out of the table parser. The 'strict'
> parameter is added so that cdb_s can be checked for non-zero
> values in parts of the cdb that are reserved. Some other changes
> include: tweak request sense response when D_SENSE differs; support
> NDOB in Write Same(16); and fix crash in Get LBA Status when LBP
> was inactive.
> ---
> drivers/scsi/scsi_debug.c | 1391 +++++++++++++++++++++++++++------------------
> 1 file changed, 833 insertions(+), 558 deletions(-)
There's a massive amount of apparently unnecessary code motion in this
patch (like moving the whole of scsi_debug_queuecommand). This caused a
reject today on the merger of the drivers-for-3.19 with core-for-3.19
(conflict with the queue depth reason elimination). Can we not do large
pieces of code motion unless there's good reason.
Perhaps, also, it's time to reconsider whether we do actually need a
core and drivers branch separation.
The merge was trivial but I've attached it below, just in case.
James
---
commit 5055a627a05df34f88265095cdf5da01b2b22a7a
Merge: 7ec0579 38d5c83
Author: James Bottomley <JBottomley@xxxxxxxxxxxxx>
Date: Tue Nov 25 06:54:28 2014 -0800
Merge remote-tracking branch 'scsi-queue/drivers-for-3.19' into for-next
Conflicts:
drivers/scsi/scsi_debug.c
diff --cc drivers/scsi/scsi_debug.c
index 378e0aa,aa4b6b8..6183419
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@@ -4083,396 -4957,9 +4957,394 @@@ static void sdebug_remove_adapter(void
}
static int
+scsi_debug_queuecommand(struct scsi_cmnd *SCpnt)
+{
+ unsigned char *cmd = SCpnt->cmnd;
+ int len, k;
+ unsigned int num;
+ unsigned long long lba;
+ u32 ei_lba;
+ int errsts = 0;
+ int target = SCpnt->device->id;
+ struct sdebug_dev_info *devip = NULL;
+ int inj_recovered = 0;
+ int inj_transport = 0;
+ int inj_dif = 0;
+ int inj_dix = 0;
+ int inj_short = 0;
+ int delay_override = 0;
+ int unmap = 0;
+
+ scsi_set_resid(SCpnt, 0);
+ if ((SCSI_DEBUG_OPT_NOISE & scsi_debug_opts) &&
+ !(SCSI_DEBUG_OPT_NO_CDB_NOISE & scsi_debug_opts)) {
+ char b[120];
+ int n;
+
+ len = SCpnt->cmd_len;
+ if (len > 32)
+ strcpy(b, "too long, over 32 bytes");
+ else {
+ for (k = 0, n = 0; k < len; ++k)
+ n += scnprintf(b + n, sizeof(b) - n, "%02x ",
+ (unsigned int)cmd[k]);
+ }
+ sdev_printk(KERN_INFO, SCpnt->device, "%s: cmd %s\n", my_name,
+ b);
+ }
+
+ if ((SCpnt->device->lun >= scsi_debug_max_luns) &&
+ (SCpnt->device->lun != SAM2_WLUN_REPORT_LUNS))
+ return schedule_resp(SCpnt, NULL, DID_NO_CONNECT << 16, 0);
+ devip = devInfoReg(SCpnt->device);
+ if (NULL == devip)
+ return schedule_resp(SCpnt, NULL, DID_NO_CONNECT << 16, 0);
+
+ if ((scsi_debug_every_nth != 0) &&
+ (atomic_inc_return(&sdebug_cmnd_count) >=
+ abs(scsi_debug_every_nth))) {
+ atomic_set(&sdebug_cmnd_count, 0);
+ if (scsi_debug_every_nth < -1)
+ scsi_debug_every_nth = -1;
+ if (SCSI_DEBUG_OPT_TIMEOUT & scsi_debug_opts)
+ return 0; /* ignore command causing timeout */
+ else if (SCSI_DEBUG_OPT_MAC_TIMEOUT & scsi_debug_opts &&
+ scsi_medium_access_command(SCpnt))
+ return 0; /* time out reads and writes */
+ else if (SCSI_DEBUG_OPT_RECOVERED_ERR & scsi_debug_opts)
+ inj_recovered = 1; /* to reads and writes below */
+ else if (SCSI_DEBUG_OPT_TRANSPORT_ERR & scsi_debug_opts)
+ inj_transport = 1; /* to reads and writes below */
+ else if (SCSI_DEBUG_OPT_DIF_ERR & scsi_debug_opts)
+ inj_dif = 1; /* to reads and writes below */
+ else if (SCSI_DEBUG_OPT_DIX_ERR & scsi_debug_opts)
+ inj_dix = 1; /* to reads and writes below */
+ else if (SCSI_DEBUG_OPT_SHORT_TRANSFER & scsi_debug_opts)
+ inj_short = 1;
+ }
+
+ if (devip->wlun) {
+ switch (*cmd) {
+ case INQUIRY:
+ case REQUEST_SENSE:
+ case TEST_UNIT_READY:
+ case REPORT_LUNS:
+ break; /* only allowable wlun commands */
+ default:
+ if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
+ printk(KERN_INFO "scsi_debug: Opcode: 0x%x "
+ "not supported for wlun\n", *cmd);
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_OPCODE, 0);
+ errsts = check_condition_result;
+ return schedule_resp(SCpnt, devip, errsts, 0);
+ }
+ }
+
+ switch (*cmd) {
+ case INQUIRY: /* mandatory, ignore unit attention */
+ delay_override = 1;
+ errsts = resp_inquiry(SCpnt, target, devip);
+ break;
+ case REQUEST_SENSE: /* mandatory, ignore unit attention */
+ delay_override = 1;
+ errsts = resp_requests(SCpnt, devip);
+ break;
+ case REZERO_UNIT: /* actually this is REWIND for SSC */
+ case START_STOP:
+ errsts = resp_start_stop(SCpnt, devip);
+ break;
+ case ALLOW_MEDIUM_REMOVAL:
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ if (errsts)
+ break;
+ if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
+ printk(KERN_INFO "scsi_debug: Medium removal %s\n",
+ cmd[4] ? "inhibited" : "enabled");
+ break;
+ case SEND_DIAGNOSTIC: /* mandatory */
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ break;
+ case TEST_UNIT_READY: /* mandatory */
+ /* delay_override = 1; */
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ break;
+ case RESERVE:
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ break;
+ case RESERVE_10:
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ break;
+ case RELEASE:
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ break;
+ case RELEASE_10:
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ break;
+ case READ_CAPACITY:
+ errsts = resp_readcap(SCpnt, devip);
+ break;
+ case SERVICE_ACTION_IN_16:
+ if (cmd[1] == SAI_READ_CAPACITY_16)
+ errsts = resp_readcap16(SCpnt, devip);
+ else if (cmd[1] == SAI_GET_LBA_STATUS) {
+
+ if (scsi_debug_lbp() == 0) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_COMMAND_OPCODE, 0);
+ errsts = check_condition_result;
+ } else
+ errsts = resp_get_lba_status(SCpnt, devip);
+ } else {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_OPCODE, 0);
+ errsts = check_condition_result;
+ }
+ break;
+ case MAINTENANCE_IN:
+ if (MI_REPORT_TARGET_PGS != cmd[1]) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ }
+ errsts = resp_report_tgtpgs(SCpnt, devip);
+ break;
+ case READ_16:
+ case READ_12:
+ case READ_10:
+ /* READ{10,12,16} and DIF Type 2 are natural enemies */
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+ cmd[1] & 0xe0) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_COMMAND_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ }
+
+ if ((scsi_debug_dif == SD_DIF_TYPE1_PROTECTION ||
+ scsi_debug_dif == SD_DIF_TYPE3_PROTECTION) &&
+ (cmd[1] & 0xe0) == 0)
+ printk(KERN_ERR "Unprotected RD/WR to DIF device\n");
+
+ /* fall through */
+ case READ_6:
+read:
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ if (errsts)
+ break;
+ if (scsi_debug_fake_rw)
+ break;
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+
+ if (inj_short)
+ num /= 2;
+
+ errsts = resp_read(SCpnt, lba, num, ei_lba);
+ if (inj_recovered && (0 == errsts)) {
+ mk_sense_buffer(SCpnt, RECOVERED_ERROR,
+ THRESHOLD_EXCEEDED, 0);
+ errsts = check_condition_result;
+ } else if (inj_transport && (0 == errsts)) {
+ mk_sense_buffer(SCpnt, ABORTED_COMMAND,
+ TRANSPORT_PROBLEM, ACK_NAK_TO);
+ errsts = check_condition_result;
+ } else if (inj_dif && (0 == errsts)) {
+ /* Logical block guard check failed */
+ mk_sense_buffer(SCpnt, ABORTED_COMMAND, 0x10, 1);
+ errsts = illegal_condition_result;
+ } else if (inj_dix && (0 == errsts)) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST, 0x10, 1);
+ errsts = illegal_condition_result;
+ }
+ break;
+ case REPORT_LUNS: /* mandatory, ignore unit attention */
+ delay_override = 1;
+ errsts = resp_report_luns(SCpnt, devip);
+ break;
+ case VERIFY: /* 10 byte SBC-2 command */
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ break;
+ case WRITE_16:
+ case WRITE_12:
+ case WRITE_10:
+ /* WRITE{10,12,16} and DIF Type 2 are natural enemies */
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+ cmd[1] & 0xe0) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_COMMAND_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ }
+
+ if ((scsi_debug_dif == SD_DIF_TYPE1_PROTECTION ||
+ scsi_debug_dif == SD_DIF_TYPE3_PROTECTION) &&
+ (cmd[1] & 0xe0) == 0)
+ printk(KERN_ERR "Unprotected RD/WR to DIF device\n");
+
+ /* fall through */
+ case WRITE_6:
+write:
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ if (errsts)
+ break;
+ if (scsi_debug_fake_rw)
+ break;
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+ errsts = resp_write(SCpnt, lba, num, ei_lba);
+ if (inj_recovered && (0 == errsts)) {
+ mk_sense_buffer(SCpnt, RECOVERED_ERROR,
+ THRESHOLD_EXCEEDED, 0);
+ errsts = check_condition_result;
+ } else if (inj_dif && (0 == errsts)) {
+ mk_sense_buffer(SCpnt, ABORTED_COMMAND, 0x10, 1);
+ errsts = illegal_condition_result;
+ } else if (inj_dix && (0 == errsts)) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST, 0x10, 1);
+ errsts = illegal_condition_result;
+ }
+ break;
+ case WRITE_SAME_16:
+ case WRITE_SAME:
+ if (cmd[1] & 0x8) {
+ if ((*cmd == WRITE_SAME_16 && scsi_debug_lbpws == 0) ||
+ (*cmd == WRITE_SAME && scsi_debug_lbpws10 == 0)) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_FIELD_IN_CDB, 0);
+ errsts = check_condition_result;
+ } else
+ unmap = 1;
+ }
+ if (errsts)
+ break;
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ if (errsts)
+ break;
+ if (scsi_debug_fake_rw)
+ break;
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+ errsts = resp_write_same(SCpnt, lba, num, ei_lba, unmap);
+ break;
+ case UNMAP:
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ if (errsts)
+ break;
+ if (scsi_debug_fake_rw)
+ break;
+
+ if (scsi_debug_unmap_max_desc == 0 || scsi_debug_lbpu == 0) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_COMMAND_OPCODE, 0);
+ errsts = check_condition_result;
+ } else
+ errsts = resp_unmap(SCpnt, devip);
+ break;
+ case MODE_SENSE:
+ case MODE_SENSE_10:
+ errsts = resp_mode_sense(SCpnt, target, devip);
+ break;
+ case MODE_SELECT:
+ errsts = resp_mode_select(SCpnt, 1, devip);
+ break;
+ case MODE_SELECT_10:
+ errsts = resp_mode_select(SCpnt, 0, devip);
+ break;
+ case LOG_SENSE:
+ errsts = resp_log_sense(SCpnt, devip);
+ break;
+ case SYNCHRONIZE_CACHE:
+ delay_override = 1;
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ break;
+ case WRITE_BUFFER:
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ break;
+ case XDWRITEREAD_10:
+ if (!scsi_bidi_cmnd(SCpnt)) {
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_FIELD_IN_CDB, 0);
+ errsts = check_condition_result;
+ break;
+ }
+
+ errsts = check_readiness(SCpnt, UAS_TUR, devip);
+ if (errsts)
+ break;
+ if (scsi_debug_fake_rw)
+ break;
+ get_data_transfer_info(cmd, &lba, &num, &ei_lba);
+ errsts = resp_read(SCpnt, lba, num, ei_lba);
+ if (errsts)
+ break;
+ errsts = resp_write(SCpnt, lba, num, ei_lba);
+ if (errsts)
+ break;
+ errsts = resp_xdwriteread(SCpnt, lba, num, devip);
+ break;
+ case VARIABLE_LENGTH_CMD:
+ if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION) {
+
+ if ((cmd[10] & 0xe0) == 0)
+ printk(KERN_ERR
+ "Unprotected RD/WR to DIF device\n");
+
+ if (cmd[9] == READ_32) {
+ BUG_ON(SCpnt->cmd_len < 32);
+ goto read;
+ }
+
+ if (cmd[9] == WRITE_32) {
+ BUG_ON(SCpnt->cmd_len < 32);
+ goto write;
+ }
+ }
+
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_FIELD_IN_CDB, 0);
+ errsts = check_condition_result;
+ break;
+ case 0x85:
+ if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
+ sdev_printk(KERN_INFO, SCpnt->device,
+ "%s: ATA PASS-THROUGH(16) not supported\n", my_name);
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST,
+ INVALID_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ default:
+ if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
+ sdev_printk(KERN_INFO, SCpnt->device,
+ "%s: Opcode: 0x%x not supported\n",
+ my_name, *cmd);
+ errsts = check_readiness(SCpnt, UAS_ONLY, devip);
+ if (errsts)
+ break; /* Unit attention takes precedence */
+ mk_sense_buffer(SCpnt, ILLEGAL_REQUEST, INVALID_OPCODE, 0);
+ errsts = check_condition_result;
+ break;
+ }
+ return schedule_resp(SCpnt, devip, errsts,
+ (delay_override ? 0 : scsi_debug_delay));
+}
+
+static int
+sdebug_queuecommand_lock_or_not(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
+{
+ if (scsi_debug_host_lock) {
+ unsigned long iflags;
+ int rc;
+
+ spin_lock_irqsave(shost->host_lock, iflags);
+ rc = scsi_debug_queuecommand(cmd);
+ spin_unlock_irqrestore(shost->host_lock, iflags);
+ return rc;
+ } else
+ return scsi_debug_queuecommand(cmd);
+}
+
- static int
- sdebug_change_qdepth(struct scsi_device *sdev, int qdepth, int reason)
+ sdebug_change_qdepth(struct scsi_device *sdev, int qdepth)
{
int num_in_q = 0;
- int bad = 0;
unsigned long iflags;
struct sdebug_dev_info *devip;
--
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
