* [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops
@ 2010-08-30 9:20 Nicholas A. Bellinger
2010-09-02 5:56 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 11+ messages in thread
From: Nicholas A. Bellinger @ 2010-08-30 9:20 UTC (permalink / raw)
To: linux-scsi, linux-kernel
Cc: FUJITA Tomonori, Mike Christie, Christoph Hellwig,
Hannes Reinecke, James Bottomley, Jens Axboe, Boaz Harrosh,
Nicholas Bellinger
From: Nicholas Bellinger <nab@linux-iscsi.org>
This patch adds TCM Core v4 base data structures definitions + macros
and fabric module function pointer API in struct target_core_fabric_ops.
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
include/target/target_core_base.h | 985 +++++++++++++++++++++++++++++++
include/target/target_core_fabric_ops.h | 89 +++
2 files changed, 1074 insertions(+), 0 deletions(-)
create mode 100644 include/target/target_core_base.h
create mode 100644 include/target/target_core_fabric_ops.h
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
new file mode 100644
index 0000000..f4d0c70
--- /dev/null
+++ b/include/target/target_core_base.h
@@ -0,0 +1,985 @@
+#ifndef TARGET_CORE_BASE_H
+#define TARGET_CORE_BASE_H
+
+#include <linux/in.h>
+#include <linux/configfs.h>
+#include <net/sock.h>
+#include <net/tcp.h>
+#include "target_core_mib.h"
+
+#define TARGET_CORE_MOD_VERSION "v4.0.0-rc3"
+#define SHUTDOWN_SIGS (sigmask(SIGKILL)|sigmask(SIGINT)|sigmask(SIGABRT))
+
+/* SCSI Command Descriptor Block Size a la SCSI's MAX_COMMAND_SIZE */
+#define SCSI_CDB_SIZE 16
+#define TRANSPORT_IOV_DATA_BUFFER 5
+
+/* Maximum Number of LUNs per Target Portal Group */
+#define TRANSPORT_MAX_LUNS_PER_TPG 256
+
+/* From include/scsi/scsi_cmnd.h:SCSI_SENSE_BUFFERSIZE */
+#define TRANSPORT_SENSE_BUFFER SCSI_SENSE_BUFFERSIZE
+
+#define SPC_SENSE_KEY_OFFSET 2
+#define SPC_ASC_KEY_OFFSET 12
+#define SPC_ASCQ_KEY_OFFSET 13
+
+/* Currently same as ISCSI_IQN_LEN */
+#define TRANSPORT_IQN_LEN 224
+#define LU_GROUP_NAME_BUF 256
+#define TG_PT_GROUP_NAME_BUF 256
+/* Used to parse VPD into struct t10_vpd */
+#define VPD_TMP_BUF_SIZE 128
+/* Used for target_core-pscsi.c:pscsi_transport_complete() */
+#define VPD_BUF_LEN 256
+/* Used for struct se_subsystem_dev-->se_dev_alias, must be less than
+ PAGE_SIZE */
+#define SE_DEV_ALIAS_LEN 512
+/* Used for struct se_subsystem_dev->se_dev_udev_path[], must be less than
+ PAGE_SIZE */
+#define SE_UDEV_PATH_LEN 512
+/* Used for struct se_dev_snap_attrib->contact */
+#define SNAP_CONTACT_LEN 128
+/* Used for struct se_dev_snap_attrib->lv_group */
+#define SNAP_GROUP_LEN 128
+/* Used for struct se_dev_snap_attrib->lvc_size */
+#define SNAP_LVC_LEN 32
+/* Used by struct t10_reservation_template->pr_[i,t]_port[] */
+#define PR_APTPL_MAX_IPORT_LEN 256
+#define PR_APTPL_MAX_TPORT_LEN 256
+/* Used by struct t10_reservation_template->pr_aptpl_buf_len */
+#define PR_APTPL_BUF_LEN 8192
+/* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
+#define ALUA_MD_BUF_LEN 1024
+/* Used by struct t10_pr_registration->pr_reg_isid */
+#define PR_REG_ISID_LEN 16
+/* PR_REG_ISID_LEN + ',i,0x' */
+#define PR_REG_ISID_ID_LEN (PR_REG_ISID_LEN + 5)
+
+/* used by PSCSI and iBlock Transport drivers */
+#define READ_BLOCK_LEN 6
+#define READ_CAP_LEN 8
+#define READ_POSITION_LEN 20
+#define INQUIRY_LEN 36
+#define INQUIRY_VPD_SERIAL_LEN 254
+#define INQUIRY_VPD_DEVICE_IDENTIFIER_LEN 254
+
+/* struct se_cmd->data_direction */
+#define SE_DIRECTION_NONE 0
+#define SE_DIRECTION_READ 1
+#define SE_DIRECTION_WRITE 2
+#define SE_DIRECTION_BIDI 3
+
+/* struct se_hba->hba_flags */
+#define HBA_FLAGS_INTERNAL_USE 0x00000001
+#define HBA_FLAGS_PSCSI_MODE 0x00000002
+
+/* struct se_hba->hba_status and iscsi_tpg_hba->thba_status */
+#define HBA_STATUS_FREE 0x00000001
+#define HBA_STATUS_ACTIVE 0x00000002
+#define HBA_STATUS_INACTIVE 0x00000004
+#define HBA_STATUS_SHUTDOWN 0x00000008
+
+/* struct se_lun->lun_status */
+#define TRANSPORT_LUN_STATUS_FREE 0
+#define TRANSPORT_LUN_STATUS_ACTIVE 1
+
+/* struct se_lun->lun_type */
+#define TRANSPORT_LUN_TYPE_NONE 0
+#define TRANSPORT_LUN_TYPE_DEVICE 1
+
+/* struct se_portal_group->se_tpg_type */
+#define TRANSPORT_TPG_TYPE_NORMAL 0
+#define TRANSPORT_TPG_TYPE_DISCOVERY 1
+
+/* Used for se_node_acl->nodeacl_flags */
+#define NAF_DYNAMIC_NODE_ACL 0x01
+
+/* struct se_map_sg->map_flags */
+#define MAP_SG_KMAP 0x01
+
+/* Used for generate timer flags */
+#define TF_RUNNING 0x01
+#define TF_STOP 0x02
+
+/* Special transport agnostic struct se_cmd->t_states */
+#define TRANSPORT_NO_STATE 240
+#define TRANSPORT_NEW_CMD 241
+#define TRANSPORT_DEFERRED_CMD 242
+#define TRANSPORT_WRITE_PENDING 243
+#define TRANSPORT_PROCESS_WRITE 244
+#define TRANSPORT_PROCESSING 245
+#define TRANSPORT_COMPLETE_OK 246
+#define TRANSPORT_COMPLETE_FAILURE 247
+#define TRANSPORT_COMPLETE_TIMEOUT 248
+#define TRANSPORT_PROCESS_TMR 249
+#define TRANSPORT_TMR_COMPLETE 250
+#define TRANSPORT_ISTATE_PROCESSING 251
+#define TRANSPORT_ISTATE_PROCESSED 252
+#define TRANSPORT_KILL 253
+#define TRANSPORT_REMOVE 254
+#define TRANSPORT_FREE 255
+
+#define SCF_SUPPORTED_SAM_OPCODE 0x00000001
+#define SCF_TRANSPORT_TASK_SENSE 0x00000002
+#define SCF_EMULATED_TASK_SENSE 0x00000004
+#define SCF_SCSI_DATA_SG_IO_CDB 0x00000008
+#define SCF_SCSI_CONTROL_SG_IO_CDB 0x00000010
+#define SCF_SCSI_CONTROL_NONSG_IO_CDB 0x00000020
+#define SCF_SCSI_NON_DATA_CDB 0x00000040
+#define SCF_SCSI_CDB_EXCEPTION 0x00000080
+#define SCF_SCSI_RESERVATION_CONFLICT 0x00000100
+#define SCF_CMD_PASSTHROUGH 0x00000200
+#define SCF_CMD_PASSTHROUGH_NOALLOC 0x00000400
+#define SCF_SE_CMD_FAILED 0x00000800
+#define SCF_SE_LUN_CMD 0x00001000
+#define SCF_SE_ALLOW_EOO 0x00002000
+#define SCF_SE_DISABLE_ONLINE_CHECK 0x00004000
+#define SCF_SENT_CHECK_CONDITION 0x00008000
+#define SCF_OVERFLOW_BIT 0x00010000
+#define SCF_UNDERFLOW_BIT 0x00020000
+#define SCF_SENT_DELAYED_TAS 0x00040000
+#define SCF_ALUA_NON_OPTIMIZED 0x00080000
+#define SCF_DELAYED_CMD_FROM_SAM_ATTR 0x00100000
+#define SCF_PASSTHROUGH_SG_TO_MEM 0x00200000
+#define SCF_PASSTHROUGH_CONTIG_TO_SG 0x00400000
+#define SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC 0x00800000
+#define SCF_EMULATE_SYNC_CACHE 0x01000000
+#define SCF_EMULATE_CDB_ASYNC 0x02000000
+
+/* struct se_device->type */
+#define PSCSI 1
+#define STGT 2
+#define PATA 3
+#define IBLOCK 4
+#define RAMDISK_DR 5
+#define RAMDISK_MCP 6
+#define FILEIO 7
+#define VROM 8
+#define VTAPE 9
+#define MEDIA_CHANGER 10
+
+/* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
+#define TRANSPORT_LUNFLAGS_NO_ACCESS 0x00000000
+#define TRANSPORT_LUNFLAGS_INITIATOR_ACCESS 0x00000001
+#define TRANSPORT_LUNFLAGS_READ_ONLY 0x00000002
+#define TRANSPORT_LUNFLAGS_READ_WRITE 0x00000004
+
+/* struct se_device->dev_status */
+#define TRANSPORT_DEVICE_ACTIVATED 0x01
+#define TRANSPORT_DEVICE_DEACTIVATED 0x02
+#define TRANSPORT_DEVICE_QUEUE_FULL 0x04
+#define TRANSPORT_DEVICE_SHUTDOWN 0x08
+#define TRANSPORT_DEVICE_OFFLINE_ACTIVATED 0x10
+#define TRANSPORT_DEVICE_OFFLINE_DEACTIVATED 0x20
+
+#define DEV_STATUS_THR_TUR 1
+#define DEV_STATUS_THR_TAKE_ONLINE 2
+#define DEV_STATUS_THR_TAKE_OFFLINE 3
+#define DEV_STATUS_THR_SHUTDOWN 4
+
+/* struct se_dev_entry->deve_flags */
+#define DEF_PR_REGISTERED 0x01
+
+/* Used for struct t10_pr_registration->pr_reg_flags */
+#define PRF_ISID_PRESENT_AT_REG 0x01
+
+/* transport_send_check_condition_and_sense() */
+#define NON_EXISTENT_LUN 0x1
+#define UNSUPPORTED_SCSI_OPCODE 0x2
+#define INCORRECT_AMOUNT_OF_DATA 0x3
+#define UNEXPECTED_UNSOLICITED_DATA 0x4
+#define SERVICE_CRC_ERROR 0x5
+#define SNACK_REJECTED 0x6
+#define SECTOR_COUNT_TOO_MANY 0x7
+#define INVALID_CDB_FIELD 0x8
+#define INVALID_PARAMETER_LIST 0x9
+#define LOGICAL_UNIT_COMMUNICATION_FAILURE 0xa
+#define UNKNOWN_MODE_PAGE 0xb
+#define WRITE_PROTECTED 0xc
+#define CHECK_CONDITION_ABORT_CMD 0xd
+#define CHECK_CONDITION_UNIT_ATTENTION 0xe
+#define CHECK_CONDITION_NOT_READY 0xf
+
+struct se_obj {
+ atomic_t obj_access_count;
+} ____cacheline_aligned;
+
+typedef enum {
+ SPC_ALUA_PASSTHROUGH,
+ SPC2_ALUA_DISABLED,
+ SPC3_ALUA_EMULATED
+} t10_alua_index_t;
+
+typedef enum {
+ SAM_TASK_ATTR_PASSTHROUGH,
+ SAM_TASK_ATTR_UNTAGGED,
+ SAM_TASK_ATTR_EMULATED
+} t10_task_attr_index_t;
+
+struct se_cmd;
+
+struct t10_alua {
+ t10_alua_index_t alua_type;
+ /* ALUA Target Port Group ID */
+ u16 alua_tg_pt_gps_counter;
+ u32 alua_tg_pt_gps_count;
+ spinlock_t tg_pt_gps_lock;
+ struct se_subsystem_dev *t10_sub_dev;
+ /* Used for default ALUA Target Port Group */
+ struct t10_alua_tg_pt_gp *default_tg_pt_gp;
+ /* Used for default ALUA Target Port Group ConfigFS group */
+ struct config_group alua_tg_pt_gps_group;
+ int (*alua_state_check)(struct se_cmd *, unsigned char *, u8 *);
+ struct list_head tg_pt_gps_list;
+} ____cacheline_aligned;
+
+struct t10_alua_lu_gp {
+ u16 lu_gp_id;
+ int lu_gp_valid_id;
+ u32 lu_gp_members;
+ atomic_t lu_gp_shutdown;
+ atomic_t lu_gp_ref_cnt;
+ spinlock_t lu_gp_lock;
+ struct config_group lu_gp_group;
+ struct list_head lu_gp_list;
+ struct list_head lu_gp_mem_list;
+} ____cacheline_aligned;
+
+struct t10_alua_lu_gp_member {
+ int lu_gp_assoc;
+ atomic_t lu_gp_mem_ref_cnt;
+ spinlock_t lu_gp_mem_lock;
+ struct t10_alua_lu_gp *lu_gp;
+ struct se_device *lu_gp_mem_dev;
+ struct list_head lu_gp_mem_list;
+} ____cacheline_aligned;
+
+struct t10_alua_tg_pt_gp {
+ u16 tg_pt_gp_id;
+ int tg_pt_gp_valid_id;
+ int tg_pt_gp_alua_access_status;
+ int tg_pt_gp_alua_access_type;
+ int tg_pt_gp_nonop_delay_msecs;
+ int tg_pt_gp_trans_delay_msecs;
+ int tg_pt_gp_pref;
+ int tg_pt_gp_write_metadata;
+ u32 tg_pt_gp_md_buf_len;
+ u32 tg_pt_gp_members;
+ atomic_t tg_pt_gp_alua_access_state;
+ atomic_t tg_pt_gp_ref_cnt;
+ spinlock_t tg_pt_gp_lock;
+ struct mutex tg_pt_gp_md_mutex;
+ struct se_subsystem_dev *tg_pt_gp_su_dev;
+ struct config_group tg_pt_gp_group;
+ struct list_head tg_pt_gp_list;
+ struct list_head tg_pt_gp_mem_list;
+} ____cacheline_aligned;
+
+struct t10_alua_tg_pt_gp_member {
+ int tg_pt_gp_assoc;
+ atomic_t tg_pt_gp_mem_ref_cnt;
+ spinlock_t tg_pt_gp_mem_lock;
+ struct t10_alua_tg_pt_gp *tg_pt_gp;
+ struct se_port *tg_pt;
+ struct list_head tg_pt_gp_mem_list;
+} ____cacheline_aligned;
+
+struct t10_vpd {
+ unsigned char device_identifier[INQUIRY_VPD_DEVICE_IDENTIFIER_LEN];
+ int protocol_identifier_set;
+ u32 protocol_identifier;
+ u32 device_identifier_code_set;
+ u32 association;
+ u32 device_identifier_type;
+ struct list_head vpd_list;
+} ____cacheline_aligned;
+
+struct t10_wwn {
+ unsigned char vendor[8];
+ unsigned char model[16];
+ unsigned char revision[4];
+ unsigned char unit_serial[INQUIRY_VPD_SERIAL_LEN];
+ spinlock_t t10_vpd_lock;
+ struct se_subsystem_dev *t10_sub_dev;
+ struct config_group t10_wwn_group;
+ struct list_head t10_vpd_list;
+} ____cacheline_aligned;
+
+typedef enum {
+ SPC_PASSTHROUGH,
+ SPC2_RESERVATIONS,
+ SPC3_PERSISTENT_RESERVATIONS
+} t10_reservations_index_t;
+
+struct t10_pr_registration {
+ /* Used for fabrics that contain WWN+ISID */
+ char pr_reg_isid[PR_REG_ISID_LEN];
+ /* Used during APTPL metadata reading */
+ unsigned char pr_iport[PR_APTPL_MAX_IPORT_LEN];
+ /* Used during APTPL metadata reading */
+ unsigned char pr_tport[PR_APTPL_MAX_TPORT_LEN];
+ /* For writing out live meta data */
+ unsigned char *pr_aptpl_buf;
+ u16 pr_aptpl_rpti;
+ u16 pr_reg_tpgt;
+ /* Reservation effects all target ports */
+ int pr_reg_all_tg_pt;
+ /* Activate Persistence across Target Power Loss */
+ int pr_reg_aptpl;
+ int pr_res_holder;
+ int pr_res_type;
+ int pr_res_scope;
+ u32 pr_reg_flags;
+ u32 pr_res_mapped_lun;
+ u32 pr_aptpl_target_lun;
+ u32 pr_res_generation;
+ u64 pr_reg_bin_isid;
+ u64 pr_res_key;
+ atomic_t pr_res_holders;
+ struct se_node_acl *pr_reg_nacl;
+ struct se_dev_entry *pr_reg_deve;
+ struct se_lun *pr_reg_tg_pt_lun;
+ struct list_head pr_reg_list;
+ struct list_head pr_reg_abort_list;
+ struct list_head pr_reg_aptpl_list;
+ struct list_head pr_reg_atp_list;
+ struct list_head pr_reg_atp_mem_list;
+} ____cacheline_aligned;
+
+struct t10_reservation_template {
+ /* Reservation effects all target ports */
+ int pr_all_tg_pt;
+ /* Activate Persistence across Target Power Loss enabled
+ * for SCSI device */
+ int pr_aptpl_active;
+ u32 pr_aptpl_buf_len;
+ u32 pr_generation;
+ t10_reservations_index_t res_type;
+ spinlock_t registration_lock;
+ spinlock_t aptpl_reg_lock;
+ /* Reservation holder when pr_all_tg_pt=1 */
+ struct se_node_acl *pr_res_holder;
+ struct list_head registration_list;
+ struct list_head aptpl_reg_list;
+ int (*t10_reservation_check)(struct se_cmd *, u32 *);
+ int (*t10_seq_non_holder)(struct se_cmd *, unsigned char *, u32);
+ int (*t10_pr_register)(struct se_cmd *);
+ int (*t10_pr_clear)(struct se_cmd *);
+} ____cacheline_aligned;
+
+struct se_queue_req {
+ int state;
+ void *cmd;
+ struct list_head qr_list;
+} ____cacheline_aligned;
+
+struct se_queue_obj {
+ atomic_t queue_cnt;
+ spinlock_t cmd_queue_lock;
+ struct list_head qobj_list;
+ wait_queue_head_t thread_wq;
+ struct completion thread_create_comp;
+ struct completion thread_done_comp;
+} ____cacheline_aligned;
+
+struct se_transport_task {
+ unsigned char t_task_cdb[SCSI_CDB_SIZE];
+ unsigned long long t_task_lba;
+ int t_tasks_failed;
+ int t_task_fua;
+ u32 t_task_cdbs;
+ u32 t_task_check;
+ u32 t_task_no;
+ u32 t_task_sectors;
+ u32 t_task_se_num;
+ atomic_t t_fe_count;
+ atomic_t t_se_count;
+ atomic_t t_task_cdbs_left;
+ atomic_t t_task_cdbs_ex_left;
+ atomic_t t_task_cdbs_timeout_left;
+ atomic_t t_task_cdbs_sent;
+ atomic_t t_transport_aborted;
+ atomic_t t_transport_active;
+ atomic_t t_transport_complete;
+ atomic_t t_transport_queue_active;
+ atomic_t t_transport_sent;
+ atomic_t t_transport_stop;
+ atomic_t t_transport_timeout;
+ atomic_t transport_dev_active;
+ atomic_t transport_lun_active;
+ atomic_t transport_lun_fe_stop;
+ atomic_t transport_lun_stop;
+ spinlock_t t_state_lock;
+ struct completion t_transport_stop_comp;
+ struct completion t_transport_passthrough_comp;
+ struct completion t_transport_passthrough_wcomp;
+ struct completion transport_lun_fe_stop_comp;
+ struct completion transport_lun_stop_comp;
+ void *t_task_buf;
+ void *t_task_pt_buf;
+ struct list_head t_task_list;
+ struct list_head *t_mem_list;
+} ____cacheline_aligned;
+
+struct se_task {
+ unsigned char task_sense;
+ struct scatterlist *task_sg;
+ void *transport_req;
+ u8 task_scsi_status;
+ u8 task_flags;
+ int task_error_status;
+ int task_state_flags;
+ unsigned long long task_lba;
+ u32 task_no;
+ u32 task_sectors;
+ u32 task_size;
+ u32 task_sg_num;
+ u32 task_sg_offset;
+ struct se_cmd *task_se_cmd;
+ struct se_device *se_dev;
+ struct completion task_stop_comp;
+ atomic_t task_active;
+ atomic_t task_execute_queue;
+ atomic_t task_timeout;
+ atomic_t task_sent;
+ atomic_t task_stop;
+ atomic_t task_state_active;
+ struct timer_list task_timer;
+ int (*transport_map_task)(struct se_task *, u32);
+ struct se_device *se_obj_ptr;
+ struct list_head t_list;
+ struct list_head t_execute_list;
+ struct list_head t_state_list;
+} ____cacheline_aligned;
+
+#define TASK_CMD(task) ((struct se_cmd *)task->task_se_cmd)
+#define TASK_DEV(task) ((struct se_device *)task->se_dev)
+
+struct se_transform_info {
+ int ti_set_counts;
+ u32 ti_data_length;
+ unsigned long long ti_lba;
+ struct se_cmd *ti_se_cmd;
+ struct se_device *ti_dev;
+ struct se_device *se_obj_ptr;
+ struct se_device *ti_obj_ptr;
+} ____cacheline_aligned;
+
+struct se_offset_map {
+ int map_reset;
+ u32 iovec_length;
+ u32 iscsi_offset;
+ u32 current_offset;
+ u32 orig_offset;
+ u32 sg_count;
+ u32 sg_current;
+ u32 sg_length;
+ struct page *sg_page;
+ struct se_mem *map_se_mem;
+ struct se_mem *map_orig_se_mem;
+ void *iovec_base;
+} ____cacheline_aligned;
+
+struct se_map_sg {
+ int map_flags;
+ u32 data_length;
+ u32 data_offset;
+ void *fabric_cmd;
+ struct se_cmd *se_cmd;
+ struct iovec *iov;
+} ____cacheline_aligned;
+
+struct se_unmap_sg {
+ u32 data_length;
+ u32 sg_count;
+ u32 sg_offset;
+ u32 padding;
+ u32 t_offset;
+ void *fabric_cmd;
+ struct se_cmd *se_cmd;
+ struct se_offset_map lmap;
+ struct se_mem *cur_se_mem;
+} ____cacheline_aligned;
+
+struct se_cmd {
+ /* SAM response code being sent to initiator */
+ u8 scsi_status;
+ u8 scsi_asc;
+ u8 scsi_ascq;
+ u8 scsi_sense_reason;
+ u16 scsi_sense_length;
+ /* Delay for ALUA Active/NonOptimized state access in milliseconds */
+ int alua_nonop_delay;
+ int data_direction;
+ /* For SAM Task Attribute */
+ int sam_task_attr;
+ /* Transport protocol dependent state */
+ int t_state;
+ /* Transport protocol dependent state for out of order CmdSNs */
+ int deferred_t_state;
+ /* Transport specific error status */
+ int transport_error_status;
+ u32 se_cmd_flags;
+ u32 se_ordered_id;
+ /* Total size in bytes associated with command */
+ u32 data_length;
+ /* SCSI Presented Data Transfer Length */
+ u32 cmd_spdtl;
+ u32 residual_count;
+ u32 orig_fe_lun;
+ /* Number of iovecs iovecs used for IP stack calls */
+ u32 iov_data_count;
+ /* Number of iovecs allocated for iscsi_cmd_t->iov_data */
+ u32 orig_iov_data_count;
+ /* Persistent Reservation key */
+ u64 pr_res_key;
+ atomic_t transport_sent;
+ /* Used for sense data */
+ void *sense_buffer;
+ /* Used with sockets based fabric plugins */
+ struct iovec *iov_data;
+ struct list_head se_delayed_list;
+ struct list_head se_ordered_list;
+ struct list_head se_lun_list;
+ struct se_device *se_dev;
+ struct se_dev_entry *se_deve;
+ struct se_device *se_obj_ptr;
+ struct se_device *se_orig_obj_ptr;
+ struct se_lun *se_lun;
+ void *se_fabric_cmd_ptr;
+ struct se_session *se_sess;
+ struct se_tmr_req *se_tmr_req;
+ struct se_transport_task *t_task;
+ struct target_core_fabric_ops *se_tfo;
+ int (*transport_add_cmd_to_queue)(struct se_cmd *, u8);
+ int (*transport_allocate_resources)(struct se_cmd *, u32, u32);
+ int (*transport_cdb_transform)(struct se_cmd *,
+ struct se_transform_info *);
+ int (*transport_do_transform)(struct se_cmd *,
+ struct se_transform_info *);
+ int (*transport_emulate_cdb)(struct se_cmd *);
+ void (*transport_free_resources)(struct se_cmd *);
+ u32 (*transport_get_lba)(unsigned char *);
+ unsigned long long (*transport_get_long_lba)(unsigned char *);
+ struct se_task *(*transport_get_task)(struct se_transform_info *,
+ struct se_cmd *, void *);
+ int (*transport_map_buffers_to_tasks)(struct se_cmd *);
+ void (*transport_map_SG_segments)(struct se_unmap_sg *);
+ void (*transport_passthrough_done)(struct se_cmd *);
+ void (*transport_unmap_SG_segments)(struct se_unmap_sg *);
+ int (*transport_set_iovec_ptrs)(struct se_map_sg *,
+ struct se_unmap_sg *);
+ void (*transport_split_cdb)(unsigned long long, u32 *, unsigned char *);
+ void (*transport_wait_for_tasks)(struct se_cmd *, int, int);
+ void (*callback)(struct se_cmd *cmd, void *callback_arg,
+ int complete_status);
+ void *callback_arg;
+} ____cacheline_aligned;
+
+#define T_TASK(cmd) ((struct se_transport_task *)(cmd->t_task))
+#define CMD_TFO(cmd) ((struct target_core_fabric_ops *)cmd->se_tfo)
+
+struct se_tmr_req {
+ /* Task Management function to be preformed */
+ u8 function;
+ /* Task Management response to send */
+ u8 response;
+ int call_transport;
+ /* Reference to ITT that Task Mgmt should be preformed */
+ u32 ref_task_tag;
+ /* 64-bit encoded SAM LUN from $FABRIC_MOD TMR header */
+ u64 ref_task_lun;
+ void *fabric_tmr_ptr;
+ struct se_cmd *task_cmd;
+ struct se_cmd *ref_cmd;
+ struct se_device *tmr_dev;
+ struct se_lun *tmr_lun;
+ struct list_head tmr_list;
+} ____cacheline_aligned;
+
+struct se_ua {
+ u8 ua_asc;
+ u8 ua_ascq;
+ struct se_node_acl *ua_nacl;
+ struct list_head ua_dev_list;
+ struct list_head ua_nacl_list;
+} ____cacheline_aligned;
+
+struct se_node_acl {
+ char initiatorname[TRANSPORT_IQN_LEN];
+ int nodeacl_flags;
+ u32 queue_depth;
+ u32 acl_index;
+ u64 num_cmds;
+ u64 read_bytes;
+ u64 write_bytes;
+ spinlock_t stats_lock;
+ /* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
+ atomic_t acl_pr_ref_count;
+ struct se_dev_entry *device_list;
+ struct se_session *nacl_sess;
+ struct se_portal_group *se_tpg;
+ spinlock_t device_list_lock;
+ spinlock_t nacl_sess_lock;
+ struct config_group acl_group;
+ struct config_group acl_attrib_group;
+ struct config_group acl_auth_group;
+ struct config_group acl_param_group;
+ struct config_group *acl_default_groups[4];
+ struct list_head acl_list;
+ struct list_head acl_sess_list;
+} ____cacheline_aligned;
+
+struct se_session {
+ u64 sess_bin_isid;
+ struct se_node_acl *se_node_acl;
+ struct se_portal_group *se_tpg;
+ void *fabric_sess_ptr;
+ struct list_head sess_list;
+ struct list_head sess_acl_list;
+} ____cacheline_aligned;
+
+#define SE_SESS(cmd) ((struct se_session *)(cmd)->se_sess)
+#define SE_NODE_ACL(sess) ((struct se_node_acl *)(sess)->se_node_acl)
+
+struct se_device;
+struct se_transform_info;
+struct scatterlist;
+
+struct se_lun_acl {
+ char initiatorname[TRANSPORT_IQN_LEN];
+ u32 mapped_lun;
+ struct se_node_acl *se_lun_nacl;
+ struct se_lun *se_lun;
+ struct list_head lacl_list;
+ struct config_group se_lun_group;
+} ____cacheline_aligned;
+
+struct se_dev_entry {
+ u32 lun_flags;
+ u32 deve_cmds;
+ u32 deve_flags;
+ u32 mapped_lun;
+ u32 average_bytes;
+ u32 last_byte_count;
+ u32 total_cmds;
+ u32 total_bytes;
+ u64 pr_res_key;
+ u64 creation_time;
+ u32 attach_count;
+ u64 read_bytes;
+ u64 write_bytes;
+ atomic_t ua_count;
+ /* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
+ atomic_t pr_ref_count;
+ struct se_lun_acl *se_lun_acl;
+ spinlock_t ua_lock;
+ struct se_lun *se_lun;
+ struct list_head alua_port_list;
+ struct list_head ua_list;
+} ____cacheline_aligned;
+
+struct se_dev_attrib {
+ int emulate_dpo;
+ int emulate_fua_write;
+ int emulate_fua_read;
+ int emulate_write_cache;
+ int emulate_ua_intlck_ctrl;
+ int emulate_tas;
+ int emulate_reservations;
+ int emulate_alua;
+ int enforce_pr_isids;
+ u32 hw_block_size;
+ u32 block_size;
+ u32 hw_max_sectors;
+ u32 max_sectors;
+ u32 hw_queue_depth;
+ u32 queue_depth;
+ u32 task_timeout;
+ struct se_subsystem_dev *da_sub_dev;
+ struct config_group da_group;
+} ____cacheline_aligned;
+
+struct se_dev_snap_attrib {
+ unsigned char contact[SNAP_CONTACT_LEN];
+ unsigned char lv_group[SNAP_GROUP_LEN];
+ /* in lvcreate --size shorthand */
+ unsigned char lvc_size[SNAP_LVC_LEN];
+ pid_t pid;
+ int enabled;
+ int permissions;
+ int max_snapshots;
+ int max_warn;
+ int check_interval;
+ int create_interval;
+ int usage;
+ int usage_warn;
+ int vgs_usage_warn;
+} ____cacheline_aligned;
+
+struct se_subsystem_dev {
+ unsigned char se_dev_alias[SE_DEV_ALIAS_LEN];
+ unsigned char se_dev_udev_path[SE_UDEV_PATH_LEN];
+ u32 su_dev_flags;
+ struct se_hba *se_dev_hba;
+ struct se_device *se_dev_ptr;
+ struct se_dev_attrib se_dev_attrib;
+ struct se_dev_snap_attrib se_snap_attrib;
+ /* T10 Asymmetric Logical Unit Assignment for Target Ports */
+ struct t10_alua t10_alua;
+ /* T10 Inquiry and VPD WWN Information */
+ struct t10_wwn t10_wwn;
+ /* T10 SPC-2 + SPC-3 Reservations */
+ struct t10_reservation_template t10_reservation;
+ spinlock_t se_dev_lock;
+ void *se_dev_su_ptr;
+ struct list_head g_se_dev_list;
+ struct config_group se_dev_group;
+ /* For T10 Reservations */
+ struct config_group se_dev_pr_group;
+ /* For userspace lvm utils */
+ struct config_group se_dev_snap_group;
+} ____cacheline_aligned;
+
+#define SE_DEV_SNAP(su_dev) (&(su_dev)->se_snap_attrib)
+#define T10_ALUA(su_dev) (&(su_dev)->t10_alua)
+#define T10_RES(su_dev) (&(su_dev)->t10_reservation)
+
+struct se_device {
+ /* Type of disk transport used for device */
+ u8 type;
+ /* Set to 1 if thread is NOT sleeping on thread_sem */
+ u8 thread_active;
+ u8 dev_status_timer_flags;
+ /* RELATIVE TARGET PORT IDENTIFER Counter */
+ u16 dev_rpti_counter;
+ /* Used for SAM Task Attribute ordering */
+ u32 dev_cur_ordered_id;
+ u32 dev_flags;
+ u32 dev_port_count;
+ u32 dev_status;
+ u32 dev_tcq_window_closed;
+ /* Physical device queue depth */
+ u32 queue_depth;
+ /* Used for SPC-2 reservations enforce of ISIDs */
+ u64 dev_res_bin_isid;
+ t10_task_attr_index_t dev_task_attr_type;
+ unsigned long long dev_sectors_total;
+ /* Pointer to transport specific device structure */
+ void *dev_ptr;
+ u32 dev_index;
+ u64 creation_time;
+ u32 num_resets;
+ u64 num_cmds;
+ u64 read_bytes;
+ u64 write_bytes;
+ spinlock_t stats_lock;
+ /* Active commands on this virtual SE device */
+ atomic_t active_cmds;
+ atomic_t simple_cmds;
+ atomic_t depth_left;
+ atomic_t dev_ordered_id;
+ atomic_t dev_tur_active;
+ atomic_t execute_tasks;
+ atomic_t dev_status_thr_count;
+ atomic_t dev_hoq_count;
+ atomic_t dev_ordered_sync;
+ struct se_obj dev_obj;
+ struct se_obj dev_access_obj;
+ struct se_obj dev_export_obj;
+ struct se_queue_obj *dev_queue_obj;
+ struct se_queue_obj *dev_status_queue_obj;
+ spinlock_t delayed_cmd_lock;
+ spinlock_t ordered_cmd_lock;
+ spinlock_t execute_task_lock;
+ spinlock_t state_task_lock;
+ spinlock_t dev_alua_lock;
+ spinlock_t dev_reservation_lock;
+ spinlock_t dev_state_lock;
+ spinlock_t dev_status_lock;
+ spinlock_t dev_status_thr_lock;
+ spinlock_t se_port_lock;
+ spinlock_t se_tmr_lock;
+ /* Used for legacy SPC-2 reservationsa */
+ struct se_node_acl *dev_reserved_node_acl;
+ /* Used for ALUA Logical Unit Group membership */
+ struct t10_alua_lu_gp_member *dev_alua_lu_gp_mem;
+ /* Used for SPC-3 Persistent Reservations */
+ struct t10_pr_registration *dev_pr_res_holder;
+ struct list_head dev_sep_list;
+ struct list_head dev_tmr_list;
+ struct timer_list dev_status_timer;
+ /* Pointer to descriptor for processing thread */
+ struct task_struct *process_thread;
+ pid_t process_thread_pid;
+ struct task_struct *dev_mgmt_thread;
+ int (*write_pending)(struct se_task *);
+ void (*dev_generate_cdb)(unsigned long long, u32 *,
+ unsigned char *, int);
+ struct list_head delayed_cmd_list;
+ struct list_head ordered_cmd_list;
+ struct list_head execute_task_list;
+ struct list_head state_task_list;
+ /* Pointer to associated SE HBA */
+ struct se_hba *se_hba;
+ struct se_subsystem_dev *se_sub_dev;
+ /* Pointer to template of function pointers for transport */
+ struct se_subsystem_api *transport;
+ /* Linked list for struct se_hba struct se_device list */
+ struct list_head dev_list;
+ /* Linked list for struct se_global->g_se_dev_list */
+ struct list_head g_se_dev_list;
+} ____cacheline_aligned;
+
+#define SE_DEV(cmd) ((struct se_device *)(cmd)->se_lun->lun_se_dev)
+#define SU_DEV(dev) ((struct se_subsystem_dev *)(dev)->se_sub_dev)
+#define ISCSI_DEV(cmd) SE_DEV(cmd)
+#define DEV_ATTRIB(dev) (&(dev)->se_sub_dev->se_dev_attrib)
+#define DEV_T10_WWN(dev) (&(dev)->se_sub_dev->t10_wwn)
+
+struct se_hba {
+ u16 hba_tpgt;
+ u32 hba_status;
+ u32 hba_id;
+ u32 hba_flags;
+ /* Virtual iSCSI devices attached. */
+ u32 dev_count;
+ u32 hba_index;
+ atomic_t dev_mib_access_count;
+ atomic_t load_balance_queue;
+ atomic_t left_queue_depth;
+ /* Maximum queue depth the HBA can handle. */
+ atomic_t max_queue_depth;
+ /* Pointer to transport specific host structure. */
+ void *hba_ptr;
+ /* Linked list for struct se_device */
+ struct list_head hba_dev_list;
+ struct list_head hba_list;
+ spinlock_t device_lock;
+ spinlock_t hba_queue_lock;
+ struct config_group hba_group;
+ struct mutex hba_access_mutex;
+ struct se_subsystem_api *transport;
+} ____cacheline_aligned;
+
+#define ISCSI_HBA(d) ((struct se_hba *)(d)->se_hba)
+/* Using SE_HBA() for new code */
+#define SE_HBA(d) ((struct se_hba *)(d)->se_hba)
+
+struct se_lun {
+ int lun_type;
+ int lun_status;
+ u32 lun_access;
+ u32 lun_flags;
+ u32 unpacked_lun;
+ atomic_t lun_acl_count;
+ spinlock_t lun_acl_lock;
+ spinlock_t lun_cmd_lock;
+ spinlock_t lun_sep_lock;
+ struct completion lun_shutdown_comp;
+ struct list_head lun_cmd_list;
+ struct list_head lun_acl_list;
+ struct se_device *lun_se_dev;
+ struct config_group lun_group;
+ struct se_port *lun_sep;
+} ____cacheline_aligned;
+
+#define SE_LUN(c) ((struct se_lun *)(c)->se_lun)
+#define ISCSI_LUN(c) SE_LUN(c)
+
+struct se_port {
+ /* RELATIVE TARGET PORT IDENTIFER */
+ u16 sep_rtpi;
+ int sep_tg_pt_secondary_stat;
+ int sep_tg_pt_secondary_write_md;
+ u32 sep_index;
+ struct scsi_port_stats sep_stats;
+ /* Used for ALUA Target Port Groups membership */
+ atomic_t sep_tg_pt_gp_active;
+ atomic_t sep_tg_pt_secondary_offline;
+ /* Used for PR ALL_TG_PT=1 */
+ atomic_t sep_tg_pt_ref_cnt;
+ spinlock_t sep_alua_lock;
+ struct mutex sep_tg_pt_md_mutex;
+ struct t10_alua_tg_pt_gp_member *sep_alua_tg_pt_gp_mem;
+ struct se_lun *sep_lun;
+ struct se_portal_group *sep_tpg;
+ struct list_head sep_alua_list;
+ struct list_head sep_list;
+} ____cacheline_aligned;
+
+struct se_tpg_np {
+ struct config_group tpg_np_group;
+} ____cacheline_aligned;
+
+struct se_portal_group {
+ /* Type of target portal group */
+ int se_tpg_type;
+ /* Number of ACLed Initiator Nodes for this TPG */
+ u32 num_node_acls;
+ /* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
+ atomic_t tpg_pr_ref_count;
+ /* Spinlock for adding/removing ACLed Nodes */
+ spinlock_t acl_node_lock;
+ /* Spinlock for adding/removing sessions */
+ spinlock_t session_lock;
+ spinlock_t tpg_lun_lock;
+ /* Pointer to $FABRIC_MOD portal group */
+ void *se_tpg_fabric_ptr;
+ struct list_head se_tpg_list;
+ /* linked list for initiator ACL list */
+ struct list_head acl_node_list;
+ struct se_lun *tpg_lun_list;
+ struct se_lun tpg_virt_lun0;
+ /* List of TCM sessions assoicated wth this TPG */
+ struct list_head tpg_sess_list;
+ /* Pointer to $FABRIC_MOD dependent code */
+ struct target_core_fabric_ops *se_tpg_tfo;
+ struct se_wwn *se_tpg_wwn;
+ struct config_group tpg_group;
+ struct config_group *tpg_default_groups[6];
+ struct config_group tpg_lun_group;
+ struct config_group tpg_np_group;
+ struct config_group tpg_acl_group;
+ struct config_group tpg_attrib_group;
+ struct config_group tpg_param_group;
+} ____cacheline_aligned;
+
+#define TPG_TFO(se_tpg) ((struct target_core_fabric_ops *)(se_tpg)->se_tpg_tfo)
+
+struct se_wwn {
+ struct target_fabric_configfs *wwn_tf;
+ struct config_group wwn_group;
+} ____cacheline_aligned;
+
+struct se_global {
+ u16 alua_lu_gps_counter;
+ int g_sub_api_initialized;
+ u32 in_shutdown;
+ u32 alua_lu_gps_count;
+ u32 g_hba_id_counter;
+ struct config_group target_core_hbagroup;
+ struct config_group alua_group;
+ struct config_group alua_lu_gps_group;
+ struct list_head g_lu_gps_list;
+ struct list_head g_se_tpg_list;
+ struct list_head g_hba_list;
+ struct list_head g_se_dev_list;
+ struct list_head g_sub_api_list;
+ struct se_hba *g_lun0_hba;
+ struct se_subsystem_dev *g_lun0_su_dev;
+ struct se_device *g_lun0_dev;
+ struct t10_alua_lu_gp *default_lu_gp;
+ struct mutex g_sub_api_mutex;
+ spinlock_t g_device_lock;
+ spinlock_t hba_lock;
+ spinlock_t se_tpg_lock;
+ spinlock_t lu_gps_lock;
+ spinlock_t plugin_class_lock;
+#ifdef DEBUG_DEV
+ spinlock_t debug_dev_lock;
+#endif
+} ____cacheline_aligned;
+
+#endif /* TARGET_CORE_BASE_H */
diff --git a/include/target/target_core_fabric_ops.h b/include/target/target_core_fabric_ops.h
new file mode 100644
index 0000000..3bc7fd6
--- /dev/null
+++ b/include/target/target_core_fabric_ops.h
@@ -0,0 +1,89 @@
+/* Defined in target_core_configfs.h */
+struct target_fabric_configfs;
+
+struct target_core_fabric_ops {
+ struct configfs_subsystem *tf_subsys;
+ char *(*get_fabric_name)(void);
+ u8 (*get_fabric_proto_ident)(struct se_portal_group *);
+ char *(*tpg_get_wwn)(struct se_portal_group *);
+ u16 (*tpg_get_tag)(struct se_portal_group *);
+ u32 (*tpg_get_default_depth)(struct se_portal_group *);
+ u32 (*tpg_get_pr_transport_id)(struct se_portal_group *,
+ struct se_node_acl *,
+ struct t10_pr_registration *, int *,
+ unsigned char *);
+ u32 (*tpg_get_pr_transport_id_len)(struct se_portal_group *,
+ struct se_node_acl *,
+ struct t10_pr_registration *, int *);
+ char *(*tpg_parse_pr_out_transport_id)(struct se_portal_group *,
+ const char *, u32 *, char **);
+ int (*tpg_check_demo_mode)(struct se_portal_group *);
+ int (*tpg_check_demo_mode_cache)(struct se_portal_group *);
+ int (*tpg_check_demo_mode_write_protect)(struct se_portal_group *);
+ int (*tpg_check_prod_mode_write_protect)(struct se_portal_group *);
+ struct se_node_acl *(*tpg_alloc_fabric_acl)(
+ struct se_portal_group *);
+ void (*tpg_release_fabric_acl)(struct se_portal_group *,
+ struct se_node_acl *);
+ u32 (*tpg_get_inst_index)(struct se_portal_group *);
+ /*
+ * Optional function pointer for TCM fabric modules that use
+ * Linux/NET sockets to allocate struct iovec array to struct se_cmd
+ */
+ int (*alloc_cmd_iovecs)(struct se_cmd *);
+ /*
+ * Optional to release struct se_cmd and fabric dependent allocated
+ * I/O descriptor in transport_cmd_check_stop()
+ */
+ void (*check_stop_free)(struct se_cmd *);
+ void (*release_cmd_to_pool)(struct se_cmd *);
+ void (*release_cmd_direct)(struct se_cmd *);
+ int (*dev_del_lun)(struct se_portal_group *, u32);
+ /*
+ * Called with spin_lock_bh(struct se_portal_group->session_lock held.
+ */
+ int (*shutdown_session)(struct se_session *);
+ void (*close_session)(struct se_session *);
+ void (*stop_session)(struct se_session *, int, int);
+ void (*fall_back_to_erl0)(struct se_session *);
+ int (*sess_logged_in)(struct se_session *);
+ u32 (*sess_get_index)(struct se_session *);
+ /*
+ * Used only for SCSI fabrics that contain multi-value TransportIDs
+ * (like iSCSI). All other SCSI fabrics should set this to NULL.
+ */
+ u32 (*sess_get_initiator_sid)(struct se_session *,
+ unsigned char *, u32);
+ int (*write_pending)(struct se_cmd *);
+ int (*write_pending_status)(struct se_cmd *);
+ void (*set_default_node_attributes)(struct se_node_acl *);
+ u32 (*get_task_tag)(struct se_cmd *);
+ int (*get_cmd_state)(struct se_cmd *);
+ void (*new_cmd_failure)(struct se_cmd *);
+ int (*queue_data_in)(struct se_cmd *);
+ int (*queue_status)(struct se_cmd *);
+ int (*queue_tm_rsp)(struct se_cmd *);
+ u16 (*set_fabric_sense_len)(struct se_cmd *, u32);
+ u16 (*get_fabric_sense_len)(void);
+ int (*is_state_remove)(struct se_cmd *);
+ u64 (*pack_lun)(unsigned int);
+ /*
+ * fabric module calls for target_core_fabric_configfs.c
+ */
+ struct se_wwn *(*fabric_make_wwn)(struct target_fabric_configfs *,
+ struct config_group *, const char *);
+ void (*fabric_drop_wwn)(struct se_wwn *);
+ struct se_portal_group *(*fabric_make_tpg)(struct se_wwn *,
+ struct config_group *, const char *);
+ void (*fabric_drop_tpg)(struct se_portal_group *);
+ int (*fabric_post_link)(struct se_portal_group *,
+ struct se_lun *);
+ void (*fabric_pre_unlink)(struct se_portal_group *,
+ struct se_lun *);
+ struct se_tpg_np *(*fabric_make_np)(struct se_portal_group *,
+ struct config_group *, const char *);
+ void (*fabric_drop_np)(struct se_tpg_np *);
+ struct se_node_acl *(*fabric_make_nodeacl)(struct se_portal_group *,
+ struct config_group *, const char *);
+ void (*fabric_drop_nodeacl)(struct se_node_acl *);
+};
--
1.7.2.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops
2010-08-30 9:20 [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops Nicholas A. Bellinger
@ 2010-09-02 5:56 ` Konrad Rzeszutek Wilk
2010-09-02 19:19 ` Nicholas A. Bellinger
0 siblings, 1 reply; 11+ messages in thread
From: Konrad Rzeszutek Wilk @ 2010-09-02 5:56 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: linux-scsi, linux-kernel, FUJITA Tomonori, Mike Christie,
Christoph Hellwig, Hannes Reinecke, James Bottomley, Jens Axboe,
Boaz Harrosh
On Monday 30 August 2010 05:20:40 Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
Hey Nicholas,
I did a cursory review .
>
> This patch adds TCM Core v4 base data structures definitions + macros
> and fabric module function pointer API in struct target_core_fabric_ops.
>
> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> ---
> include/target/target_core_base.h | 985
> +++++++++++++++++++++++++++++++ include/target/target_core_fabric_ops.h |
> 89 +++
> 2 files changed, 1074 insertions(+), 0 deletions(-)
> create mode 100644 include/target/target_core_base.h
> create mode 100644 include/target/target_core_fabric_ops.h
>
> diff --git a/include/target/target_core_base.h
> b/include/target/target_core_base.h new file mode 100644
> index 0000000..f4d0c70
> --- /dev/null
> +++ b/include/target/target_core_base.h
> @@ -0,0 +1,985 @@
> +#ifndef TARGET_CORE_BASE_H
> +#define TARGET_CORE_BASE_H
> +
> +#include <linux/in.h>
> +#include <linux/configfs.h>
> +#include <net/sock.h>
> +#include <net/tcp.h>
> +#include "target_core_mib.h"
> +
> +#define TARGET_CORE_MOD_VERSION "v4.0.0-rc3"
> +#define SHUTDOWN_SIGS (sigmask(SIGKILL)|sigmask(SIGINT)|sigmask(SIGABRT))
> +
> +/* SCSI Command Descriptor Block Size a la SCSI's MAX_COMMAND_SIZE */
> +#define SCSI_CDB_SIZE 16
> +#define TRANSPORT_IOV_DATA_BUFFER 5
> +
> +/* Maximum Number of LUNs per Target Portal Group */
> +#define TRANSPORT_MAX_LUNS_PER_TPG 256
> +
> +/* From include/scsi/scsi_cmnd.h:SCSI_SENSE_BUFFERSIZE */
> +#define TRANSPORT_SENSE_BUFFER SCSI_SENSE_BUFFERSIZE
> +
> +#define SPC_SENSE_KEY_OFFSET 2
> +#define SPC_ASC_KEY_OFFSET 12
> +#define SPC_ASCQ_KEY_OFFSET 13
> +
.. from here on
> +/* Currently same as ISCSI_IQN_LEN */
> +#define TRANSPORT_IQN_LEN 224
> +#define LU_GROUP_NAME_BUF 256
> +#define TG_PT_GROUP_NAME_BUF 256
> +/* Used to parse VPD into struct t10_vpd */
> +#define VPD_TMP_BUF_SIZE 128
> +/* Used for target_core-pscsi.c:pscsi_transport_complete() */
> +#define VPD_BUF_LEN 256
> +/* Used for struct se_subsystem_dev-->se_dev_alias, must be less than
> + PAGE_SIZE */
> +#define SE_DEV_ALIAS_LEN 512
> +/* Used for struct se_subsystem_dev->se_dev_udev_path[], must be less than
> + PAGE_SIZE */
> +#define SE_UDEV_PATH_LEN 512
> +/* Used for struct se_dev_snap_attrib->contact */
> +#define SNAP_CONTACT_LEN 128
> +/* Used for struct se_dev_snap_attrib->lv_group */
> +#define SNAP_GROUP_LEN 128
> +/* Used for struct se_dev_snap_attrib->lvc_size */
> +#define SNAP_LVC_LEN 32
> +/* Used by struct t10_reservation_template->pr_[i,t]_port[] */
> +#define PR_APTPL_MAX_IPORT_LEN 256
> +#define PR_APTPL_MAX_TPORT_LEN 256
> +/* Used by struct t10_reservation_template->pr_aptpl_buf_len */
> +#define PR_APTPL_BUF_LEN 8192
> +/* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
> +#define ALUA_MD_BUF_LEN 1024
> +/* Used by struct t10_pr_registration->pr_reg_isid */
> +#define PR_REG_ISID_LEN 16
> +/* PR_REG_ISID_LEN + ',i,0x' */
> +#define PR_REG_ISID_ID_LEN (PR_REG_ISID_LEN + 5)
to here I am unsure which one of these are defined by a spec (like the IQN
length) and which ones are Linux nomenclature. Perhaps it might be prudent to
split them in two camps. And how did you come up with some of the length
values that don't correspond to a spec?
> +
> +/* used by PSCSI and iBlock Transport drivers */
> +#define READ_BLOCK_LEN 6
> +#define READ_CAP_LEN 8
> +#define READ_POSITION_LEN 20
> +#define INQUIRY_LEN 36
> +#define INQUIRY_VPD_SERIAL_LEN 254
> +#define INQUIRY_VPD_DEVICE_IDENTIFIER_LEN 254
> +
> +/* struct se_cmd->data_direction */
> +#define SE_DIRECTION_NONE 0
> +#define SE_DIRECTION_READ 1
> +#define SE_DIRECTION_WRITE 2
> +#define SE_DIRECTION_BIDI 3
Ugh, how about an enum instead?
> +
> +/* struct se_hba->hba_flags */
> +#define HBA_FLAGS_INTERNAL_USE 0x00000001
> +#define HBA_FLAGS_PSCSI_MODE 0x00000002
> +
> +/* struct se_hba->hba_status and iscsi_tpg_hba->thba_status */
> +#define HBA_STATUS_FREE 0x00000001
> +#define HBA_STATUS_ACTIVE 0x00000002
> +#define HBA_STATUS_INACTIVE 0x00000004
> +#define HBA_STATUS_SHUTDOWN 0x00000008
> +
> +/* struct se_lun->lun_status */
> +#define TRANSPORT_LUN_STATUS_FREE 0
> +#define TRANSPORT_LUN_STATUS_ACTIVE 1
> +
> +/* struct se_lun->lun_type */
> +#define TRANSPORT_LUN_TYPE_NONE 0
> +#define TRANSPORT_LUN_TYPE_DEVICE 1
> +
> +/* struct se_portal_group->se_tpg_type */
> +#define TRANSPORT_TPG_TYPE_NORMAL 0
> +#define TRANSPORT_TPG_TYPE_DISCOVERY 1
These sound quite generic. Perhaps they should be moved to a more generic
header file?
> +
> +/* Used for se_node_acl->nodeacl_flags */
That "nodeacl" prefix for "flags" looks unnecessary. Why not
just have it defined as "se_node_acl->flags"
> +#define NAF_DYNAMIC_NODE_ACL 0x01
> +
> +/* struct se_map_sg->map_flags */
Ditto
> +#define MAP_SG_KMAP 0x01
> +
> +/* Used for generate timer flags */
> +#define TF_RUNNING 0x01
> +#define TF_STOP 0x02
> +
> +/* Special transport agnostic struct se_cmd->t_states */
> +#define TRANSPORT_NO_STATE 240
> +#define TRANSPORT_NEW_CMD 241
> +#define TRANSPORT_DEFERRED_CMD 242
> +#define TRANSPORT_WRITE_PENDING 243
> +#define TRANSPORT_PROCESS_WRITE 244
> +#define TRANSPORT_PROCESSING 245
> +#define TRANSPORT_COMPLETE_OK 246
> +#define TRANSPORT_COMPLETE_FAILURE 247
> +#define TRANSPORT_COMPLETE_TIMEOUT 248
> +#define TRANSPORT_PROCESS_TMR 249
> +#define TRANSPORT_TMR_COMPLETE 250
> +#define TRANSPORT_ISTATE_PROCESSING 251
> +#define TRANSPORT_ISTATE_PROCESSED 252
> +#define TRANSPORT_KILL 253
> +#define TRANSPORT_REMOVE 254
> +#define TRANSPORT_FREE 255
These really look generic to me. Perhaps you can move them to a separate
header file?
> +
> +#define SCF_SUPPORTED_SAM_OPCODE 0x00000001
> +#define SCF_TRANSPORT_TASK_SENSE 0x00000002
> +#define SCF_EMULATED_TASK_SENSE 0x00000004
> +#define SCF_SCSI_DATA_SG_IO_CDB 0x00000008
> +#define SCF_SCSI_CONTROL_SG_IO_CDB 0x00000010
> +#define SCF_SCSI_CONTROL_NONSG_IO_CDB 0x00000020
> +#define SCF_SCSI_NON_DATA_CDB 0x00000040
> +#define SCF_SCSI_CDB_EXCEPTION 0x00000080
> +#define SCF_SCSI_RESERVATION_CONFLICT 0x00000100
> +#define SCF_CMD_PASSTHROUGH 0x00000200
> +#define SCF_CMD_PASSTHROUGH_NOALLOC 0x00000400
> +#define SCF_SE_CMD_FAILED 0x00000800
> +#define SCF_SE_LUN_CMD 0x00001000
> +#define SCF_SE_ALLOW_EOO 0x00002000
> +#define SCF_SE_DISABLE_ONLINE_CHECK 0x00004000
> +#define SCF_SENT_CHECK_CONDITION 0x00008000
> +#define SCF_OVERFLOW_BIT 0x00010000
> +#define SCF_UNDERFLOW_BIT 0x00020000
> +#define SCF_SENT_DELAYED_TAS 0x00040000
> +#define SCF_ALUA_NON_OPTIMIZED 0x00080000
> +#define SCF_DELAYED_CMD_FROM_SAM_ATTR 0x00100000
> +#define SCF_PASSTHROUGH_SG_TO_MEM 0x00200000
> +#define SCF_PASSTHROUGH_CONTIG_TO_SG 0x00400000
> +#define SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC 0x00800000
> +#define SCF_EMULATE_SYNC_CACHE 0x01000000
> +#define SCF_EMULATE_CDB_ASYNC 0x02000000
Maybe an explanation what 'SCF' stands for?
> +
> +/* struct se_device->type */
> +#define PSCSI 1
> +#define STGT 2
> +#define PATA 3
> +#define IBLOCK 4
> +#define RAMDISK_DR 5
> +#define RAMDISK_MCP 6
> +#define FILEIO 7
> +#define VROM 8
> +#define VTAPE 9
> +#define MEDIA_CHANGER 10
Enum?
> +
> +/* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
> +#define TRANSPORT_LUNFLAGS_NO_ACCESS 0x00000000
> +#define TRANSPORT_LUNFLAGS_INITIATOR_ACCESS 0x00000001
> +#define TRANSPORT_LUNFLAGS_READ_ONLY 0x00000002
> +#define TRANSPORT_LUNFLAGS_READ_WRITE 0x00000004
> +
> +/* struct se_device->dev_status */
> +#define TRANSPORT_DEVICE_ACTIVATED 0x01
> +#define TRANSPORT_DEVICE_DEACTIVATED 0x02
> +#define TRANSPORT_DEVICE_QUEUE_FULL 0x04
> +#define TRANSPORT_DEVICE_SHUTDOWN 0x08
> +#define TRANSPORT_DEVICE_OFFLINE_ACTIVATED 0x10
> +#define TRANSPORT_DEVICE_OFFLINE_DEACTIVATED 0x20
Hmm.. Maybe it might be prudent to restructure this file altogether so that
the #defines are right next to the variables. Something akin to how 'ehci.h'
has it setup?
It would make it so much simpler to figure out what are the possible states of
a variable in a structure.
> +
> +#define DEV_STATUS_THR_TUR 1
> +#define DEV_STATUS_THR_TAKE_ONLINE 2
> +#define DEV_STATUS_THR_TAKE_OFFLINE 3
> +#define DEV_STATUS_THR_SHUTDOWN 4
For most if not all of the #define you mentioned to what the variable the
#defines correspond to. You missed it for these four above.
> +
> +/* struct se_dev_entry->deve_flags */
> +#define DEF_PR_REGISTERED 0x01
> +
> +/* Used for struct t10_pr_registration->pr_reg_flags */
> +#define PRF_ISID_PRESENT_AT_REG 0x01
> +
> +/* transport_send_check_condition_and_sense() */
That is one lengthy function name, but I don't see it in this patch?
> +#define NON_EXISTENT_LUN 0x1
> +#define UNSUPPORTED_SCSI_OPCODE 0x2
> +#define INCORRECT_AMOUNT_OF_DATA 0x3
> +#define UNEXPECTED_UNSOLICITED_DATA 0x4
> +#define SERVICE_CRC_ERROR 0x5
> +#define SNACK_REJECTED 0x6
> +#define SECTOR_COUNT_TOO_MANY 0x7
> +#define INVALID_CDB_FIELD 0x8
> +#define INVALID_PARAMETER_LIST 0x9
> +#define LOGICAL_UNIT_COMMUNICATION_FAILURE 0xa
> +#define UNKNOWN_MODE_PAGE 0xb
> +#define WRITE_PROTECTED 0xc
> +#define CHECK_CONDITION_ABORT_CMD 0xd
> +#define CHECK_CONDITION_UNIT_ATTENTION 0xe
> +#define CHECK_CONDITION_NOT_READY 0xf
So these #defines have nothing to do with SAM values. And looking at
include/scsi/iscsi_proto.h, it also has an incremental list, but it has a
prefix.
Should these #defines have a prefix? Say TARGET_
> +
> +struct se_obj {
> + atomic_t obj_access_count;
> +} ____cacheline_aligned;
> +
> +typedef enum {
> + SPC_ALUA_PASSTHROUGH,
> + SPC2_ALUA_DISABLED,
> + SPC3_ALUA_EMULATED
> +} t10_alua_index_t;
Could you reference which spec has this defined?
>
> +typedef enum {
> + SAM_TASK_ATTR_PASSTHROUGH,
> + SAM_TASK_ATTR_UNTAGGED,
> + SAM_TASK_ATTR_EMULATED
> +} t10_task_attr_index_t;
> +
Ditto.
> +struct se_cmd;
> +
> +struct t10_alua {
> + t10_alua_index_t alua_type;
> + /* ALUA Target Port Group ID */
> + u16 alua_tg_pt_gps_counter;
> + u32 alua_tg_pt_gps_count;
So the ALUA tpg id is the alua_tg_pt_gps_counter or the alua_tg_pt_gps_count?
Or both? That is a mouthfull. Can you just call them 'counter' and 'count'
respectively?
> + spinlock_t tg_pt_gps_lock;
> + struct se_subsystem_dev *t10_sub_dev;
Just call it "sub_dev" ?
> + /* Used for default ALUA Target Port Group */
> + struct t10_alua_tg_pt_gp *default_tg_pt_gp;
You don't seem to define the "struct t10_alua_tg_pt_gp" - won't the compiler
throw a fit here? Can you call it 'default'?
> + /* Used for default ALUA Target Port Group ConfigFS group */
> + struct config_group alua_tg_pt_gps_group;
And here 'group' instead of 'alua_tg_pt_gps_group'?
> + int (*alua_state_check)(struct se_cmd *, unsigned char *, u8 *);
> + struct list_head tg_pt_gps_list;
how about just 'list' ?
> +} ____cacheline_aligned;
> +
> +struct t10_alua_lu_gp {
> + u16 lu_gp_id;
> + int lu_gp_valid_id;
> + u32 lu_gp_members;
> + atomic_t lu_gp_shutdown;
> + atomic_t lu_gp_ref_cnt;
> + spinlock_t lu_gp_lock;
> + struct config_group lu_gp_group;
> + struct list_head lu_gp_list;
> + struct list_head lu_gp_mem_list;
How about getting rid of the 'lu_gp' prefix for all members?
> +} ____cacheline_aligned;
> +
> +struct t10_alua_lu_gp_member {
> + int lu_gp_assoc;
> + atomic_t lu_gp_mem_ref_cnt;
> + spinlock_t lu_gp_mem_lock;
> + struct t10_alua_lu_gp *lu_gp;
> + struct se_device *lu_gp_mem_dev;
> + struct list_head lu_gp_mem_list;
Ditto
> +} ____cacheline_aligned;
> +
> +struct t10_alua_tg_pt_gp {
> + u16 tg_pt_gp_id;
> + int tg_pt_gp_valid_id;
> + int tg_pt_gp_alua_access_status;
> + int tg_pt_gp_alua_access_type;
> + int tg_pt_gp_nonop_delay_msecs;
> + int tg_pt_gp_trans_delay_msecs;
> + int tg_pt_gp_pref;
> + int tg_pt_gp_write_metadata;
> + u32 tg_pt_gp_md_buf_len;
> + u32 tg_pt_gp_members;
> + atomic_t tg_pt_gp_alua_access_state;
> + atomic_t tg_pt_gp_ref_cnt;
> + spinlock_t tg_pt_gp_lock;
> + struct mutex tg_pt_gp_md_mutex;
> + struct se_subsystem_dev *tg_pt_gp_su_dev;
> + struct config_group tg_pt_gp_group;
> + struct list_head tg_pt_gp_list;
> + struct list_head tg_pt_gp_mem_list;
Ditto
> +} ____cacheline_aligned;
> +
> +struct t10_alua_tg_pt_gp_member {
> + int tg_pt_gp_assoc;
> + atomic_t tg_pt_gp_mem_ref_cnt;
> + spinlock_t tg_pt_gp_mem_lock;
> + struct t10_alua_tg_pt_gp *tg_pt_gp;
> + struct se_port *tg_pt;
> + struct list_head tg_pt_gp_mem_list;
Ditto
> +} ____cacheline_aligned;
> +
> +struct t10_vpd {
> + unsigned char device_identifier[INQUIRY_VPD_DEVICE_IDENTIFIER_LEN];
> + int protocol_identifier_set;
> + u32 protocol_identifier;
> + u32 device_identifier_code_set;
> + u32 association;
> + u32 device_identifier_type;
> + struct list_head vpd_list;
> +} ____cacheline_aligned;
The 't10' makes this sound official. If so, can you provide in the comment
section the appropriate spec number?
> +
> +struct t10_wwn {
> + unsigned char vendor[8];
> + unsigned char model[16];
> + unsigned char revision[4];
> + unsigned char unit_serial[INQUIRY_VPD_SERIAL_LEN];
> + spinlock_t t10_vpd_lock;
vpd_lock or wwn_lock? Why do you need a spinlock for this structure?
BTW, if you run checkpatch I think it throws a fit if you don't document the
spinlock usage. You did use checkpatch.pl ,right?
> + struct se_subsystem_dev *t10_sub_dev;
How about 'sub_dev' instead?
> + struct config_group t10_wwn_group;
Take out the 't10_wwn'
> + struct list_head t10_vpd_list;
And make this 'list' by itself.
> +} ____cacheline_aligned;
> +
> +typedef enum {
> + SPC_PASSTHROUGH,
> + SPC2_RESERVATIONS,
> + SPC3_PERSISTENT_RESERVATIONS
> +} t10_reservations_index_t;
Are those official values ? Spec number, page?
> +
> +struct t10_pr_registration {
> + /* Used for fabrics that contain WWN+ISID */
> + char pr_reg_isid[PR_REG_ISID_LEN];
> + /* Used during APTPL metadata reading */
> + unsigned char pr_iport[PR_APTPL_MAX_IPORT_LEN];
> + /* Used during APTPL metadata reading */
> + unsigned char pr_tport[PR_APTPL_MAX_TPORT_LEN];
> + /* For writing out live meta data */
> + unsigned char *pr_aptpl_buf;
> + u16 pr_aptpl_rpti;
> + u16 pr_reg_tpgt;
> + /* Reservation effects all target ports */
> + int pr_reg_all_tg_pt;
> + /* Activate Persistence across Target Power Loss */
> + int pr_reg_aptpl;
> + int pr_res_holder;
> + int pr_res_type;
> + int pr_res_scope;
> + u32 pr_reg_flags;
> + u32 pr_res_mapped_lun;
> + u32 pr_aptpl_target_lun;
> + u32 pr_res_generation;
> + u64 pr_reg_bin_isid;
> + u64 pr_res_key;
> + atomic_t pr_res_holders;
> + struct se_node_acl *pr_reg_nacl;
> + struct se_dev_entry *pr_reg_deve;
> + struct se_lun *pr_reg_tg_pt_lun;
> + struct list_head pr_reg_list;
> + struct list_head pr_reg_abort_list;
> + struct list_head pr_reg_aptpl_list;
> + struct list_head pr_reg_atp_list;
> + struct list_head pr_reg_atp_mem_list;
> +} ____cacheline_aligned;
> +
What is 'pr' ? Pringles? You could call that structure 'persist_reg' and
remove all of those 'pr_reg_' prefixes.
> +struct t10_reservation_template {
> + /* Reservation effects all target ports */
> + int pr_all_tg_pt;
Um, not sure what the comment has to do with 'pr_all_tg_pt' ? What
does 'pr_all_tg_tg' do?
> + /* Activate Persistence across Target Power Loss enabled
> + * for SCSI device */
> + int pr_aptpl_active;
Perhaps 'is_active' ?
> + u32 pr_aptpl_buf_len;
Where is the buf?
> + u32 pr_generation;
> + t10_reservations_index_t res_type;
> + spinlock_t registration_lock;
> + spinlock_t aptpl_reg_lock;
> + /* Reservation holder when pr_all_tg_pt=1 */
Ah, and when it is another value?
> + struct se_node_acl *pr_res_holder;
> + struct list_head registration_list;
> + struct list_head aptpl_reg_list;
> + int (*t10_reservation_check)(struct se_cmd *, u32 *);
> + int (*t10_seq_non_holder)(struct se_cmd *, unsigned char *, u32);
> + int (*t10_pr_register)(struct se_cmd *);
> + int (*t10_pr_clear)(struct se_cmd *);
You can get rid of the t10_ prefix.
> +} ____cacheline_aligned;
> +
This is the second structure I see where you mix data with functions.
I don't know what the actual StyleGuide is for this, but would it be prudent
to split the data (spinlocks, list, values) from the behavior (functions?)
This could be done by embedding another struct - which only has function
pointers?
> +struct se_queue_req {
> + int state;
> + void *cmd;
> + struct list_head qr_list;
> +} ____cacheline_aligned;
> +
> +struct se_queue_obj {
> + atomic_t queue_cnt;
> + spinlock_t cmd_queue_lock;
> + struct list_head qobj_list;
> + wait_queue_head_t thread_wq;
> + struct completion thread_create_comp;
> + struct completion thread_done_comp;
> +} ____cacheline_aligned;
> +
> +struct se_transport_task {
> + unsigned char t_task_cdb[SCSI_CDB_SIZE];
> + unsigned long long t_task_lba;
> + int t_tasks_failed;
> + int t_task_fua;
> + u32 t_task_cdbs;
> + u32 t_task_check;
> + u32 t_task_no;
> + u32 t_task_sectors;
> + u32 t_task_se_num;
> + atomic_t t_fe_count;
> + atomic_t t_se_count;
> + atomic_t t_task_cdbs_left;
> + atomic_t t_task_cdbs_ex_left;
> + atomic_t t_task_cdbs_timeout_left;
> + atomic_t t_task_cdbs_sent;
> + atomic_t t_transport_aborted;
> + atomic_t t_transport_active;
> + atomic_t t_transport_complete;
> + atomic_t t_transport_queue_active;
> + atomic_t t_transport_sent;
> + atomic_t t_transport_stop;
> + atomic_t t_transport_timeout;
> + atomic_t transport_dev_active;
> + atomic_t transport_lun_active;
> + atomic_t transport_lun_fe_stop;
> + atomic_t transport_lun_stop;
> + spinlock_t t_state_lock;
> + struct completion t_transport_stop_comp;
> + struct completion t_transport_passthrough_comp;
> + struct completion t_transport_passthrough_wcomp;
> + struct completion transport_lun_fe_stop_comp;
> + struct completion transport_lun_stop_comp;
> + void *t_task_buf;
> + void *t_task_pt_buf;
> + struct list_head t_task_list;
> + struct list_head *t_mem_list;
> +} ____cacheline_aligned;
I think you can remove the "t_" prefix. I am though a bit confused, the
struct is called 'transport_task' and there are values that are
transport_task related, but then there are also some that are clearly related
to a task:t_task_no, t_task_check? Should they just be plural?
> +
> +struct se_task {
> + unsigned char task_sense;
> + struct scatterlist *task_sg;
> + void *transport_req;
> + u8 task_scsi_status;
> + u8 task_flags;
> + int task_error_status;
> + int task_state_flags;
> + unsigned long long task_lba;
> + u32 task_no;
> + u32 task_sectors;
> + u32 task_size;
> + u32 task_sg_num;
> + u32 task_sg_offset;
> + struct se_cmd *task_se_cmd;
> + struct se_device *se_dev;
> + struct completion task_stop_comp;
> + atomic_t task_active;
> + atomic_t task_execute_queue;
> + atomic_t task_timeout;
> + atomic_t task_sent;
> + atomic_t task_stop;
> + atomic_t task_state_active;
> + struct timer_list task_timer;
> + int (*transport_map_task)(struct se_task *, u32);
> + struct se_device *se_obj_ptr;
> + struct list_head t_list;
> + struct list_head t_execute_list;
> + struct list_head t_state_list;
> +} ____cacheline_aligned;
> +
> +#define TASK_CMD(task) ((struct se_cmd *)task->task_se_cmd)
> +#define TASK_DEV(task) ((struct se_device *)task->se_dev)
> +
I stopped the review here. I think that using the ideas of how ehci.h mixes
the struct variables with #defines will make this much easier to read?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops
2010-09-02 5:56 ` Konrad Rzeszutek Wilk
@ 2010-09-02 19:19 ` Nicholas A. Bellinger
2010-09-04 23:53 ` Nicholas A. Bellinger
0 siblings, 1 reply; 11+ messages in thread
From: Nicholas A. Bellinger @ 2010-09-02 19:19 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-scsi, linux-kernel, FUJITA Tomonori, Mike Christie,
Christoph Hellwig, Hannes Reinecke, James Bottomley, Jens Axboe,
Boaz Harrosh
On Thu, 2010-09-02 at 01:56 -0400, Konrad Rzeszutek Wilk wrote:
> On Monday 30 August 2010 05:20:40 Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> Hey Nicholas,
>
> I did a cursory review .
>
> >
> > This patch adds TCM Core v4 base data structures definitions + macros
> > and fabric module function pointer API in struct target_core_fabric_ops.
> >
> > Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> > ---
> > include/target/target_core_base.h | 985
> > +++++++++++++++++++++++++++++++ include/target/target_core_fabric_ops.h |
> > 89 +++
> > 2 files changed, 1074 insertions(+), 0 deletions(-)
> > create mode 100644 include/target/target_core_base.h
> > create mode 100644 include/target/target_core_fabric_ops.h
> >
> > diff --git a/include/target/target_core_base.h
> > b/include/target/target_core_base.h new file mode 100644
> > index 0000000..f4d0c70
> > --- /dev/null
> > +++ b/include/target/target_core_base.h
> > @@ -0,0 +1,985 @@
> > +#ifndef TARGET_CORE_BASE_H
> > +#define TARGET_CORE_BASE_H
> > +
> > +#include <linux/in.h>
> > +#include <linux/configfs.h>
> > +#include <net/sock.h>
> > +#include <net/tcp.h>
> > +#include "target_core_mib.h"
> > +
> > +#define TARGET_CORE_MOD_VERSION "v4.0.0-rc3"
> > +#define SHUTDOWN_SIGS (sigmask(SIGKILL)|sigmask(SIGINT)|sigmask(SIGABRT))
> > +
> > +/* SCSI Command Descriptor Block Size a la SCSI's MAX_COMMAND_SIZE */
> > +#define SCSI_CDB_SIZE 16
> > +#define TRANSPORT_IOV_DATA_BUFFER 5
> > +
> > +/* Maximum Number of LUNs per Target Portal Group */
> > +#define TRANSPORT_MAX_LUNS_PER_TPG 256
> > +
> > +/* From include/scsi/scsi_cmnd.h:SCSI_SENSE_BUFFERSIZE */
> > +#define TRANSPORT_SENSE_BUFFER SCSI_SENSE_BUFFERSIZE
> > +
> > +#define SPC_SENSE_KEY_OFFSET 2
> > +#define SPC_ASC_KEY_OFFSET 12
> > +#define SPC_ASCQ_KEY_OFFSET 13
> > +
>
> .. from here on
> > +/* Currently same as ISCSI_IQN_LEN */
> > +#define TRANSPORT_IQN_LEN 224
> > +#define LU_GROUP_NAME_BUF 256
> > +#define TG_PT_GROUP_NAME_BUF 256
> > +/* Used to parse VPD into struct t10_vpd */
> > +#define VPD_TMP_BUF_SIZE 128
> > +/* Used for target_core-pscsi.c:pscsi_transport_complete() */
> > +#define VPD_BUF_LEN 256
> > +/* Used for struct se_subsystem_dev-->se_dev_alias, must be less than
> > + PAGE_SIZE */
> > +#define SE_DEV_ALIAS_LEN 512
> > +/* Used for struct se_subsystem_dev->se_dev_udev_path[], must be less than
> > + PAGE_SIZE */
> > +#define SE_UDEV_PATH_LEN 512
> > +/* Used for struct se_dev_snap_attrib->contact */
> > +#define SNAP_CONTACT_LEN 128
> > +/* Used for struct se_dev_snap_attrib->lv_group */
> > +#define SNAP_GROUP_LEN 128
> > +/* Used for struct se_dev_snap_attrib->lvc_size */
> > +#define SNAP_LVC_LEN 32
> > +/* Used by struct t10_reservation_template->pr_[i,t]_port[] */
> > +#define PR_APTPL_MAX_IPORT_LEN 256
> > +#define PR_APTPL_MAX_TPORT_LEN 256
> > +/* Used by struct t10_reservation_template->pr_aptpl_buf_len */
> > +#define PR_APTPL_BUF_LEN 8192
> > +/* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
> > +#define ALUA_MD_BUF_LEN 1024
> > +/* Used by struct t10_pr_registration->pr_reg_isid */
> > +#define PR_REG_ISID_LEN 16
> > +/* PR_REG_ISID_LEN + ',i,0x' */
> > +#define PR_REG_ISID_ID_LEN (PR_REG_ISID_LEN + 5)
>
> to here I am unsure which one of these are defined by a spec (like the IQN
> length) and which ones are Linux nomenclature. Perhaps it might be prudent to
> split them in two camps.
Hmm, TRANSPORT_IQN_LEN is indeed a special case for the Initiator Port
WWPN in struct se_node_acl->initiatorname[]. We could always make this
fabric module size dependent and do a special allocation for this, but I
am not the benefit is really worth it.
> And how did you come up with some of the length
> values that don't correspond to a spec?
Well in the case of the PR APTPL defs, I tried to estimate what I
thought would be required for future fabrics. For the others like the
LU_GROUP_NAME_BUF and TG_PT_GROUP_NAME_BUF related to ALUA, I thought
these where reasonable limits.
> > +
> > +/* used by PSCSI and iBlock Transport drivers */
> > +#define READ_BLOCK_LEN 6
> > +#define READ_CAP_LEN 8
> > +#define READ_POSITION_LEN 20
> > +#define INQUIRY_LEN 36
> > +#define INQUIRY_VPD_SERIAL_LEN 254
> > +#define INQUIRY_VPD_DEVICE_IDENTIFIER_LEN 254
> > +
> > +/* struct se_cmd->data_direction */
> > +#define SE_DIRECTION_NONE 0
> > +#define SE_DIRECTION_READ 1
> > +#define SE_DIRECTION_WRITE 2
> > +#define SE_DIRECTION_BIDI 3
>
> Ugh, how about an enum instead?
Sure, fair enough.
>
> > +
> > +/* struct se_hba->hba_flags */
> > +#define HBA_FLAGS_INTERNAL_USE 0x00000001
> > +#define HBA_FLAGS_PSCSI_MODE 0x00000002
> > +
> > +/* struct se_hba->hba_status and iscsi_tpg_hba->thba_status */
> > +#define HBA_STATUS_FREE 0x00000001
> > +#define HBA_STATUS_ACTIVE 0x00000002
> > +#define HBA_STATUS_INACTIVE 0x00000004
> > +#define HBA_STATUS_SHUTDOWN 0x00000008
> > +
>
> > +/* struct se_lun->lun_status */
> > +#define TRANSPORT_LUN_STATUS_FREE 0
> > +#define TRANSPORT_LUN_STATUS_ACTIVE 1
> > +
> > +/* struct se_lun->lun_type */
> > +#define TRANSPORT_LUN_TYPE_NONE 0
> > +#define TRANSPORT_LUN_TYPE_DEVICE 1
> > +
> > +/* struct se_portal_group->se_tpg_type */
> > +#define TRANSPORT_TPG_TYPE_NORMAL 0
> > +#define TRANSPORT_TPG_TYPE_DISCOVERY 1
>
> These sound quite generic. Perhaps they should be moved to a more generic
> header file?
Ok, these can go into include/target/target_core_tpg.h..
> > +
> > +/* Used for se_node_acl->nodeacl_flags */
>
> That "nodeacl" prefix for "flags" looks unnecessary. Why not
> just have it defined as "se_node_acl->flags"
I like having the prefix so that the code can be easily grepped.
> > +#define NAF_DYNAMIC_NODE_ACL 0x01
>
> > +
> > +/* struct se_map_sg->map_flags */
> Ditto
Same for this..
> > +#define MAP_SG_KMAP 0x01
>
> > +
> > +/* Used for generate timer flags */
> > +#define TF_RUNNING 0x01
> > +#define TF_STOP 0x02
> > +
> > +/* Special transport agnostic struct se_cmd->t_states */
> > +#define TRANSPORT_NO_STATE 240
> > +#define TRANSPORT_NEW_CMD 241
> > +#define TRANSPORT_DEFERRED_CMD 242
> > +#define TRANSPORT_WRITE_PENDING 243
> > +#define TRANSPORT_PROCESS_WRITE 244
> > +#define TRANSPORT_PROCESSING 245
> > +#define TRANSPORT_COMPLETE_OK 246
> > +#define TRANSPORT_COMPLETE_FAILURE 247
> > +#define TRANSPORT_COMPLETE_TIMEOUT 248
> > +#define TRANSPORT_PROCESS_TMR 249
> > +#define TRANSPORT_TMR_COMPLETE 250
> > +#define TRANSPORT_ISTATE_PROCESSING 251
> > +#define TRANSPORT_ISTATE_PROCESSED 252
> > +#define TRANSPORT_KILL 253
> > +#define TRANSPORT_REMOVE 254
> > +#define TRANSPORT_FREE 255
>
> These really look generic to me. Perhaps you can move them to a separate
> header file?
These are used by TCM Core and fabric modules to signal state, as these
are used in core code by target_core_transport.h code, moving these to
target_core_transport.h is fine by me..
> > +
> > +#define SCF_SUPPORTED_SAM_OPCODE 0x00000001
> > +#define SCF_TRANSPORT_TASK_SENSE 0x00000002
> > +#define SCF_EMULATED_TASK_SENSE 0x00000004
> > +#define SCF_SCSI_DATA_SG_IO_CDB 0x00000008
> > +#define SCF_SCSI_CONTROL_SG_IO_CDB 0x00000010
> > +#define SCF_SCSI_CONTROL_NONSG_IO_CDB 0x00000020
> > +#define SCF_SCSI_NON_DATA_CDB 0x00000040
> > +#define SCF_SCSI_CDB_EXCEPTION 0x00000080
> > +#define SCF_SCSI_RESERVATION_CONFLICT 0x00000100
> > +#define SCF_CMD_PASSTHROUGH 0x00000200
> > +#define SCF_CMD_PASSTHROUGH_NOALLOC 0x00000400
> > +#define SCF_SE_CMD_FAILED 0x00000800
> > +#define SCF_SE_LUN_CMD 0x00001000
> > +#define SCF_SE_ALLOW_EOO 0x00002000
> > +#define SCF_SE_DISABLE_ONLINE_CHECK 0x00004000
> > +#define SCF_SENT_CHECK_CONDITION 0x00008000
> > +#define SCF_OVERFLOW_BIT 0x00010000
> > +#define SCF_UNDERFLOW_BIT 0x00020000
> > +#define SCF_SENT_DELAYED_TAS 0x00040000
> > +#define SCF_ALUA_NON_OPTIMIZED 0x00080000
> > +#define SCF_DELAYED_CMD_FROM_SAM_ATTR 0x00100000
> > +#define SCF_PASSTHROUGH_SG_TO_MEM 0x00200000
> > +#define SCF_PASSTHROUGH_CONTIG_TO_SG 0x00400000
> > +#define SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC 0x00800000
> > +#define SCF_EMULATE_SYNC_CACHE 0x01000000
> > +#define SCF_EMULATE_CDB_ASYNC 0x02000000
>
> Maybe an explanation what 'SCF' stands for?
Sure, I will add a comment here.. "Se_Cmd Flags"
> > +
> > +/* struct se_device->type */
> > +#define PSCSI 1
> > +#define STGT 2
> > +#define PATA 3
> > +#define IBLOCK 4
> > +#define RAMDISK_DR 5
> > +#define RAMDISK_MCP 6
> > +#define FILEIO 7
> > +#define VROM 8
> > +#define VTAPE 9
> > +#define MEDIA_CHANGER 10
>
> Enum?
Sure.
> > +
> > +/* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
> > +#define TRANSPORT_LUNFLAGS_NO_ACCESS 0x00000000
> > +#define TRANSPORT_LUNFLAGS_INITIATOR_ACCESS 0x00000001
> > +#define TRANSPORT_LUNFLAGS_READ_ONLY 0x00000002
> > +#define TRANSPORT_LUNFLAGS_READ_WRITE 0x00000004
> > +
> > +/* struct se_device->dev_status */
> > +#define TRANSPORT_DEVICE_ACTIVATED 0x01
> > +#define TRANSPORT_DEVICE_DEACTIVATED 0x02
> > +#define TRANSPORT_DEVICE_QUEUE_FULL 0x04
> > +#define TRANSPORT_DEVICE_SHUTDOWN 0x08
> > +#define TRANSPORT_DEVICE_OFFLINE_ACTIVATED 0x10
> > +#define TRANSPORT_DEVICE_OFFLINE_DEACTIVATED 0x20
>
> Hmm.. Maybe it might be prudent to restructure this file altogether so that
> the #defines are right next to the variables. Something akin to how 'ehci.h'
> has it setup?
>
Hmmmm, fair point. That would make things a bit easier to read.
> It would make it so much simpler to figure out what are the possible states of
> a variable in a structure.
>
> > +
> > +#define DEV_STATUS_THR_TUR 1
> > +#define DEV_STATUS_THR_TAKE_ONLINE 2
> > +#define DEV_STATUS_THR_TAKE_OFFLINE 3
> > +#define DEV_STATUS_THR_SHUTDOWN 4
>
> For most if not all of the #define you mentioned to what the variable the
> #defines correspond to. You missed it for these four above.
>
Actually, this is dead code from 2.x that I will drop.
> > +
> > +/* struct se_dev_entry->deve_flags */
> > +#define DEF_PR_REGISTERED 0x01
> > +
> > +/* Used for struct t10_pr_registration->pr_reg_flags */
> > +#define PRF_ISID_PRESENT_AT_REG 0x01
>
> > +
> > +/* transport_send_check_condition_and_sense() */
>
> That is one lengthy function name, but I don't see it in this patch?
>
This one lives in drivers/target/target_core_transport.c
> > +#define NON_EXISTENT_LUN 0x1
> > +#define UNSUPPORTED_SCSI_OPCODE 0x2
> > +#define INCORRECT_AMOUNT_OF_DATA 0x3
> > +#define UNEXPECTED_UNSOLICITED_DATA 0x4
> > +#define SERVICE_CRC_ERROR 0x5
> > +#define SNACK_REJECTED 0x6
> > +#define SECTOR_COUNT_TOO_MANY 0x7
> > +#define INVALID_CDB_FIELD 0x8
> > +#define INVALID_PARAMETER_LIST 0x9
> > +#define LOGICAL_UNIT_COMMUNICATION_FAILURE 0xa
> > +#define UNKNOWN_MODE_PAGE 0xb
> > +#define WRITE_PROTECTED 0xc
> > +#define CHECK_CONDITION_ABORT_CMD 0xd
> > +#define CHECK_CONDITION_UNIT_ATTENTION 0xe
> > +#define CHECK_CONDITION_NOT_READY 0xf
>
> So these #defines have nothing to do with SAM values. And looking at
> include/scsi/iscsi_proto.h, it also has an incremental list, but it has a
> prefix.
>
Yes, these are used by TCM to signal which ASC/ASCQ payload should be
built in transport_send_check_condition_and_sense().
To prevent namespace pollution I am happy to add a prefix to these..
> Should these #defines have a prefix? Say TARGET_
> > +
> > +struct se_obj {
> > + atomic_t obj_access_count;
> > +} ____cacheline_aligned;
> > +
> > +typedef enum {
> > + SPC_ALUA_PASSTHROUGH,
> > + SPC2_ALUA_DISABLED,
> > + SPC3_ALUA_EMULATED
> > +} t10_alua_index_t;
>
> Could you reference which spec has this defined?
> >
> > +typedef enum {
> > + SAM_TASK_ATTR_PASSTHROUGH,
> > + SAM_TASK_ATTR_UNTAGGED,
> > + SAM_TASK_ATTR_EMULATED
> > +} t10_task_attr_index_t;
> > +
> Ditto.
>
Both of these are internal states for ALUA and Task Attr emulation in
order to known if the:
*) emulation is disabled for TCM/pSCSI passthrough (eg: the underlying
firmware will support it)
*) Emulation is diabled all together for non passthrough ops
*) Emulation is enabled
I think keeping the Task Attr emulation here makes sense, but I will go
ahead and move the ALUA ones to target_core_alua.h
> > +struct se_cmd;
> > +
> > +struct t10_alua {
> > + t10_alua_index_t alua_type;
> > + /* ALUA Target Port Group ID */
> > + u16 alua_tg_pt_gps_counter;
> > + u32 alua_tg_pt_gps_count;
>
> So the ALUA tpg id is the alua_tg_pt_gps_counter or the alua_tg_pt_gps_count?
> Or both? That is a mouthfull. Can you just call them 'counter' and 'count'
> respectively?
I agree that these are a bit long, but is really does make it nice for
grepping having these (erm, unique) prefixes.
>
> > + spinlock_t tg_pt_gps_lock;
> > + struct se_subsystem_dev *t10_sub_dev;
>
> Just call it "sub_dev" ?
Ditto on the prefix bit.. :-)
> > + /* Used for default ALUA Target Port Group */
> > + struct t10_alua_tg_pt_gp *default_tg_pt_gp;
>
> You don't seem to define the "struct t10_alua_tg_pt_gp" - won't the compiler
> throw a fit here? Can you call it 'default'?
>
> > + /* Used for default ALUA Target Port Group ConfigFS group */
> > + struct config_group alua_tg_pt_gps_group;
>
> And here 'group' instead of 'alua_tg_pt_gps_group'?
> > + int (*alua_state_check)(struct se_cmd *, unsigned char *, u8 *);
>
> > + struct list_head tg_pt_gps_list;
>
> how about just 'list' ?
> > +} ____cacheline_aligned;
>
> > +
> > +struct t10_alua_lu_gp {
> > + u16 lu_gp_id;
> > + int lu_gp_valid_id;
> > + u32 lu_gp_members;
> > + atomic_t lu_gp_shutdown;
> > + atomic_t lu_gp_ref_cnt;
> > + spinlock_t lu_gp_lock;
> > + struct config_group lu_gp_group;
> > + struct list_head lu_gp_list;
> > + struct list_head lu_gp_mem_list;
>
> How about getting rid of the 'lu_gp' prefix for all members?
> > +} ____cacheline_aligned;
> > +
> > +struct t10_alua_lu_gp_member {
> > + int lu_gp_assoc;
> > + atomic_t lu_gp_mem_ref_cnt;
> > + spinlock_t lu_gp_mem_lock;
> > + struct t10_alua_lu_gp *lu_gp;
> > + struct se_device *lu_gp_mem_dev;
> > + struct list_head lu_gp_mem_list;
>
> Ditto
> > +} ____cacheline_aligned;
> > +
> > +struct t10_alua_tg_pt_gp {
> > + u16 tg_pt_gp_id;
> > + int tg_pt_gp_valid_id;
> > + int tg_pt_gp_alua_access_status;
> > + int tg_pt_gp_alua_access_type;
> > + int tg_pt_gp_nonop_delay_msecs;
> > + int tg_pt_gp_trans_delay_msecs;
> > + int tg_pt_gp_pref;
> > + int tg_pt_gp_write_metadata;
> > + u32 tg_pt_gp_md_buf_len;
> > + u32 tg_pt_gp_members;
> > + atomic_t tg_pt_gp_alua_access_state;
> > + atomic_t tg_pt_gp_ref_cnt;
> > + spinlock_t tg_pt_gp_lock;
> > + struct mutex tg_pt_gp_md_mutex;
> > + struct se_subsystem_dev *tg_pt_gp_su_dev;
> > + struct config_group tg_pt_gp_group;
> > + struct list_head tg_pt_gp_list;
> > + struct list_head tg_pt_gp_mem_list;
>
> Ditto
> > +} ____cacheline_aligned;
> > +
> > +struct t10_alua_tg_pt_gp_member {
> > + int tg_pt_gp_assoc;
> > + atomic_t tg_pt_gp_mem_ref_cnt;
> > + spinlock_t tg_pt_gp_mem_lock;
> > + struct t10_alua_tg_pt_gp *tg_pt_gp;
> > + struct se_port *tg_pt;
> > + struct list_head tg_pt_gp_mem_list;
>
> Ditto
So yeah, I would like to keep the prefixes for grokability here.. 8-)
> > +} ____cacheline_aligned;
> > +
> > +struct t10_vpd {
> > + unsigned char device_identifier[INQUIRY_VPD_DEVICE_IDENTIFIER_LEN];
> > + int protocol_identifier_set;
> > + u32 protocol_identifier;
> > + u32 device_identifier_code_set;
> > + u32 association;
> > + u32 device_identifier_type;
> > + struct list_head vpd_list;
> > +} ____cacheline_aligned;
>
> The 't10' makes this sound official. If so, can you provide in the comment
> section the appropriate spec number?
Sure.
> > +
> > +struct t10_wwn {
> > + unsigned char vendor[8];
> > + unsigned char model[16];
> > + unsigned char revision[4];
> > + unsigned char unit_serial[INQUIRY_VPD_SERIAL_LEN];
> > + spinlock_t t10_vpd_lock;
>
> vpd_lock or wwn_lock? Why do you need a spinlock for this structure?
This lock protects the list of Device identifier (0x83) VPDs that are
scanned during struct se_device registration.
> BTW, if you run checkpatch I think it throws a fit if you don't document the
> spinlock usage. You did use checkpatch.pl ,right?
Yes, at this point there are a few TAB, whitespace and some 80 character
columns warning. Things are clean other than these nits, and a few
false positives on some of the target_core_configfs.c CPP macros.
>
> > + struct se_subsystem_dev *t10_sub_dev;
>
> How about 'sub_dev' instead?
> > + struct config_group t10_wwn_group;
>
> Take out the 't10_wwn'
> > + struct list_head t10_vpd_list;
> And make this 'list' by itself.
>
> > +} ____cacheline_aligned;
>
>
> > +
> > +typedef enum {
> > + SPC_PASSTHROUGH,
> > + SPC2_RESERVATIONS,
> > + SPC3_PERSISTENT_RESERVATIONS
> > +} t10_reservations_index_t;
>
> Are those official values ? Spec number, page?
These are also internal emulation states for PR emulation. I will move
these to target_core_pr.h..
> > +
> > +struct t10_pr_registration {
> > + /* Used for fabrics that contain WWN+ISID */
> > + char pr_reg_isid[PR_REG_ISID_LEN];
> > + /* Used during APTPL metadata reading */
> > + unsigned char pr_iport[PR_APTPL_MAX_IPORT_LEN];
> > + /* Used during APTPL metadata reading */
> > + unsigned char pr_tport[PR_APTPL_MAX_TPORT_LEN];
> > + /* For writing out live meta data */
> > + unsigned char *pr_aptpl_buf;
> > + u16 pr_aptpl_rpti;
> > + u16 pr_reg_tpgt;
> > + /* Reservation effects all target ports */
> > + int pr_reg_all_tg_pt;
> > + /* Activate Persistence across Target Power Loss */
> > + int pr_reg_aptpl;
> > + int pr_res_holder;
> > + int pr_res_type;
> > + int pr_res_scope;
> > + u32 pr_reg_flags;
> > + u32 pr_res_mapped_lun;
> > + u32 pr_aptpl_target_lun;
> > + u32 pr_res_generation;
> > + u64 pr_reg_bin_isid;
> > + u64 pr_res_key;
> > + atomic_t pr_res_holders;
> > + struct se_node_acl *pr_reg_nacl;
> > + struct se_dev_entry *pr_reg_deve;
> > + struct se_lun *pr_reg_tg_pt_lun;
> > + struct list_head pr_reg_list;
> > + struct list_head pr_reg_abort_list;
> > + struct list_head pr_reg_aptpl_list;
> > + struct list_head pr_reg_atp_list;
> > + struct list_head pr_reg_atp_mem_list;
> > +} ____cacheline_aligned;
> > +
> What is 'pr' ? Pringles? You could call that structure 'persist_reg' and
> remove all of those 'pr_reg_' prefixes.
This is intended to be shorthand for 'Persistent Reservations
Registration'..
>
> > +struct t10_reservation_template {
> > + /* Reservation effects all target ports */
> > + int pr_all_tg_pt;
>
> Um, not sure what the comment has to do with 'pr_all_tg_pt' ? What
> does 'pr_all_tg_tg' do?
This has to do with PR logic effecting the other LUNs of the same device
server that the same Initiator Port is connecting to over the same or
different target port.
> > + /* Activate Persistence across Target Power Loss enabled
> > + * for SCSI device */
> > + int pr_aptpl_active;
>
> Perhaps 'is_active' ?
>
> > + u32 pr_aptpl_buf_len;
>
> Where is the buf?
This is used to allocate a buffer for each registration at struct
t10_pr_registration->pr_aptpl_buf in drivers/target/target_core_pr.c:
__core_scsi3_do_alloc_registration()
>
> > + u32 pr_generation;
> > + t10_reservations_index_t res_type;
> > + spinlock_t registration_lock;
> > + spinlock_t aptpl_reg_lock;
> > + /* Reservation holder when pr_all_tg_pt=1 */
>
> Ah, and when it is another value?
Actually, I think this is wrong.. Will fix this comment..
>
> > + struct se_node_acl *pr_res_holder;
> > + struct list_head registration_list;
> > + struct list_head aptpl_reg_list;
> > + int (*t10_reservation_check)(struct se_cmd *, u32 *);
> > + int (*t10_seq_non_holder)(struct se_cmd *, unsigned char *, u32);
> > + int (*t10_pr_register)(struct se_cmd *);
> > + int (*t10_pr_clear)(struct se_cmd *);
>
> You can get rid of the t10_ prefix.
> > +} ____cacheline_aligned;
> > +
>
> This is the second structure I see where you mix data with functions.
>
> I don't know what the actual StyleGuide is for this, but would it be prudent
> to split the data (spinlocks, list, values) from the behavior (functions?)
> This could be done by embedding another struct - which only has function
> pointers?
Sure, I don't have a problem with doing that here..
>
> > +struct se_queue_req {
> > + int state;
> > + void *cmd;
> > + struct list_head qr_list;
> > +} ____cacheline_aligned;
> > +
> > +struct se_queue_obj {
> > + atomic_t queue_cnt;
> > + spinlock_t cmd_queue_lock;
> > + struct list_head qobj_list;
> > + wait_queue_head_t thread_wq;
> > + struct completion thread_create_comp;
> > + struct completion thread_done_comp;
> > +} ____cacheline_aligned;
> > +
> > +struct se_transport_task {
> > + unsigned char t_task_cdb[SCSI_CDB_SIZE];
> > + unsigned long long t_task_lba;
> > + int t_tasks_failed;
> > + int t_task_fua;
> > + u32 t_task_cdbs;
> > + u32 t_task_check;
> > + u32 t_task_no;
> > + u32 t_task_sectors;
> > + u32 t_task_se_num;
> > + atomic_t t_fe_count;
> > + atomic_t t_se_count;
> > + atomic_t t_task_cdbs_left;
> > + atomic_t t_task_cdbs_ex_left;
> > + atomic_t t_task_cdbs_timeout_left;
> > + atomic_t t_task_cdbs_sent;
> > + atomic_t t_transport_aborted;
> > + atomic_t t_transport_active;
> > + atomic_t t_transport_complete;
> > + atomic_t t_transport_queue_active;
> > + atomic_t t_transport_sent;
> > + atomic_t t_transport_stop;
> > + atomic_t t_transport_timeout;
> > + atomic_t transport_dev_active;
> > + atomic_t transport_lun_active;
> > + atomic_t transport_lun_fe_stop;
> > + atomic_t transport_lun_stop;
> > + spinlock_t t_state_lock;
> > + struct completion t_transport_stop_comp;
> > + struct completion t_transport_passthrough_comp;
> > + struct completion t_transport_passthrough_wcomp;
> > + struct completion transport_lun_fe_stop_comp;
> > + struct completion transport_lun_stop_comp;
> > + void *t_task_buf;
> > + void *t_task_pt_buf;
> > + struct list_head t_task_list;
> > + struct list_head *t_mem_list;
> > +} ____cacheline_aligned;
>
> I think you can remove the "t_" prefix. I am though a bit confused, the
> struct is called 'transport_task' and there are values that are
> transport_task related, but then there are also some that are clearly related
> to a task:t_task_no, t_task_check? Should they just be plural?
>
Indeed, I think that make t_task into t_tasks makes sense above..
> > +
> > +struct se_task {
> > + unsigned char task_sense;
> > + struct scatterlist *task_sg;
> > + void *transport_req;
> > + u8 task_scsi_status;
> > + u8 task_flags;
> > + int task_error_status;
> > + int task_state_flags;
> > + unsigned long long task_lba;
> > + u32 task_no;
> > + u32 task_sectors;
> > + u32 task_size;
> > + u32 task_sg_num;
> > + u32 task_sg_offset;
> > + struct se_cmd *task_se_cmd;
> > + struct se_device *se_dev;
> > + struct completion task_stop_comp;
> > + atomic_t task_active;
> > + atomic_t task_execute_queue;
> > + atomic_t task_timeout;
> > + atomic_t task_sent;
> > + atomic_t task_stop;
> > + atomic_t task_state_active;
> > + struct timer_list task_timer;
> > + int (*transport_map_task)(struct se_task *, u32);
> > + struct se_device *se_obj_ptr;
> > + struct list_head t_list;
> > + struct list_head t_execute_list;
> > + struct list_head t_state_list;
> > +} ____cacheline_aligned;
> > +
> > +#define TASK_CMD(task) ((struct se_cmd *)task->task_se_cmd)
> > +#define TASK_DEV(task) ((struct se_device *)task->se_dev)
> > +
>
> I stopped the review here. I think that using the ideas of how ehci.h mixes
> the struct variables with #defines will make this much easier to read?
Ok, I will go ahead and make the changes per the above comments and will
drop you a line when the push is ready. I do agree that mixing the
struct variables with #defines in target_core_base.h does make things a
bit more readable. Let me consider this a bit.
Any comments from the kernel folks on this particular item..?
Thanks for your comments Konrad!
Best,
--nab
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops
2010-09-02 19:19 ` Nicholas A. Bellinger
@ 2010-09-04 23:53 ` Nicholas A. Bellinger
2010-09-05 0:35 ` ldd scull_load question runcoderen
2010-09-07 20:56 ` [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops Konrad Rzeszutek Wilk
0 siblings, 2 replies; 11+ messages in thread
From: Nicholas A. Bellinger @ 2010-09-04 23:53 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-scsi, linux-kernel, FUJITA Tomonori, Mike Christie,
Christoph Hellwig, Hannes Reinecke, James Bottomley, Jens Axboe,
Boaz Harrosh
On Thu, 2010-09-02 at 12:19 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2010-09-02 at 01:56 -0400, Konrad Rzeszutek Wilk wrote:
> > On Monday 30 August 2010 05:20:40 Nicholas A. Bellinger wrote:
> > > From: Nicholas Bellinger <nab@linux-iscsi.org>
> >
> > Hey Nicholas,
> >
> > I did a cursory review .
> >
<BIG SNIP>
>
> Ok, I will go ahead and make the changes per the above comments and will
> drop you a line when the push is ready. I do agree that mixing the
> struct variables with #defines in target_core_base.h does make things a
> bit more readable. Let me consider this a bit.
>
> Any comments from the kernel folks on this particular item..?
>
> Thanks for your comments Konrad!
>
Hi Konard,
I just pushed most of your recommend cleanups for target_core_base.h to
lio-core-2.6.git/lio-4.0. The cleanup series has been posted on
lio-devel list to prevent extra noise on linux-scsi, et al, but for
reference the shortlog and diffstat are attached below.. I did not dive
into intermixed struct + #defines, but I am still pretty indifferent on
this particular item.
Please have a look and feel free to make additional comments.
Thanks again!
--nab
Nicholas Bellinger (13):
tcm: Remove legacy struct se_lun->lun_type usage
tcm: Remove legacy DEV_STATUS_THR_* defines
tcm: Convert TCM base defines to enums, round 1
tcm: Convert NAF_DYNAMIC_NODE_ACL to struct
se_node_acl->dynamic_node_acl bitfield
tcm: Convert MAP_SG_KMAP to struct se_map_sg->sg_kmap_active bitfield
tcm: Convert TCM base defines to enums, round 2:
tcm: Convert DEF_PR_REGISTERED to struct
se_dev_entry->def_pr_registered bitfield
tcm: Convert PRF_ISID_PRESENT_AT_REG to struct
t10_pr_registration->isid_present_at_reg bitfield
tcm: Convert struct se_cmd->scsi_sense_reason enum and add TCM_*
prefix
tcm: Add comments for t10_alua_index_t, t10_task_attr_index_t and
t10_reservations_index_t
tcm/pr: Add proper comment for struct
t10_reservation_template->pr_res_holder
tcm/alua: Move (*t10_*)() callers into struct t10_reservation_ops
tcm: Cleanups for struct se_transport_task usage
drivers/target/lio-target/iscsi_target.c | 12 +-
drivers/target/lio-target/iscsi_target_erl0.c | 4 +-
drivers/target/lio-target/iscsi_target_mib.c | 6 +-
drivers/target/lio-target/iscsi_target_util.c | 6 +-
drivers/target/target_core_device.c | 17 +-
drivers/target/target_core_fabric_lib.c | 5 +-
drivers/target/target_core_file.c | 2 +-
drivers/target/target_core_iblock.c | 2 +-
drivers/target/target_core_mib.c | 6 +-
drivers/target/target_core_pr.c | 46 ++--
drivers/target/target_core_rd.c | 6 +-
drivers/target/target_core_tpg.c | 38 +--
drivers/target/target_core_transport.c | 161 ++++++------
drivers/target/tcm_fc/tfc_cmd.c | 4 +-
drivers/target/tcm_fc/tfc_io.c | 4 +-
drivers/target/tcm_loop/tcm_loop_fabric_scsi.c | 4 +-
include/target/target_core_base.h | 314 ++++++++++++++----------
include/target/target_core_tpg.h | 4 +-
18 files changed, 331 insertions(+), 310 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* ldd scull_load question
2010-09-04 23:53 ` Nicholas A. Bellinger
@ 2010-09-05 0:35 ` runcoderen
2010-09-05 0:43 ` Randy Dunlap
2010-09-05 3:21 ` Adam Jiang
2010-09-07 20:56 ` [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops Konrad Rzeszutek Wilk
1 sibling, 2 replies; 11+ messages in thread
From: runcoderen @ 2010-09-05 0:35 UTC (permalink / raw)
To: linux-kernel
hi all:
I am reading ldd(3rd).
when I read the scull_load part.
I can't understand some special sentences in the scripts as follows:
/sbin/insmod ./$module.ko $* || exit 1
why we add $* and || exit 1 after /sbin/insmod ./$module.ko?
can anyone help me to understand?
runcoderen
--
/****************************************
http://runcoderen.wordpress.com
****************************************/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ldd scull_load question
2010-09-05 0:35 ` ldd scull_load question runcoderen
@ 2010-09-05 0:43 ` Randy Dunlap
2010-09-05 1:01 ` runcoderen
2010-09-05 3:21 ` Adam Jiang
1 sibling, 1 reply; 11+ messages in thread
From: Randy Dunlap @ 2010-09-05 0:43 UTC (permalink / raw)
To: runcoderen; +Cc: linux-kernel
On Sun, 05 Sep 2010 08:35:14 +0800 runcoderen wrote:
> hi all:
> I am reading ldd(3rd).
> when I read the scull_load part.
> I can't understand some special sentences in the scripts as follows:
>
> /sbin/insmod ./$module.ko $* || exit 1
>
> why we add $* and || exit 1 after /sbin/insmod ./$module.ko?
>
> can anyone help me to understand?
"$*" passes all parameters of the script to the module as module parameters.
"insmod module.ko || exit 1"
means "if insmod is successful (no error), then the command is finished,
otherwise exit with an error code of 1."
Does that help?
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ldd scull_load question
2010-09-05 0:43 ` Randy Dunlap
@ 2010-09-05 1:01 ` runcoderen
2010-09-05 21:20 ` Randy Dunlap
0 siblings, 1 reply; 11+ messages in thread
From: runcoderen @ 2010-09-05 1:01 UTC (permalink / raw)
To: Randy Dunlap; +Cc: linux-kernel
yes, you do!
there is another question confused me.
I have seem the command in the scull.c:
int scull_major = SCULL_MAJOR;and so on.
module_param(scull_major, int, S_IRUGO); and so on.
if we don't do "./scull_load scull_major" command.
we just use "./scull_load"
it will make scull_major = SCULL_MAJOR default?
在 2010-09-04六的 17:43 -0700，Randy Dunlap写道：
> On Sun, 05 Sep 2010 08:35:14 +0800 runcoderen wrote:
>
> > hi all:
> > I am reading ldd(3rd).
> > when I read the scull_load part.
> > I can't understand some special sentences in the scripts as follows:
> >
> > /sbin/insmod ./$module.ko $* || exit 1
> >
> > why we add $* and || exit 1 after /sbin/insmod ./$module.ko?
> >
> > can anyone help me to understand?
>
> "$*" passes all parameters of the script to the module as module parameters.
>
> "insmod module.ko || exit 1"
>
> means "if insmod is successful (no error), then the command is finished,
> otherwise exit with an error code of 1."
>
> Does that help?
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
--
/****************************************
http://runcoderen.wordpress.com
****************************************/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ldd scull_load question
2010-09-05 0:35 ` ldd scull_load question runcoderen
2010-09-05 0:43 ` Randy Dunlap
@ 2010-09-05 3:21 ` Adam Jiang
1 sibling, 0 replies; 11+ messages in thread
From: Adam Jiang @ 2010-09-05 3:21 UTC (permalink / raw)
To: runcoderen; +Cc: linux-kernel
On Sun, Sep 05, 2010 at 08:35:14AM +0800, runcoderen wrote:
> hi all:
> I am reading ldd(3rd).
> when I read the scull_load part.
> I can't understand some special sentences in the scripts as follows:
>
> /sbin/insmod ./$module.ko $* || exit 1
>
> why we add $* and || exit 1 after /sbin/insmod ./$module.ko?
> can anyone help me to understand?
Hey,
The '$*' means bash variables. I believe it is a question not related to
Linux kernel. See
http://tldp.org/LDP/abs/html/othertypesv.html
/Adam
> runcoderen
>
> --
>
> /****************************************
> http://runcoderen.wordpress.com
> ****************************************/
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ldd scull_load question
2010-09-05 1:01 ` runcoderen
@ 2010-09-05 21:20 ` Randy Dunlap
0 siblings, 0 replies; 11+ messages in thread
From: Randy Dunlap @ 2010-09-05 21:20 UTC (permalink / raw)
To: runcoderen; +Cc: linux-kernel
On Sun, 05 Sep 2010 09:01:55 +0800 runcoderen wrote:
> yes, you do!
>
> there is another question confused me.
> I have seem the command in the scull.c:
> int scull_major = SCULL_MAJOR;and so on.
>
> module_param(scull_major, int, S_IRUGO); and so on.
>
> if we don't do "./scull_load scull_major" command.
>
> we just use "./scull_load"
>
> it will make scull_major = SCULL_MAJOR default?
Yes. scull_major is initialized to SCULL_MAJOR every time
that the module is loaded.
The module_param() allows someone to change the value of
scull_major.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops
2010-09-04 23:53 ` Nicholas A. Bellinger
2010-09-05 0:35 ` ldd scull_load question runcoderen
@ 2010-09-07 20:56 ` Konrad Rzeszutek Wilk
2010-09-08 1:44 ` Nicholas A. Bellinger
1 sibling, 1 reply; 11+ messages in thread
From: Konrad Rzeszutek Wilk @ 2010-09-07 20:56 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: linux-scsi, linux-kernel, FUJITA Tomonori, Mike Christie,
Christoph Hellwig, Hannes Reinecke, James Bottomley, Jens Axboe,
Boaz Harrosh
> Hi Konard,
>
> I just pushed most of your recommend cleanups for target_core_base.h to
> lio-core-2.6.git/lio-4.0. The cleanup series has been posted on
> lio-devel list to prevent extra noise on linux-scsi, et al, but for
> reference the shortlog and diffstat are attached below.. I did not dive
> into intermixed struct + #defines, but I am still pretty indifferent on
> this particular item.
>
So the reasons why I thought it would be a good idea to combine the #defines
and the values and also dropping the prefix of structs for all of the 'flag'
members is that:
1) once you have them intermixed, searching for a particular flag either using
cscope/ctags becomes easier. You have the definition of the 'flag' and the
different values it can have in one localized spot. It also removes your
worry about the 'grep' - you usually search for the values that a specific
flag has and if you use that tool you can find it right there along with the
structure it was defined in.
2). From a perspective of a vendor maintainer who has to find a fix, getting
familiarized with the code within a short window time frame and having the
acceptable states and its definitions in one place (and even pointers to the
specs if there are some) makes it sooo much easier to grok the code.
Having even nice ascii art of how/what works together is even better.
Obviously thought there are tools that can do this for you. But when the bug
hits I somehow never managed to have those tools ready :-(
>
> Please have a look and feel free to make additional comments.
Sure.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops
2010-09-07 20:56 ` [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops Konrad Rzeszutek Wilk
@ 2010-09-08 1:44 ` Nicholas A. Bellinger
0 siblings, 0 replies; 11+ messages in thread
From: Nicholas A. Bellinger @ 2010-09-08 1:44 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-scsi, linux-kernel, FUJITA Tomonori, Mike Christie,
Christoph Hellwig, Hannes Reinecke, James Bottomley, Jens Axboe,
Boaz Harrosh
On Tue, 2010-09-07 at 16:56 -0400, Konrad Rzeszutek Wilk wrote:
> > Hi Konard,
> >
> > I just pushed most of your recommend cleanups for target_core_base.h to
> > lio-core-2.6.git/lio-4.0. The cleanup series has been posted on
> > lio-devel list to prevent extra noise on linux-scsi, et al, but for
> > reference the shortlog and diffstat are attached below.. I did not dive
> > into intermixed struct + #defines, but I am still pretty indifferent on
> > this particular item.
> >
>
> So the reasons why I thought it would be a good idea to combine the #defines
> and the values and also dropping the prefix of structs for all of the 'flag'
> members is that:
>
> 1) once you have them intermixed, searching for a particular flag either using
> cscope/ctags becomes easier. You have the definition of the 'flag' and the
> different values it can have in one localized spot. It also removes your
> worry about the 'grep' - you usually search for the values that a specific
> flag has and if you use that tool you can find it right there along with the
> structure it was defined in.
>
> 2). From a perspective of a vendor maintainer who has to find a fix, getting
> familiarized with the code within a short window time frame and having the
> acceptable states and its definitions in one place (and even pointers to the
> specs if there are some) makes it sooo much easier to grok the code.
> Having even nice ascii art of how/what works together is even better.
Ok, I think these are both reasonable points for combining the #defines
near the structure members where they are actually used. I will go
ahead and make this change, but I would still like to keep the exiting
prefixes for structure members for now.
Thanks for your comments Konrad!
--nab
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-09-08 1:48 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-30 9:20 [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops Nicholas A. Bellinger
2010-09-02 5:56 ` Konrad Rzeszutek Wilk
2010-09-02 19:19 ` Nicholas A. Bellinger
2010-09-04 23:53 ` Nicholas A. Bellinger
2010-09-05 0:35 ` ldd scull_load question runcoderen
2010-09-05 0:43 ` Randy Dunlap
2010-09-05 1:01 ` runcoderen
2010-09-05 21:20 ` Randy Dunlap
2010-09-05 3:21 ` Adam Jiang
2010-09-07 20:56 ` [RFC 04/22] tcm: Add v4 base data structures and struct target_core_fabric_ops Konrad Rzeszutek Wilk
2010-09-08 1:44 ` Nicholas A. Bellinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).