Big CDBs handling improvements.

git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@2295 d57e44dd-8a1f-0410-8b47-8ef2f437770f

doc/scst_user_spec.txt

@@ -2,7 +2,7 @@
 
 	  USER SPACE INTERFACE DESCRIPTION.
 
-		   Version 2.0.0
+		   Version 2.1.0
 
 
 		I. Description.
@@ -384,7 +384,6 @@ struct scst_user_scsi_cmd_parse
 
 	uint8_t cdb[SCST_MAX_CDB_SIZE];
 	uint16_t cdb_len;
-	uint16_t ext_cdb_len;
 
 	uint32_t timeout;
 	int32_t bufflen;
@@ -411,9 +410,6 @@ where:
 
  - cdb_len - SCSI CDB length
 
- - ext_cdb_len - size of extended CDB, see SCST_USER_GET_EXTENDED_CDB
-   for more details
-
  - timeout - CDB execution timeout
 
  - bufflen - command's buffer length
@@ -499,7 +495,6 @@ struct scst_user_scsi_cmd_alloc_mem
 
 	uint8_t cdb[SCST_MAX_CDB_SIZE];
 	uint16_t cdb_len;
-	uint16_t ext_cdb_len;
 
 	int32_t alloc_len;
 
@@ -517,9 +512,6 @@ where:
 
  - cdb_len - SCSI CDB length
 
- - ext_cdb_len - size of extended CDB, see SCST_USER_GET_EXTENDED_CDB
-   for more details
-
  - alloc_len - command's buffer length
 
  - queue_type - SCSI task attribute (queue type )
@@ -602,7 +594,6 @@ struct scst_user_scsi_cmd_exec
 
 	uint8_t cdb[SCST_MAX_CDB_SIZE];
 	uint16_t cdb_len;
-	uint16_t ext_cdb_len;
 
 	int32_t data_len;
 	int32_t bufflen;
@@ -631,9 +622,6 @@ where:
 
  - cdb_len - SCSI CDB length
 
- - ext_cdb_len - size of extended CDB, see SCST_USER_GET_EXTENDED_CDB
-   for more details
-
  - data_len - command's data length. Could be different from bufflen for
    commands like VERIFY, which transfer different amount of data, than
    process, or even none of them

fcst/ft_cmd.c

@@ -126,8 +126,7 @@ void ft_cmd_dump(struct scst_cmd *cmd, const char *caller)
 		"host_status %x driver_status %x\n",
 		prefix, cmd->status, cmd->msg_status,
 		cmd->host_status, cmd->driver_status);
-	printk(KERN_INFO "%s cdb_len %d ext_cdb_len %u\n",
-		prefix, cmd->cdb_len, cmd->ext_cdb_len);
+	printk(KERN_INFO "%s cdb_len %d\n", prefix, cmd->cdb_len);
 	snprintf(buf, sizeof(buf), "%s cdb ", prefix);
 	print_hex_dump(KERN_INFO, buf, DUMP_PREFIX_NONE,
 		16, 4, cmd->cdb, SCST_MAX_CDB_SIZE, 0);

scst/include/scst.h

@@ -1768,11 +1768,9 @@ struct scst_cmd {
 
 	uint32_t tgt_sn; /* SN set by target driver (for TM purposes) */
 
-	/* CDB and its len */
-	uint8_t cdb[SCST_MAX_CDB_SIZE];
+	uint8_t *cdb; /* Pointer on CDB. Points on cdb_buf for small CDBs. */
 	unsigned short cdb_len;
-	unsigned short ext_cdb_len;
-	uint8_t *ext_cdb;
+	uint8_t cdb_buf[SCST_MAX_CDB_SIZE];
 
 	enum scst_cdb_flags op_flags;
 	const char *op_name;
@@ -2768,25 +2766,8 @@ static inline unsigned int scst_cmd_get_cdb_len(struct scst_cmd *cmd)
 	return cmd->cdb_len;
 }
 
