diff mbox series

[14/14] target: remove target_submit_cmd_map_sgls

Message ID 20210211122728.31721-15-michael.christie@oracle.com (mailing list archive)
State New, archived
Headers show
Series [01/14] target: move t_task_cdb initialization | expand

Commit Message

Mike Christie Feb. 11, 2021, 12:27 p.m. UTC
Convert target_submit_cmd to do its own calls and then reomve
target_submit_cmd_map_sgls since no one uses it.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_transport.c | 69 ++++----------------------
 include/target/target_core_fabric.h    |  6 +--
 2 files changed, 11 insertions(+), 64 deletions(-)

Comments

Christoph Hellwig Feb. 11, 2021, 3:17 p.m. UTC | #1
> + * This function must only be used by drivers that do their own
> + * sync during shutdown and does not use target_stop_session. If there
> + * is a failure this function will call into the fabric driver's
> + * queue_status with a CHECK_CONDITION.
>   */
> -int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
> +void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>  		unsigned char *cdb, unsigned char *sense, u64 unpacked_lun,
>  		u32 data_length, int task_attr, int data_dir, int flags)
>  {
> +	target_init_cmd(se_cmd, se_sess, sense, unpacked_lun, data_length,
> +			task_attr, data_dir, flags);
> +	target_submit_prep(se_cmd, cdb, NULL, 0, NULL, 0, NULL, 0);

Do we want a WARN_ON_ONCE here to catch the case where the API is
misued?
Mike Christie Feb. 11, 2021, 6:11 p.m. UTC | #2
On 2/11/21 9:17 AM, Christoph Hellwig wrote:
>> + * This function must only be used by drivers that do their own
>> + * sync during shutdown and does not use target_stop_session. If there
>> + * is a failure this function will call into the fabric driver's
>> + * queue_status with a CHECK_CONDITION.
>>   */
>> -int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>> +void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>>  		unsigned char *cdb, unsigned char *sense, u64 unpacked_lun,
>>  		u32 data_length, int task_attr, int data_dir, int flags)
>>  {
>> +	target_init_cmd(se_cmd, se_sess, sense, unpacked_lun, data_length,
>> +			task_attr, data_dir, flags);
>> +	target_submit_prep(se_cmd, cdb, NULL, 0, NULL, 0, NULL, 0);
> 
> Do we want a WARN_ON_ONCE here to catch the case where the API is
> misued?

It would be nice, but I couldn't figure out how to detect it here.

For example if the driver calls scsi_remove_device to prevent incoming
IO and flush running IO and so they don't need target_stop_session,
then I have no way to detect that above. The driver writer had to know.
diff mbox series

Patch

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index a82b7da09872..ad30a99a5cb2 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1751,57 +1751,6 @@  void target_submit(struct se_cmd *se_cmd)
 }
 EXPORT_SYMBOL_GPL(target_submit);
 
-/**
- * target_submit_cmd_map_sgls - lookup unpacked lun and submit uninitialized
- *					se_cmd + use pre-allocated SGL memory.
- *
- * @se_cmd: command descriptor to submit
- * @se_sess: associated se_sess for endpoint
- * @cdb: pointer to SCSI CDB
- * @sense: pointer to SCSI sense buffer
- * @unpacked_lun: unpacked LUN to reference for struct se_lun
- * @data_length: fabric expected data transfer length
- * @task_attr: SAM task attribute
- * @data_dir: DMA data direction
- * @flags: flags for command submission from target_sc_flags_tables
- * @sgl: struct scatterlist memory for unidirectional mapping
- * @sgl_count: scatterlist count for unidirectional mapping
- * @sgl_bidi: struct scatterlist memory for bidirectional READ mapping
- * @sgl_bidi_count: scatterlist count for bidirectional READ mapping
- * @sgl_prot: struct scatterlist memory protection information
- * @sgl_prot_count: scatterlist count for protection information
- *
- * Task tags are supported if the caller has set @se_cmd->tag.
- *
- * Returns non zero to signal active I/O shutdown failure.  All other
- * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
- * but still return zero here.
- *
- * This may only be called from process context, and also currently
- * assumes internal allocation of fabric payload buffer by target-core.
- */
-int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess,
-		unsigned char *cdb, unsigned char *sense, u64 unpacked_lun,
-		u32 data_length, int task_attr, int data_dir, int flags,
-		struct scatterlist *sgl, u32 sgl_count,
-		struct scatterlist *sgl_bidi, u32 sgl_bidi_count,
-		struct scatterlist *sgl_prot, u32 sgl_prot_count)
-{
-	int rc;
-
-	rc = target_init_cmd(se_cmd, se_sess, sense, unpacked_lun,
-			     data_length, task_attr, data_dir, flags);
-	if (rc < 0)
-		return rc;
-
-
-	target_submit_prep(se_cmd, cdb, sgl, sgl_count, sgl_bidi,
-			   sgl_bidi_count, sgl_prot, sgl_prot_count);
-	target_submit(se_cmd);
-	return 0;
-}
-EXPORT_SYMBOL(target_submit_cmd_map_sgls);
-
 /**
  * target_submit_cmd - lookup unpacked lun and submit uninitialized se_cmd
  *
@@ -1817,22 +1766,24 @@  EXPORT_SYMBOL(target_submit_cmd_map_sgls);
  *
  * Task tags are supported if the caller has set @se_cmd->tag.
  *
- * Returns non zero to signal active I/O shutdown failure.  All other
- * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
- * but still return zero here.
- *
  * This may only be called from process context, and also currently
  * assumes internal allocation of fabric payload buffer by target-core.
  *
  * It also assumes interal target core SGL memory allocation.
+ *
+ * This function must only be used by drivers that do their own
+ * sync during shutdown and does not use target_stop_session. If there
+ * is a failure this function will call into the fabric driver's
+ * queue_status with a CHECK_CONDITION.
  */
-int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
+void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
 		unsigned char *cdb, unsigned char *sense, u64 unpacked_lun,
 		u32 data_length, int task_attr, int data_dir, int flags)
 {
-	return target_submit_cmd_map_sgls(se_cmd, se_sess, cdb, sense,
-			unpacked_lun, data_length, task_attr, data_dir,
-			flags, NULL, 0, NULL, 0, NULL, 0);
+	target_init_cmd(se_cmd, se_sess, sense, unpacked_lun, data_length,
+			task_attr, data_dir, flags);
+	target_submit_prep(se_cmd, cdb, NULL, 0, NULL, 0, NULL, 0);
+	target_submit(se_cmd);
 }
 EXPORT_SYMBOL(target_submit_cmd);
 
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 4b5f6687393a..86b0d4a7df92 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -162,11 +162,7 @@  void	target_submit(struct se_cmd *se_cmd);
 sense_reason_t transport_lookup_cmd_lun(struct se_cmd *);
 sense_reason_t target_cmd_init_cdb(struct se_cmd *, unsigned char *);
 sense_reason_t target_cmd_parse_cdb(struct se_cmd *);
-int	target_submit_cmd_map_sgls(struct se_cmd *, struct se_session *,
-		unsigned char *, unsigned char *, u64, u32, int, int, int,
-		struct scatterlist *, u32, struct scatterlist *, u32,
-		struct scatterlist *, u32);
-int	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
+void	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
 		unsigned char *, u64, u32, int, int, int);
 int	target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
 		unsigned char *sense, u64 unpacked_lun,