From e381fe9e89909ced3d878b8c3c3a9b344c02cbdc Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 6 Jan 2017 11:32:08 +0100 Subject: target/iscsi: Introduce a helper function for TMF translation This patch does not change any functionality. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Cc: Nicholas A. Bellinger Signed-off-by: Nicholas Bellinger --- include/target/target_core_base.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index da854fb4530f..92d5628775c2 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -197,6 +197,7 @@ enum tcm_tmreq_table { TMR_LUN_RESET = 5, TMR_TARGET_WARM_RESET = 6, TMR_TARGET_COLD_RESET = 7, + TMR_UNKNOWN = 0xff, }; /* fabric independent task management response values */ -- cgit v1.2.3 From 4f4c6c3ddb15af396cd8cf81b487b59261199ee9 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 23 Dec 2016 13:23:45 +0100 Subject: target: Remove se_tmr_req.tmr_lun Member tmr_lun of se_tmr_req is set but not used. Hence remove it. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Reviewed-by: Himanshu Madhani Cc: Giridhar Malavali Signed-off-by: Nicholas Bellinger --- include/target/target_core_base.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 92d5628775c2..c8b06e226f2e 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -398,7 +398,6 @@ struct se_tmr_req { void *fabric_tmr_ptr; struct se_cmd *task_cmd; struct se_device *tmr_dev; - struct se_lun *tmr_lun; struct list_head tmr_list; }; -- cgit v1.2.3 From b1a2ecdad72e0a4f0ebdbaff580b5330ebcf68c3 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 1 Feb 2017 16:58:35 -0800 Subject: target: Inline transport_cmd_check_stop() The function transport_cmd_check_stop() has two callers. These callers invoke this function as follows: * transport_cmd_check_stop(cmd, true, false) * transport_cmd_check_stop(cmd, false, true) Hence inline this function into its callers. This patch does not change any functionality but improves source code readability. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Reviewed-by: Bryant G. Ly Cc: Christoph Hellwig Cc: Andy Grover Cc: David Disseldorp Signed-off-by: Nicholas Bellinger --- include/target/target_core_fabric.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h index 358041bad1da..d7dd1427fe0d 100644 --- a/include/target/target_core_fabric.h +++ b/include/target/target_core_fabric.h @@ -47,7 +47,7 @@ struct target_core_fabric_ops { u32 (*tpg_get_inst_index)(struct se_portal_group *); /* * Optional to release struct se_cmd and fabric dependent allocated - * I/O descriptor in transport_cmd_check_stop(). + * I/O descriptor after command execution has finished. * * Returning 1 will signal a descriptor has been released. * Returning 0 will signal a descriptor has not been released. -- cgit v1.2.3 From fd5e64def9170392fdf258d1eee5966433ee3d45 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 3 Jan 2017 10:44:11 +0100 Subject: target: Remove command flag CMD_T_BUSY The patch that reworks task management function handling guarantees that target_remove_from_state_list() is always called with CMD_T_BUSY cleared. Since that function is the only function that tests that flag this means that that flag is now superfluous. Hence remove that flag. Signed-off-by: Bart Van Assche Reviewed-by: Andy Grover Reviewed-by: Christoph Hellwig Reviewed-by: Sagi Grimberg Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- include/target/target_core_base.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index c8b06e226f2e..7b350fd60cdb 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -489,7 +489,6 @@ struct se_cmd { #define CMD_T_SENT (1 << 4) #define CMD_T_STOP (1 << 5) #define CMD_T_DEV_ACTIVE (1 << 7) -#define CMD_T_BUSY (1 << 9) #define CMD_T_TAS (1 << 10) #define CMD_T_FABRIC_STOP (1 << 11) spinlock_t t_state_lock; -- cgit v1.2.3 From b2c9652eba6c11787b5f6cfa53aaea752f58809e Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 5 Jun 2015 16:17:51 -0700 Subject: target: Remove command flag CMD_T_DEV_ACTIVE The code that tests the CMD_T_DEV_ACTIVE flag has been removed. Hence also remove the flag itself. Signed-off-by: Bart Van Assche Reviewed-by: Christoph Hellwig Reviewed-by: Andy Grover Reviewed-by: Sagi Grimberg Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- include/target/target_core_base.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 7b350fd60cdb..d7336f3c6b60 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -488,7 +488,6 @@ struct se_cmd { #define CMD_T_COMPLETE (1 << 2) #define CMD_T_SENT (1 << 4) #define CMD_T_STOP (1 << 5) -#define CMD_T_DEV_ACTIVE (1 << 7) #define CMD_T_TAS (1 << 10) #define CMD_T_FABRIC_STOP (1 << 11) spinlock_t t_state_lock; -- cgit v1.2.3 From 9a584bf9bf0a1c608f5ed5f5e63b074bbc81a322 Mon Sep 17 00:00:00 2001 From: Varun Prakash Date: Fri, 13 Jan 2017 20:53:21 +0530 Subject: target/iscsi: split iscsit_check_dataout_hdr() Split iscsit_check_dataout_hdr() into two functions 1. __iscsit_check_dataout_hdr() - This function validates data out hdr. 2. iscsit_check_dataout_hdr() - This function finds iSCSI cmd using iscsit_find_cmd_from_itt_or_dump(), then it calls __iscsit_check_dataout_hdr() to validate iSCSI hdr. This split is required to support Chelsio T6 iSCSI DDP completion feature. T6 adapters reduce number of completions to host by generating single completion for all directly placed(DDP) iSCSI pdus in a sequence, DDP completion contains iSCSI hdr of the last pdu in a sequence. On receiving DDP completion cxgbit driver will first find iSCSI cmd using iscsit_find_cmd_from_itt_or_dump() then updates cmd->write_data_done, cmd->next_burst_len, cmd->data_sn and calls __iscsit_check_dataout_hdr() to validate iSCSI hdr. (Move XRDSL check ahead of itt lookup / dump - nab) Signed-off-by: Varun Prakash Signed-off-by: Bart Van Assche Signed-off-by: Nicholas Bellinger --- include/target/iscsi/iscsi_transport.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/target/iscsi/iscsi_transport.h b/include/target/iscsi/iscsi_transport.h index 1277e9ba0318..ff1a4f4cd66d 100644 --- a/include/target/iscsi/iscsi_transport.h +++ b/include/target/iscsi/iscsi_transport.h @@ -55,8 +55,12 @@ extern int iscsit_setup_scsi_cmd(struct iscsi_conn *, struct iscsi_cmd *, extern void iscsit_set_unsoliticed_dataout(struct iscsi_cmd *); extern int iscsit_process_scsi_cmd(struct iscsi_conn *, struct iscsi_cmd *, struct iscsi_scsi_req *); -extern int iscsit_check_dataout_hdr(struct iscsi_conn *, unsigned char *, - struct iscsi_cmd **); +extern int +__iscsit_check_dataout_hdr(struct iscsi_conn *, void *, + struct iscsi_cmd *, u32, bool *); +extern int +iscsit_check_dataout_hdr(struct iscsi_conn *conn, void *buf, + struct iscsi_cmd **out_cmd); extern int iscsit_check_dataout_payload(struct iscsi_cmd *, struct iscsi_data *, bool); extern int iscsit_setup_nop_out(struct iscsi_conn *, struct iscsi_cmd *, @@ -125,6 +129,9 @@ extern void iscsit_release_cmd(struct iscsi_cmd *); extern void iscsit_free_cmd(struct iscsi_cmd *, bool); extern void iscsit_add_cmd_to_immediate_queue(struct iscsi_cmd *, struct iscsi_conn *, u8); +extern struct iscsi_cmd * +iscsit_find_cmd_from_itt_or_dump(struct iscsi_conn *conn, + itt_t init_task_tag, u32 length); /* * From iscsi_target_nego.c -- cgit v1.2.3 From 762b6f00a995863afa274d6b5ffa3880dac1714b Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Wed, 15 Feb 2017 23:04:54 +0300 Subject: uapi: fix linux/target_core_user.h userspace compilation errors Consistently use types from linux/types.h to fix the following linux/target_core_user.h userspace compilation errors: /usr/include/linux/target_core_user.h:108:4: error: unknown type name 'uint32_t' uint32_t iov_cnt; /usr/include/linux/target_core_user.h:109:4: error: unknown type name 'uint32_t' uint32_t iov_bidi_cnt; /usr/include/linux/target_core_user.h:110:4: error: unknown type name 'uint32_t' uint32_t iov_dif_cnt; /usr/include/linux/target_core_user.h:111:4: error: unknown type name 'uint64_t' uint64_t cdb_off; /usr/include/linux/target_core_user.h:112:4: error: unknown type name 'uint64_t' uint64_t __pad1; /usr/include/linux/target_core_user.h:113:4: error: unknown type name 'uint64_t' uint64_t __pad2; /usr/include/linux/target_core_user.h:117:4: error: unknown type name 'uint8_t' uint8_t scsi_status; /usr/include/linux/target_core_user.h:118:4: error: unknown type name 'uint8_t' uint8_t __pad1; /usr/include/linux/target_core_user.h:119:4: error: unknown type name 'uint16_t' uint16_t __pad2; /usr/include/linux/target_core_user.h:120:4: error: unknown type name 'uint32_t' uint32_t __pad3; Signed-off-by: Dmitry V. Levin Signed-off-by: Nicholas Bellinger --- include/uapi/linux/target_core_user.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'include') diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h index c506cddb8165..af17b4154ef6 100644 --- a/include/uapi/linux/target_core_user.h +++ b/include/uapi/linux/target_core_user.h @@ -105,26 +105,26 @@ struct tcmu_cmd_entry { union { struct { - uint32_t iov_cnt; - uint32_t iov_bidi_cnt; - uint32_t iov_dif_cnt; - uint64_t cdb_off; - uint64_t __pad1; - uint64_t __pad2; + __u32 iov_cnt; + __u32 iov_bidi_cnt; + __u32 iov_dif_cnt; + __u64 cdb_off; + __u64 __pad1; + __u64 __pad2; struct iovec iov[0]; } req; struct { - uint8_t scsi_status; - uint8_t __pad1; - uint16_t __pad2; - uint32_t __pad3; + __u8 scsi_status; + __u8 __pad1; + __u16 __pad2; + __u32 __pad3; char sense_buffer[TCMU_SENSE_BUFFERSIZE]; } rsp; }; } __packed; -#define TCMU_OP_ALIGN_SIZE sizeof(uint64_t) +#define TCMU_OP_ALIGN_SIZE sizeof(__u64) enum tcmu_genl_cmd { TCMU_CMD_UNSPEC, -- cgit v1.2.3 From bd4e2d2907fa23a11d46217064ecf80470ddae10 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Wed, 22 Feb 2017 22:06:32 -0800 Subject: target: Fix NULL dereference during LUN lookup + active I/O shutdown When transport_clear_lun_ref() is shutting down a se_lun via configfs with new I/O in-flight, it's possible to trigger a NULL pointer dereference in transport_lookup_cmd_lun() due to the fact percpu_ref_get() doesn't do any __PERCPU_REF_DEAD checking before incrementing lun->lun_ref.count after lun->lun_ref has switched to atomic_t mode. This results in a NULL pointer dereference as LUN shutdown code in core_tpg_remove_lun() continues running after the existing ->release() -> core_tpg_lun_ref_release() callback completes, and clears the RCU protected se_lun->lun_se_dev pointer. During the OOPs, the state of lun->lun_ref in the process which triggered the NULL pointer dereference looks like the following on v4.1.y stable code: struct se_lun { lun_link_magic = 4294932337, lun_status = TRANSPORT_LUN_STATUS_FREE, ..... lun_se_dev = 0x0, lun_sep = 0x0, ..... lun_ref = { count = { counter = 1 }, percpu_count_ptr = 3, release = 0xffffffffa02fa1e0 , confirm_switch = 0x0, force_atomic = false, rcu = { next = 0xffff88154fa1a5d0, func = 0xffffffff8137c4c0 } } } To address this bug, use percpu_ref_tryget_live() to ensure once __PERCPU_REF_DEAD is visable on all CPUs and ->lun_ref has switched to atomic_t, all new I/Os will fail to obtain a new lun->lun_ref reference. Also use an explicit percpu_ref_kill_and_confirm() callback to block on ->lun_ref_comp to allow the first stage and associated RCU grace period to complete, and then block on ->lun_ref_shutdown waiting for the final percpu_ref_put() to drop the last reference via transport_lun_remove_cmd() before continuing with core_tpg_remove_lun() shutdown. Reported-by: Rob Millner Tested-by: Rob Millner Cc: Rob Millner Tested-by: Vaibhav Tandon Cc: Vaibhav Tandon Tested-by: Bryant G. Ly Cc: # v3.14+ Signed-off-by: Nicholas Bellinger --- include/target/target_core_base.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index d7336f3c6b60..16d3be8395be 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -730,6 +730,7 @@ struct se_lun { struct config_group lun_group; struct se_port_stat_grps port_stat_grps; struct completion lun_ref_comp; + struct completion lun_shutdown_comp; struct percpu_ref lun_ref; struct list_head lun_dev_link; struct hlist_node link; -- cgit v1.2.3 From c87ba9c49c1fa86261448b09c5f1b2223bf7efd9 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Thu, 19 Jan 2017 15:45:57 -0800 Subject: target: Add counters for ABORT_TASK success + failure This patch introduces two counters for ABORT_TASK success + failure under: /sys/kernel/config/target/core/$HBA/$DEV/statistics/scsi_tgt_dev/ that are useful for diagnosing various backend device latency and front fabric issues. Normally when folks see alot of aborts_complete happening, it means the backend device I/O completion latency is high, and not returning completions fast enough before host side timeouts trigger. And normally when folks see alot of aborts_no_task, it means completions are being posted by target-core into fabric driver code, but the responses aren't making it back to the host. Signed-off-by: Nicholas Bellinger --- include/target/target_core_base.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 16d3be8395be..49ce5bc9912f 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -766,6 +766,8 @@ struct se_device { u32 dev_index; u64 creation_time; atomic_long_t num_resets; + atomic_long_t aborts_complete; + atomic_long_t aborts_no_task; atomic_long_t num_cmds; atomic_long_t read_bytes; atomic_long_t write_bytes; -- cgit v1.2.3