-/* Returns cmd's extended CDB */
-static inline const uint8_t *scst_cmd_get_ext_cdb(struct scst_cmd *cmd)
-{
-	return cmd->ext_cdb;
-}
-
-/* Returns cmd's extended CDB length */
-static inline unsigned int scst_cmd_get_ext_cdb_len(struct scst_cmd *cmd)
-{
-	return cmd->ext_cdb_len;
-}
-
-/* Sets cmd's extended CDB and its length */
-static inline void scst_cmd_set_ext_cdb(struct scst_cmd *cmd,
-	uint8_t *ext_cdb, unsigned int ext_cdb_len)
-{
-	cmd->ext_cdb = ext_cdb;
-	cmd->ext_cdb_len = ext_cdb_len;
-}
+void scst_cmd_set_ext_cdb(struct scst_cmd *cmd,
+	uint8_t *ext_cdb, unsigned int ext_cdb_len);
 
 /* Returns cmd's session */
 static inline struct scst_session *scst_cmd_get_session(struct scst_cmd *cmd)

scst/include/scst_user.h

@@ -122,7 +122,6 @@ struct scst_user_scsi_cmd_parse {
 
 	uint8_t cdb[SCST_MAX_CDB_SIZE];
 	uint16_t cdb_len;
-	uint16_t ext_cdb_len;
 
 	int32_t timeout;
 	int32_t bufflen;
@@ -146,7 +145,6 @@ struct scst_user_scsi_cmd_alloc_mem {
 
 	uint8_t cdb[SCST_MAX_CDB_SIZE];
 	uint16_t cdb_len;
-	uint16_t ext_cdb_len;
 
 	int32_t alloc_len;
 
@@ -161,7 +159,6 @@ struct scst_user_scsi_cmd_exec {
 
 	uint8_t cdb[SCST_MAX_CDB_SIZE];
 	uint16_t cdb_len;
-	uint16_t ext_cdb_len;
 
 	int32_t data_len;
 	int32_t bufflen;

scst/src/dev_handlers/scst_user.c

@@ -696,9 +696,9 @@ static int dev_user_alloc_space(struct scst_user_cmd *ucmd)
 	ucmd->user_cmd.cmd_h = ucmd->h;
 	ucmd->user_cmd.subcode = SCST_USER_ALLOC_MEM;
 	ucmd->user_cmd.alloc_cmd.sess_h = (unsigned long)cmd->tgt_dev;
-	memcpy(ucmd->user_cmd.alloc_cmd.cdb, cmd->cdb, cmd->cdb_len);
+	memcpy(ucmd->user_cmd.alloc_cmd.cdb, cmd->cdb,
+		min_t(int, SCST_MAX_CDB_SIZE, cmd->cdb_len));
 	ucmd->user_cmd.alloc_cmd.cdb_len = cmd->cdb_len;
-	ucmd->user_cmd.alloc_cmd.ext_cdb_len = cmd->ext_cdb_len;
 	ucmd->user_cmd.alloc_cmd.alloc_len = ucmd->buff_cached ?
 		(cmd->sg_cnt << PAGE_SHIFT) : cmd->bufflen;
 	ucmd->user_cmd.alloc_cmd.queue_type = cmd->queue_type;
@@ -823,9 +823,9 @@ static int dev_user_parse(struct scst_cmd *cmd)
 		ucmd->user_cmd.cmd_h = ucmd->h;
 		ucmd->user_cmd.subcode = SCST_USER_PARSE;
 		ucmd->user_cmd.parse_cmd.sess_h = (unsigned long)cmd->tgt_dev;
-		memcpy(ucmd->user_cmd.parse_cmd.cdb, cmd->cdb, cmd->cdb_len);
+		memcpy(ucmd->user_cmd.parse_cmd.cdb, cmd->cdb,
+			min_t(int, SCST_MAX_CDB_SIZE, cmd->cdb_len));
 		ucmd->user_cmd.parse_cmd.cdb_len = cmd->cdb_len;
-		ucmd->user_cmd.parse_cmd.ext_cdb_len = cmd->ext_cdb_len;
 		ucmd->user_cmd.parse_cmd.timeout = cmd->timeout / HZ;
 		ucmd->user_cmd.parse_cmd.bufflen = cmd->bufflen;
 		ucmd->user_cmd.parse_cmd.out_bufflen = cmd->out_bufflen;
@@ -943,17 +943,15 @@ static int dev_user_exec(struct scst_cmd *cmd)
 	if (cmd->data_direction & SCST_DATA_WRITE)
 		dev_user_flush_dcache(ucmd);
 
-	BUILD_BUG_ON(sizeof(ucmd->user_cmd.exec_cmd.cdb) != sizeof(cmd->cdb));
-
 	ucmd->user_cmd_payload_len =
 		offsetof(struct scst_user_get_cmd, exec_cmd) +
 		sizeof(ucmd->user_cmd.exec_cmd);
 	ucmd->user_cmd.cmd_h = ucmd->h;
 	ucmd->user_cmd.subcode = SCST_USER_EXEC;
 	ucmd->user_cmd.exec_cmd.sess_h = (unsigned long)cmd->tgt_dev;
-	memcpy(ucmd->user_cmd.exec_cmd.cdb, cmd->cdb, cmd->cdb_len);
+	memcpy(ucmd->user_cmd.exec_cmd.cdb, cmd->cdb,
+		min_t(int, SCST_MAX_CDB_SIZE, cmd->cdb_len));
 	ucmd->user_cmd.exec_cmd.cdb_len = cmd->cdb_len;
-	ucmd->user_cmd.exec_cmd.ext_cdb_len = cmd->ext_cdb_len;
 	ucmd->user_cmd.exec_cmd.bufflen = cmd->bufflen;
 	ucmd->user_cmd.exec_cmd.data_len = cmd->data_len;
 	ucmd->user_cmd.exec_cmd.pbuf = ucmd->ubuff;
@@ -1292,7 +1290,7 @@ static int dev_user_process_reply_parse(struct scst_user_cmd *ucmd,
 	if (unlikely((preply->bufflen < 0) || (preply->data_len < 0)))
 		goto out_inval;
 
-	if (unlikely(preply->cdb_len > SCST_MAX_CDB_SIZE))
+	if (unlikely(preply->cdb_len > cmd->cdb_len))
 		goto out_inval;
 
 	TRACE_DBG("ucmd %p, queue_type %x, data_direction, %x, bufflen %d, "
@@ -1740,12 +1738,18 @@ static int dev_user_get_ext_cdb(struct file *file, void __user *arg)
 	if (cmd == NULL)
 		goto out_put;
 
-	if (cmd->ext_cdb == NULL)
+	BUILD_BUG_ON(sizeof(cmd->cdb_buf) != SCST_MAX_CDB_SIZE);
+
+	if (cmd->cdb_len <= SCST_MAX_CDB_SIZE)
 		goto out_cmd_put;
 
-	TRACE_BUFFER("EXT CDB", cmd->ext_cdb, cmd->ext_cdb_len);
+	EXTRACHECKS_BUG_ON(cmd->cdb_buf == cmd->cdb_buf);
+
+	TRACE_BUFFER("EXT CDB", &cmd->cdb[sizeof(cmd->cdb_buf)],
+		cmd->cdb_len - sizeof(cmd->cdb_buf));
 	rc = copy_to_user((void __user *)(unsigned long)get.ext_cdb_buffer,
-		cmd->ext_cdb, cmd->ext_cdb_len);
+		&cmd->cdb[sizeof(cmd->cdb_buf)],
+		cmd->cdb_len - sizeof(cmd->cdb_buf));
 	if (unlikely(rc != 0)) {
 		PRINT_ERROR("Failed to copy to user %d bytes", rc);
 		res = -EFAULT;

scst/src/scst_lib.c

@@ -44,11 +44,9 @@
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 30)
 struct scsi_io_context {
-	unsigned int full_cdb_used:1;
 	void *data;
 	void (*done)(void *data, char *sense, int result, int resid);
 	char sense[SCST_SENSE_BUFFERSIZE];
-	unsigned char full_cdb[0];
 };
 static struct kmem_cache *scsi_io_context_cache;
 #endif
@@ -4159,6 +4157,39 @@ void scst_cmd_put(struct scst_cmd *cmd)
 }
 EXPORT_SYMBOL(scst_cmd_put);
 
+/**
+ * scst_cmd_set_ext_cdb() - sets cmd's extended CDB and its length
+ */
+void scst_cmd_set_ext_cdb(struct scst_cmd *cmd,
+	uint8_t *ext_cdb, unsigned int ext_cdb_len)
+{
+	TRACE_ENTRY();
+
+	if ((cmd->cdb_len + ext_cdb_len) <= sizeof(cmd->cdb_buf))
+		goto copy;
+
+	cmd->cdb = kmalloc(cmd->cdb_len + ext_cdb_len, GFP_ATOMIC);
+	if (cmd->cdb == NULL)
+		goto out_err;
+
+	memcpy(cmd->cdb, cmd->cdb_buf, cmd->cdb_len);
+
+copy:
+	memcpy(&cmd->cdb[cmd->cdb_len], ext_cdb, ext_cdb_len);
+
+	cmd->cdb_len = cmd->cdb_len + ext_cdb_len;
+
+out:
+	TRACE_EXIT();
+	return;
+
+out_err:
+	cmd->cdb = cmd->cdb_buf;
+	scst_set_busy(cmd);
+	goto out;
+}
+EXPORT_SYMBOL(scst_cmd_set_ext_cdb);
+
 struct scst_cmd *scst_alloc_cmd(gfp_t gfp_mask)
 {
 	struct scst_cmd *cmd;
@@ -4183,6 +4214,7 @@ struct scst_cmd *scst_alloc_cmd(gfp_t gfp_mask)
 	atomic_set(&cmd->cmd_ref, 1);
 	cmd->cmd_threads = &scst_main_cmd_threads;
 	INIT_LIST_HEAD(&cmd->mgmt_cmd_list);
+	cmd->cdb = cmd->cdb_buf;
 	cmd->queue_type = SCST_CMD_QUEUE_SIMPLE;
 	cmd->timeout = SCST_DEFAULT_TIMEOUT;
 	cmd->retries = 0;
@@ -4299,6 +4331,9 @@ void scst_free_cmd(struct scst_cmd *cmd)
 		}
 	}
 
+	if (cmd->cdb != cmd->cdb_buf)
+		kfree(cmd->cdb);
+
 	if (likely(destroy))
 		scst_destroy_put_cmd(cmd);
 
@@ -4736,10 +4771,7 @@ static void scsi_end_async(struct request *req, int error)
 		sioc->done(sioc->data, sioc->sense, req->errors, req->resid_len);
 #endif
 
-	if (!sioc->full_cdb_used)
-		kmem_cache_free(scsi_io_context_cache, sioc);
-	else
-		kfree(sioc);
+	kmem_cache_free(scsi_io_context_cache, sioc);
 
 	__blk_put_request(req->q, req);
 	return;
@@ -4762,29 +4794,10 @@ int scst_scsi_exec_async(struct scst_cmd *cmd, void *data,
 	gfp_t gfp = GFP_KERNEL;
 	int cmd_len = cmd->cdb_len;
 
-	if (cmd->ext_cdb_len == 0) {
-		TRACE_DBG("Simple CDB (cmd_len %d)", cmd_len);
-		sioc = kmem_cache_zalloc(scsi_io_context_cache, gfp);
-		if (sioc == NULL) {
-			res = -ENOMEM;
-			goto out;
-		}
-	} else {
-		cmd_len += cmd->ext_cdb_len;
-
-		TRACE_DBG("Extended CDB (cmd_len %d)", cmd_len);
-
-		sioc = kzalloc(sizeof(*sioc) + cmd_len, gfp);
-		if (sioc == NULL) {
-			res = -ENOMEM;
-			goto out;
-		}
-
-		sioc->full_cdb_used = 1;
-
-		memcpy(sioc->full_cdb, cmd->cdb, cmd->cdb_len);
-		memcpy(&sioc->full_cdb[cmd->cdb_len], cmd->ext_cdb,
-			cmd->ext_cdb_len);
+	sioc = kmem_cache_zalloc(scsi_io_context_cache, gfp);
+	if (sioc == NULL) {
+		res = -ENOMEM;
+		goto out;
 	}
 
 	rq = blk_get_request(q, write, gfp);
@@ -4841,11 +4854,11 @@ done:
 	sioc->done = done;
 
 	rq->cmd_len = cmd_len;
-	if (cmd->ext_cdb_len == 0) {
+	if (rq->cmd_len <= BLK_MAX_CDB) {
 		memset(rq->cmd, 0, BLK_MAX_CDB); /* ATAPI hates garbage after CDB */
 		memcpy(rq->cmd, cmd->cdb, cmd->cdb_len);
 	} else
-		rq->cmd = sioc->full_cdb;
+		rq->cmd = cmd->cdb;
 
 	rq->sense = sioc->sense;
 	rq->sense_len = sizeof(sioc->sense);
@@ -4869,10 +4882,7 @@ out_free_rq:
 	blk_put_request(rq);
 
 out_free_sioc:
-	if (!sioc->full_cdb_used)
-		kmem_cache_free(scsi_io_context_cache, sioc);
-	else
-		kfree(sioc);
+	kmem_cache_free(scsi_io_context_cache, sioc);
 	goto out;
 }
 EXPORT_SYMBOL(scst_scsi_exec_async);

scst/src/scst_targ.c

@@ -327,8 +327,7 @@ void scst_cmd_init_done(struct scst_cmd *cmd,
 		case SCST_SESS_IPH_FAILED:
 			spin_unlock_irqrestore(&sess->sess_list_lock, flags);
 			scst_set_busy(cmd);
-			scst_set_cmd_abnormal_done_state(cmd);
-			goto active;
+			goto set_state;
 		default:
 			sBUG();
 		}
@@ -342,14 +341,17 @@ void scst_cmd_init_done(struct scst_cmd *cmd,
 		PRINT_ERROR("%s", "Wrong CDB len 0, finishing cmd");
 		scst_set_cmd_error(cmd,
 			   SCST_LOAD_SENSE(scst_sense_invalid_opcode));
-		scst_set_cmd_abnormal_done_state(cmd);
-		goto active;
 	}
 
 	if (unlikely(cmd->queue_type >= SCST_CMD_QUEUE_ACA)) {
 		PRINT_ERROR("Unsupported queue type %d", cmd->queue_type);
 		scst_set_cmd_error(cmd,
 			SCST_LOAD_SENSE(scst_sense_invalid_message));
+	}
+
+set_state:
+	if (unlikely(cmd->status != SAM_STAT_GOOD)) {
+		scst_set_cmd_abnormal_done_state(cmd);
 		goto active;
 	}
 

usr/fileio/common.c

@@ -515,11 +515,10 @@ static int do_alloc_mem(struct vdisk_cmd *vcmd)
 	TRACE_ENTRY();
 
 	TRACE_MEM("Alloc mem (cmd %d, sess_h %"PRIx64", cdb_len %d, "
-		"ext_cdb_len %d, alloc_len %d, queue_type %d, data_direction "
-		"%d)", cmd->cmd_h, cmd->alloc_cmd.sess_h,
-		cmd->alloc_cmd.cdb_len, cmd->alloc_cmd.ext_cdb_len,
-		cmd->alloc_cmd.alloc_len, cmd->alloc_cmd.queue_type,
-		cmd->alloc_cmd.data_direction);
+		"alloc_len %d, queue_type %d, data_direction %d)",
+		cmd->cmd_h, cmd->alloc_cmd.sess_h,
+		cmd->alloc_cmd.cdb_len, cmd->alloc_cmd.alloc_len,
+		cmd->alloc_cmd.queue_type, cmd->alloc_cmd.data_direction);
 
 	TRACE_BUFF_FLAG(TRACE_MEMORY, "CDB", cmd->alloc_cmd.cdb,
 		cmd->alloc_cmd.cdb_len);