From 902ff8603ed756637a753c1b1ce12d4da9b3ed1e Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 14 Sep 2018 13:28:08 +0100 Subject: scsi: iscsi: target: fix spelling mistake "entires" -> "entries" Trivial fix to spelling mistake in function name and comment Signed-off-by: Colin Ian King Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 2 +- drivers/target/iscsi/iscsi_target_erl2.c | 2 +- drivers/target/iscsi/iscsi_target_erl2.h | 2 +- drivers/target/iscsi/iscsi_target_login.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 94bad43c41ff..07768bfed2ab 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -4356,7 +4356,7 @@ int iscsit_close_session(struct iscsi_session *sess) transport_deregister_session(sess->se_sess); if (sess->sess_ops->ErrorRecoveryLevel == 2) - iscsit_free_connection_recovery_entires(sess); + iscsit_free_connection_recovery_entries(sess); iscsit_free_all_ooo_cmdsns(sess); diff --git a/drivers/target/iscsi/iscsi_target_erl2.c b/drivers/target/iscsi/iscsi_target_erl2.c index 8df9c90f3db3..b08b620b1bf0 100644 --- a/drivers/target/iscsi/iscsi_target_erl2.c +++ b/drivers/target/iscsi/iscsi_target_erl2.c @@ -125,7 +125,7 @@ struct iscsi_conn_recovery *iscsit_get_inactive_connection_recovery_entry( return NULL; } -void iscsit_free_connection_recovery_entires(struct iscsi_session *sess) +void iscsit_free_connection_recovery_entries(struct iscsi_session *sess) { struct iscsi_cmd *cmd, *cmd_tmp; struct iscsi_conn_recovery *cr, *cr_tmp; diff --git a/drivers/target/iscsi/iscsi_target_erl2.h b/drivers/target/iscsi/iscsi_target_erl2.h index 93e180d68d07..a39b0caf2337 100644 --- a/drivers/target/iscsi/iscsi_target_erl2.h +++ b/drivers/target/iscsi/iscsi_target_erl2.h @@ -13,7 +13,7 @@ extern void iscsit_create_conn_recovery_datain_values(struct iscsi_cmd *, __be32 extern void iscsit_create_conn_recovery_dataout_values(struct iscsi_cmd *); extern struct iscsi_conn_recovery *iscsit_get_inactive_connection_recovery_entry( struct iscsi_session *, u16); -extern void iscsit_free_connection_recovery_entires(struct iscsi_session *); +extern void iscsit_free_connection_recovery_entries(struct iscsi_session *); extern int iscsit_remove_active_connection_recovery_entry( struct iscsi_conn_recovery *, struct iscsi_session *); extern int iscsit_remove_cmd_from_connection_recovery(struct iscsi_cmd *, diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 9e74f8bc2963..e3c76a5a665e 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -615,7 +615,7 @@ int iscsi_login_post_auth_non_zero_tsih( } /* - * Check for any connection recovery entires containing CID. + * Check for any connection recovery entries containing CID. * We use the original ExpStatSN sent in the first login request * to acknowledge commands for the failed connection. * -- cgit v1.2.3 From fed564f6503bdfd57f6dc32b4a1e94b90b05e803 Mon Sep 17 00:00:00 2001 From: Greg Edwards Date: Tue, 4 Sep 2018 11:19:10 -0600 Subject: scsi: target: iblock: split T10 PI SGL across command bios When T10 PI is enabled on a backing device for the iblock backstore, the PI SGL for the entire command is attached to the first bio only. This works fine if the command is covered by a single bio, but can result in ref tag errors in the client for the other bios in a multi-bio command, e.g. [ 47.631236] sda: ref tag error at location 2048 (rcvd 0) [ 47.637658] sda: ref tag error at location 4096 (rcvd 0) [ 47.644228] sda: ref tag error at location 6144 (rcvd 0) The command will be split into multiple bios if the number of data SG elements exceeds BIO_MAX_PAGES (see iblock_get_bio()). The bios may later be split again in the block layer on the host after iblock_submit_bios(), depending on the queue limits of the backing device. The block and SCSI layers will pass through the whole PI SGL down to the LLDD however that first bio is split up, but the LLDD may only use the portion that corresponds to the data length (depends on the LLDD, tested with scsi_debug). Split the PI SGL across the bios in the command, so each bio's bio_integrity_payload contains the protection information for the data in the bio. Use an sg_mapping_iter to keep track of where we are in PI SGL, so we know where to start with the next bio. Signed-off-by: Greg Edwards Reviewed-by: Mike Christie Signed-off-by: Martin K. Petersen --- drivers/target/target_core_iblock.c | 54 +++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 17 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index ce1321a5cb7b..d97624620aab 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -635,14 +635,15 @@ static ssize_t iblock_show_configfs_dev_params(struct se_device *dev, char *b) } static int -iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio) +iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio, + struct sg_mapping_iter *miter) { struct se_device *dev = cmd->se_dev; struct blk_integrity *bi; struct bio_integrity_payload *bip; struct iblock_dev *ib_dev = IBLOCK_DEV(dev); - struct scatterlist *sg; - int i, rc; + int rc; + size_t resid, len; bi = bdev_get_integrity(ib_dev->ibd_bd); if (!bi) { @@ -650,31 +651,39 @@ iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio) return -ENODEV; } - bip = bio_integrity_alloc(bio, GFP_NOIO, cmd->t_prot_nents); + bip = bio_integrity_alloc(bio, GFP_NOIO, + min_t(unsigned int, cmd->t_prot_nents, BIO_MAX_PAGES)); if (IS_ERR(bip)) { pr_err("Unable to allocate bio_integrity_payload\n"); return PTR_ERR(bip); } - bip->bip_iter.bi_size = (cmd->data_length / dev->dev_attrib.block_size) * - dev->prot_length; - bip->bip_iter.bi_sector = bio->bi_iter.bi_sector; + bip->bip_iter.bi_size = bio_integrity_bytes(bi, bio_sectors(bio)); + bip_set_seed(bip, bio->bi_iter.bi_sector); pr_debug("IBLOCK BIP Size: %u Sector: %llu\n", bip->bip_iter.bi_size, (unsigned long long)bip->bip_iter.bi_sector); - for_each_sg(cmd->t_prot_sg, sg, cmd->t_prot_nents, i) { + resid = bip->bip_iter.bi_size; + while (resid > 0 && sg_miter_next(miter)) { - rc = bio_integrity_add_page(bio, sg_page(sg), sg->length, - sg->offset); - if (rc != sg->length) { + len = min_t(size_t, miter->length, resid); + rc = bio_integrity_add_page(bio, miter->page, len, + offset_in_page(miter->addr)); + if (rc != len) { pr_err("bio_integrity_add_page() failed; %d\n", rc); + sg_miter_stop(miter); return -ENOMEM; } - pr_debug("Added bio integrity page: %p length: %d offset; %d\n", - sg_page(sg), sg->length, sg->offset); + pr_debug("Added bio integrity page: %p length: %zu offset: %lu\n", + miter->page, len, offset_in_page(miter->addr)); + + resid -= len; + if (len < miter->length) + miter->consumed -= miter->length - len; } + sg_miter_stop(miter); return 0; } @@ -686,12 +695,13 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, struct se_device *dev = cmd->se_dev; sector_t block_lba = target_to_linux_sector(dev, cmd->t_task_lba); struct iblock_req *ibr; - struct bio *bio, *bio_start; + struct bio *bio; struct bio_list list; struct scatterlist *sg; u32 sg_num = sgl_nents; unsigned bio_cnt; - int i, op, op_flags = 0; + int i, rc, op, op_flags = 0; + struct sg_mapping_iter prot_miter; if (data_direction == DMA_TO_DEVICE) { struct iblock_dev *ib_dev = IBLOCK_DEV(dev); @@ -726,13 +736,17 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, if (!bio) goto fail_free_ibr; - bio_start = bio; bio_list_init(&list); bio_list_add(&list, bio); refcount_set(&ibr->pending, 2); bio_cnt = 1; + if (cmd->prot_type && dev->dev_attrib.pi_prot_type) + sg_miter_start(&prot_miter, cmd->t_prot_sg, cmd->t_prot_nents, + op == REQ_OP_READ ? SG_MITER_FROM_SG : + SG_MITER_TO_SG); + for_each_sg(sgl, sg, sgl_nents, i) { /* * XXX: if the length the device accepts is shorter than the @@ -741,6 +755,12 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, */ while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset) != sg->length) { + if (cmd->prot_type && dev->dev_attrib.pi_prot_type) { + rc = iblock_alloc_bip(cmd, bio, &prot_miter); + if (rc) + goto fail_put_bios; + } + if (bio_cnt >= IBLOCK_MAX_BIO_PER_TASK) { iblock_submit_bios(&list); bio_cnt = 0; @@ -762,7 +782,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, } if (cmd->prot_type && dev->dev_attrib.pi_prot_type) { - int rc = iblock_alloc_bip(cmd, bio_start); + rc = iblock_alloc_bip(cmd, bio, &prot_miter); if (rc) goto fail_put_bios; } -- cgit v1.2.3 From 801df68d617e3cb831f531c99fa6003620e6b343 Mon Sep 17 00:00:00 2001 From: Varun Prakash Date: Wed, 19 Sep 2018 19:44:57 +0530 Subject: scsi: target: iscsi: cxgbit: fix csk leak csk leak can happen if a new TCP connection gets established after cxgbit_accept_np() returns, to fix this leak free remaining csk in cxgbit_free_np(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/cxgbit/cxgbit_cm.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/cxgbit/cxgbit_cm.c b/drivers/target/iscsi/cxgbit/cxgbit_cm.c index 8de16016b6de..71888b979ab5 100644 --- a/drivers/target/iscsi/cxgbit/cxgbit_cm.c +++ b/drivers/target/iscsi/cxgbit/cxgbit_cm.c @@ -598,9 +598,12 @@ out: mutex_unlock(&cdev_list_lock); } +static void __cxgbit_free_conn(struct cxgbit_sock *csk); + void cxgbit_free_np(struct iscsi_np *np) { struct cxgbit_np *cnp = np->np_context; + struct cxgbit_sock *csk, *tmp; cnp->com.state = CSK_STATE_DEAD; if (cnp->com.cdev) @@ -608,6 +611,13 @@ void cxgbit_free_np(struct iscsi_np *np) else cxgbit_free_all_np(cnp); + spin_lock_bh(&cnp->np_accept_lock); + list_for_each_entry_safe(csk, tmp, &cnp->np_accept_list, accept_node) { + list_del_init(&csk->accept_node); + __cxgbit_free_conn(csk); + } + spin_unlock_bh(&cnp->np_accept_lock); + np->np_context = NULL; cxgbit_put_cnp(cnp); } @@ -705,9 +715,9 @@ void cxgbit_abort_conn(struct cxgbit_sock *csk) csk->tid, 600, __func__); } -void cxgbit_free_conn(struct iscsi_conn *conn) +static void __cxgbit_free_conn(struct cxgbit_sock *csk) { - struct cxgbit_sock *csk = conn->context; + struct iscsi_conn *conn = csk->conn; bool release = false; pr_debug("%s: state %d\n", @@ -716,7 +726,7 @@ void cxgbit_free_conn(struct iscsi_conn *conn) spin_lock_bh(&csk->lock); switch (csk->com.state) { case CSK_STATE_ESTABLISHED: - if (conn->conn_state == TARG_CONN_STATE_IN_LOGOUT) { + if (conn && (conn->conn_state == TARG_CONN_STATE_IN_LOGOUT)) { csk->com.state = CSK_STATE_CLOSING; cxgbit_send_halfclose(csk); } else { @@ -741,6 +751,11 @@ void cxgbit_free_conn(struct iscsi_conn *conn) cxgbit_put_csk(csk); } +void cxgbit_free_conn(struct iscsi_conn *conn) +{ + __cxgbit_free_conn(conn->context); +} + static void cxgbit_set_emss(struct cxgbit_sock *csk, u16 opt) { csk->emss = csk->com.cdev->lldi.mtus[TCPOPT_MSS_G(opt)] - @@ -803,6 +818,7 @@ void _cxgbit_free_csk(struct kref *kref) spin_unlock_bh(&cdev->cskq.lock); cxgbit_free_skb(csk); + cxgbit_put_cnp(csk->cnp); cxgbit_put_cdev(cdev); kfree(csk); @@ -1351,6 +1367,7 @@ cxgbit_pass_accept_req(struct cxgbit_device *cdev, struct sk_buff *skb) goto rel_skb; } + cxgbit_get_cnp(cnp); cxgbit_get_cdev(cdev); spin_lock(&cdev->cskq.lock); -- cgit v1.2.3 From 38fe73cc2c96fbc9942b07220f2a4f1bab37392d Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Wed, 10 Oct 2018 03:23:10 +0000 Subject: scsi: target: Fix target_wait_for_sess_cmds breakage with active signals With the addition of commit 00d909a10710 ("scsi: target: Make the session shutdown code also wait for commands that are being aborted") in v4.19-rc, it incorrectly assumes no signals will be pending for task_struct executing the normal session shutdown and I/O quiesce code-path. For example, iscsi-target and iser-target issue SIGINT to all kthreads as part of session shutdown. This has been the behaviour since day one. As-is when signals are pending with se_cmds active in se_sess->sess_cmd_list, wait_event_interruptible_lock_irq_timeout() returns a negative number and immediately kills the machine because of the do while (ret <= 0) loop that was added in commit 00d909a107 to spin while backend I/O is taking any amount of extended time (say 30 seconds) to complete. Here's what it looks like in action with debug plus delayed backend I/O completion: [ 4951.909951] se_sess: 000000003e7e08fa before target_wait_for_sess_cmds [ 4951.914600] target_wait_for_sess_cmds: signal_pending: 1 [ 4951.918015] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 0 [ 4951.921639] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 1 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 2 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 3 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 4 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 5 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 6 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 7 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 8 [ 4951.921944] wait_event_interruptible_lock_irq_timeout ret: -512 signal_pending: 1 loop count: 9 ... followed by the usual RCU CPU stalls and deadlock. There was never a case pre commit 00d909a107 where wait_for_complete(&se_cmd->cmd_wait_comp) was able to be interrupted, so to address this for v4.19+ moving forward go ahead and use wait_event_lock_irq_timeout() instead so new code works with all fabric drivers. Also for commit 00d909a107, fix a minor regression in target_release_cmd_kref() to only wake_up the new se_sess->cmd_list_wq only when shutdown has actually been triggered via se_sess->sess_tearing_down. Fixes: 00d909a10710 ("scsi: target: Make the session shutdown code also wait for commands that are being aborted") Cc: # v4.19+ Cc: Bart Van Assche Cc: Mike Christie Cc: Hannes Reinecke Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Bryant G. Ly Tested-by: Nicholas Bellinger Signed-off-by: Nicholas Bellinger Reviewed-by: Bryant G. Ly Signed-off-by: Martin K. Petersen --- drivers/target/target_core_transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 86c0156e6c88..fc3093d21b96 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2754,7 +2754,7 @@ static void target_release_cmd_kref(struct kref *kref) if (se_sess) { spin_lock_irqsave(&se_sess->sess_cmd_lock, flags); list_del_init(&se_cmd->se_cmd_list); - if (list_empty(&se_sess->sess_cmd_list)) + if (se_sess->sess_tearing_down && list_empty(&se_sess->sess_cmd_list)) wake_up(&se_sess->cmd_list_wq); spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); } @@ -2907,7 +2907,7 @@ void target_wait_for_sess_cmds(struct se_session *se_sess) spin_lock_irq(&se_sess->sess_cmd_lock); do { - ret = wait_event_interruptible_lock_irq_timeout( + ret = wait_event_lock_irq_timeout( se_sess->cmd_list_wq, list_empty(&se_sess->sess_cmd_list), se_sess->sess_cmd_lock, 180 * HZ); -- cgit v1.2.3 From df711553f4440e16af5b731ed41841dee1e2abb4 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 12 Oct 2018 12:01:16 +0200 Subject: scsi: target: use ISCSI_IQN_LEN in iscsi_target_stat Move the ISCSI_IQN_LEN definition up, so that it can be used in more places instead of a hardcoded value. Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_stat.c | 4 ++-- include/target/iscsi/iscsi_target_core.h | 6 +++--- include/target/iscsi/iscsi_target_stat.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_stat.c b/drivers/target/iscsi/iscsi_target_stat.c index df0a39811dc2..bb98882bdaa7 100644 --- a/drivers/target/iscsi/iscsi_target_stat.c +++ b/drivers/target/iscsi/iscsi_target_stat.c @@ -328,10 +328,10 @@ static ssize_t iscsi_stat_tgt_attr_fail_intr_name_show(struct config_item *item, { struct iscsi_tiqn *tiqn = iscsi_tgt_attr_tiqn(item); struct iscsi_login_stats *lstat = &tiqn->login_stats; - unsigned char buf[224]; + unsigned char buf[ISCSI_IQN_LEN]; spin_lock(&lstat->lock); - snprintf(buf, 224, "%s", lstat->last_intr_fail_name[0] ? + snprintf(buf, ISCSI_IQN_LEN, "%s", lstat->last_intr_fail_name[0] ? lstat->last_intr_fail_name : NONE); spin_unlock(&lstat->lock); diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h index f2e6abea8490..24c398f4a68f 100644 --- a/include/target/iscsi/iscsi_target_core.h +++ b/include/target/iscsi/iscsi_target_core.h @@ -25,6 +25,7 @@ struct sock; #define ISCSIT_TCP_BACKLOG 256 #define ISCSI_RX_THREAD_NAME "iscsi_trx" #define ISCSI_TX_THREAD_NAME "iscsi_ttx" +#define ISCSI_IQN_LEN 224 /* struct iscsi_node_attrib sanity values */ #define NA_DATAOUT_TIMEOUT 3 @@ -270,9 +271,9 @@ struct iscsi_conn_ops { }; struct iscsi_sess_ops { - char InitiatorName[224]; + char InitiatorName[ISCSI_IQN_LEN]; char InitiatorAlias[256]; - char TargetName[224]; + char TargetName[ISCSI_IQN_LEN]; char TargetAlias[256]; char TargetAddress[256]; u16 TargetPortalGroupTag; /* [0..65535] */ @@ -855,7 +856,6 @@ struct iscsi_wwn_stat_grps { }; struct iscsi_tiqn { -#define ISCSI_IQN_LEN 224 unsigned char tiqn[ISCSI_IQN_LEN]; enum tiqn_state_table tiqn_state; int tiqn_access_count; diff --git a/include/target/iscsi/iscsi_target_stat.h b/include/target/iscsi/iscsi_target_stat.h index 4d75a2c426ca..ff6a47209313 100644 --- a/include/target/iscsi/iscsi_target_stat.h +++ b/include/target/iscsi/iscsi_target_stat.h @@ -33,7 +33,7 @@ struct iscsi_sess_err_stats { u32 cxn_timeout_errors; u32 pdu_format_errors; u32 last_sess_failure_type; - char last_sess_fail_rem_name[224]; + char last_sess_fail_rem_name[ISCSI_IQN_LEN]; } ____cacheline_aligned; /* iSCSI login failure types (sub oids) */ @@ -56,7 +56,7 @@ struct iscsi_login_stats { u32 last_fail_type; int last_intr_fail_ip_family; struct sockaddr_storage last_intr_fail_sockaddr; - char last_intr_fail_name[224]; + char last_intr_fail_name[ISCSI_IQN_LEN]; } ____cacheline_aligned; /* iSCSI logout stats */ -- cgit v1.2.3 From d9a771fd42712cd530544674b04bc3e42cc7927a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 12 Oct 2018 12:01:17 +0200 Subject: scsi: target: log Data-Out timeouts as errors Data-Out timeouts resulting in connection outages should be logged as errors. Include the I_T Nexus in the message to aid path identification. Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_erl1.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c index 5efa42b939a1..7f3a1a06696e 100644 --- a/drivers/target/iscsi/iscsi_target_erl1.c +++ b/drivers/target/iscsi/iscsi_target_erl1.c @@ -1169,15 +1169,21 @@ void iscsit_handle_dataout_timeout(struct timer_list *t) na = iscsit_tpg_get_node_attrib(sess); if (!sess->sess_ops->ErrorRecoveryLevel) { - pr_debug("Unable to recover from DataOut timeout while" - " in ERL=0.\n"); + pr_err("Unable to recover from DataOut timeout while" + " in ERL=0, closing iSCSI connection for I_T Nexus" + " %s,i,0x%6phN,%s,t,0x%02x\n", + sess->sess_ops->InitiatorName, sess->isid, + sess->tpg->tpg_tiqn->tiqn, (u32)sess->tpg->tpgt); goto failure; } if (++cmd->dataout_timeout_retries == na->dataout_timeout_retries) { - pr_debug("Command ITT: 0x%08x exceeded max retries" - " for DataOUT timeout %u, closing iSCSI connection.\n", - cmd->init_task_tag, na->dataout_timeout_retries); + pr_err("Command ITT: 0x%08x exceeded max retries" + " for DataOUT timeout %u, closing iSCSI connection for" + " I_T Nexus %s,i,0x%6phN,%s,t,0x%02x\n", + cmd->init_task_tag, na->dataout_timeout_retries, + sess->sess_ops->InitiatorName, sess->isid, + sess->tpg->tpg_tiqn->tiqn, (u32)sess->tpg->tpgt); goto failure; } -- cgit v1.2.3 From c62ae3005b3551078a8cf959de8018a2852708bd Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 12 Oct 2018 12:01:18 +0200 Subject: scsi: target: log NOP ping timeouts as errors Events resulting in connection outages like this should be logged as errors. Include the I_T Nexus in the message to aid path identification. Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_util.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 49be1e41290c..931c51f56435 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -915,6 +915,7 @@ static int iscsit_add_nopin(struct iscsi_conn *conn, int want_response) void iscsit_handle_nopin_response_timeout(struct timer_list *t) { struct iscsi_conn *conn = from_timer(conn, t, nopin_response_timer); + struct iscsi_session *sess = conn->sess; iscsit_inc_conn_usage_count(conn); @@ -925,9 +926,10 @@ void iscsit_handle_nopin_response_timeout(struct timer_list *t) return; } - pr_debug("Did not receive response to NOPIN on CID: %hu on" - " SID: %u, failing connection.\n", conn->cid, - conn->sess->sid); + pr_err("Did not receive response to NOPIN on CID: %hu, failing" + " connection for I_T Nexus %s,i,0x%6phN,%s,t,0x%02x\n", + conn->cid, sess->sess_ops->InitiatorName, sess->isid, + sess->tpg->tpg_tiqn->tiqn, (u32)sess->tpg->tpgt); conn->nopin_response_timer_flags &= ~ISCSI_TF_RUNNING; spin_unlock_bh(&conn->nopin_timer_lock); -- cgit v1.2.3 From dce6190ca78adf7cf6fe794833653e4cb1cb2b73 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Sun, 14 Oct 2018 01:13:54 +0200 Subject: scsi: target: split out helper for cxn timeout error stashing Replace existing nested code blocks with helper function calls. Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_erl0.c | 15 +------------ drivers/target/iscsi/iscsi_target_util.c | 36 ++++++++++++++++++-------------- drivers/target/iscsi/iscsi_target_util.h | 1 + 3 files changed, 22 insertions(+), 30 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c index 718fe9a1b709..1193cf884a28 100644 --- a/drivers/target/iscsi/iscsi_target_erl0.c +++ b/drivers/target/iscsi/iscsi_target_erl0.c @@ -770,21 +770,8 @@ void iscsit_handle_time2retain_timeout(struct timer_list *t) pr_err("Time2Retain timer expired for SID: %u, cleaning up" " iSCSI session.\n", sess->sid); - { - struct iscsi_tiqn *tiqn = tpg->tpg_tiqn; - - if (tiqn) { - spin_lock(&tiqn->sess_err_stats.lock); - strcpy(tiqn->sess_err_stats.last_sess_fail_rem_name, - (void *)sess->sess_ops->InitiatorName); - tiqn->sess_err_stats.last_sess_failure_type = - ISCSI_SESS_ERR_CXN_TIMEOUT; - tiqn->sess_err_stats.cxn_timeout_errors++; - atomic_long_inc(&sess->conn_timeout_errors); - spin_unlock(&tiqn->sess_err_stats.lock); - } - } + iscsit_fill_cxn_timeout_err_stats(sess); spin_unlock_bh(&se_tpg->session_lock); iscsit_close_session(sess); } diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 931c51f56435..1227872227dc 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -933,22 +933,7 @@ void iscsit_handle_nopin_response_timeout(struct timer_list *t) conn->nopin_response_timer_flags &= ~ISCSI_TF_RUNNING; spin_unlock_bh(&conn->nopin_timer_lock); - { - struct iscsi_portal_group *tpg = conn->sess->tpg; - struct iscsi_tiqn *tiqn = tpg->tpg_tiqn; - - if (tiqn) { - spin_lock_bh(&tiqn->sess_err_stats.lock); - strcpy(tiqn->sess_err_stats.last_sess_fail_rem_name, - conn->sess->sess_ops->InitiatorName); - tiqn->sess_err_stats.last_sess_failure_type = - ISCSI_SESS_ERR_CXN_TIMEOUT; - tiqn->sess_err_stats.cxn_timeout_errors++; - atomic_long_inc(&conn->sess->conn_timeout_errors); - spin_unlock_bh(&tiqn->sess_err_stats.lock); - } - } - + iscsit_fill_cxn_timeout_err_stats(sess); iscsit_cause_connection_reinstatement(conn, 0); iscsit_dec_conn_usage_count(conn); } @@ -1407,3 +1392,22 @@ struct iscsi_tiqn *iscsit_snmp_get_tiqn(struct iscsi_conn *conn) return tpg->tpg_tiqn; } + +void iscsit_fill_cxn_timeout_err_stats(struct iscsi_session *sess) +{ + struct iscsi_portal_group *tpg = sess->tpg; + struct iscsi_tiqn *tiqn = tpg->tpg_tiqn; + + if (!tiqn) + return; + + spin_lock_bh(&tiqn->sess_err_stats.lock); + strlcpy(tiqn->sess_err_stats.last_sess_fail_rem_name, + sess->sess_ops->InitiatorName, + sizeof(tiqn->sess_err_stats.last_sess_fail_rem_name)); + tiqn->sess_err_stats.last_sess_failure_type = + ISCSI_SESS_ERR_CXN_TIMEOUT; + tiqn->sess_err_stats.cxn_timeout_errors++; + atomic_long_inc(&sess->conn_timeout_errors); + spin_unlock_bh(&tiqn->sess_err_stats.lock); +} diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h index d66dfc212624..68e84803b0a1 100644 --- a/drivers/target/iscsi/iscsi_target_util.h +++ b/drivers/target/iscsi/iscsi_target_util.h @@ -67,5 +67,6 @@ extern int rx_data(struct iscsi_conn *, struct kvec *, int, int); extern int tx_data(struct iscsi_conn *, struct kvec *, int, int); extern void iscsit_collect_login_stats(struct iscsi_conn *, u8, u8); extern struct iscsi_tiqn *iscsit_snmp_get_tiqn(struct iscsi_conn *); +extern void iscsit_fill_cxn_timeout_err_stats(struct iscsi_session *); #endif /*** ISCSI_TARGET_UTIL_H ***/ -- cgit v1.2.3 From 33b3f8ca510a2181574d6dcbd312c2b07dd9f0fa Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 12 Oct 2018 12:01:20 +0200 Subject: scsi: target: stash sess_err_stats on Data-Out timeout sess_err_stats are currently filled on NOP ping timeout, but not Data-Out timeout. Stash details of Data-Out timeouts using a ISCSI_SESS_ERR_CXN_TIMEOUT value for last_sess_failure_type. Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_erl1.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c index 7f3a1a06696e..a211e8154f4c 100644 --- a/drivers/target/iscsi/iscsi_target_erl1.c +++ b/drivers/target/iscsi/iscsi_target_erl1.c @@ -1230,6 +1230,7 @@ void iscsit_handle_dataout_timeout(struct timer_list *t) failure: spin_unlock_bh(&cmd->dataout_timeout_lock); + iscsit_fill_cxn_timeout_err_stats(sess); iscsit_cause_connection_reinstatement(conn, 0); iscsit_dec_conn_usage_count(conn); } -- cgit v1.2.3 From 4240d448a483e8c2811dc914a1408f606fe13347 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 15 Oct 2018 08:51:34 -0700 Subject: scsi: target/core: Fix spelling in two source code comments Change one occurrence of "aleady" into "already" and one occurrence of "is" into "if". Reviewed-by: Christoph Hellwig Cc: Nicholas Bellinger Cc: Mike Christie Cc: Hannes Reinecke Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index fc3093d21b96..16c774b868be 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2012,7 +2012,7 @@ void target_execute_cmd(struct se_cmd *cmd) * Determine if frontend context caller is requesting the stopping of * this command for frontend exceptions. * - * If the received CDB has aleady been aborted stop processing it here. + * If the received CDB has already been aborted stop processing it here. */ spin_lock_irq(&cmd->t_state_lock); if (__transport_check_aborted_status(cmd, 1)) { @@ -2516,7 +2516,7 @@ transport_generic_new_cmd(struct se_cmd *cmd) } /* - * Determine is the TCM fabric module has already allocated physical + * Determine if the TCM fabric module has already allocated physical * memory, and is directly calling transport_generic_map_mem_to_cmd() * beforehand. */ -- cgit v1.2.3 From c1fbff863595a3ca9bbb93ec4abec7c05cb0839c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 15 Oct 2018 08:51:35 -0700 Subject: scsi: target/core: Remove an unused data member from struct xcopy_pt_cmd A value is assigned to the xcopy_op member of struct xcopy_pt_cmd but that value is never used. Hence remove the xcopy_op member. Reviewed-by: Christoph Hellwig Cc: Nicholas Bellinger Cc: Mike Christie Cc: Hannes Reinecke Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_xcopy.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 2718a933c0c6..70adcfdca8d1 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -391,7 +391,6 @@ out: struct xcopy_pt_cmd { bool remote_port; struct se_cmd se_cmd; - struct xcopy_op *xcopy_op; struct completion xpt_passthrough_sem; unsigned char sense_buffer[TRANSPORT_SENSE_BUFFER]; }; @@ -596,8 +595,6 @@ static int target_xcopy_setup_pt_cmd( * X-COPY PUSH or X-COPY PULL based upon where the CDB was received. */ target_xcopy_init_pt_lun(se_dev, cmd, remote_port); - - xpt_cmd->xcopy_op = xop; target_xcopy_setup_pt_port(xpt_cmd, xop, remote_port); cmd->tag = 0; -- cgit v1.2.3 From 5e568d22fc7963a1cfe0d7d87c46c2ed6a934369 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 15 Oct 2018 08:51:36 -0700 Subject: scsi: target/core: Remove the SCF_COMPARE_AND_WRITE_POST flag Commit 057085e522f8 ("target: Fix race for SCF_COMPARE_AND_WRITE_POST checking") removed the code that checks the SCF_COMPARE_AND_WRITE_POST flag. Hence also remove the flag itself. Cc: Nicholas Bellinger Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_sbc.c | 6 ------ include/target/target_core_base.h | 1 - 2 files changed, 7 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index ebac2b49b9c6..f2c3b67dbb36 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -425,14 +425,8 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success, struct se_device *dev = cmd->se_dev; sense_reason_t ret = TCM_NO_SENSE; - /* - * Only set SCF_COMPARE_AND_WRITE_POST to force a response fall-through - * within target_complete_ok_work() if the command was successfully - * sent to the backend driver. - */ spin_lock_irq(&cmd->t_state_lock); if (cmd->transport_state & CMD_T_SENT) { - cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST; *post_ret = 1; if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION) diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 7a4ee7852ca4..e3bdb0550a59 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -138,7 +138,6 @@ enum se_cmd_flags_table { SCF_ALUA_NON_OPTIMIZED = 0x00008000, SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC = 0x00020000, SCF_COMPARE_AND_WRITE = 0x00080000, - SCF_COMPARE_AND_WRITE_POST = 0x00100000, SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC = 0x00200000, SCF_ACK_KREF = 0x00400000, SCF_USE_CPUID = 0x00800000, -- cgit v1.2.3 From 80b045b385cfef10939c913fbfeb19ce5491c1f2 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 15 Oct 2018 08:51:37 -0700 Subject: scsi: target/core: Use the SECTOR_SHIFT constant Instead of duplicating the SECTOR_SHIFT definition from , use it. This patch does not change any functionality. Reviewed-by: Christoph Hellwig Cc: Nicholas Bellinger Cc: Mike Christie Cc: Hannes Reinecke Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_iblock.c | 4 ++-- drivers/target/target_core_iblock.h | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index d97624620aab..b5ed9c377060 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -514,7 +514,7 @@ iblock_execute_write_same(struct se_cmd *cmd) } /* Always in 512 byte units for Linux/Block */ - block_lba += sg->length >> IBLOCK_LBA_SHIFT; + block_lba += sg->length >> SECTOR_SHIFT; sectors -= 1; } @@ -777,7 +777,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, } /* Always in 512 byte units for Linux/Block */ - block_lba += sg->length >> IBLOCK_LBA_SHIFT; + block_lba += sg->length >> SECTOR_SHIFT; sg_num--; } diff --git a/drivers/target/target_core_iblock.h b/drivers/target/target_core_iblock.h index 9cc3843404d4..cefc641145b3 100644 --- a/drivers/target/target_core_iblock.h +++ b/drivers/target/target_core_iblock.h @@ -9,7 +9,6 @@ #define IBLOCK_VERSION "4.0" #define IBLOCK_MAX_CDBS 16 -#define IBLOCK_LBA_SHIFT 9 struct iblock_req { refcount_t pending; -- cgit v1.2.3 From 81b6ca6dbada186d0d41c67db8d8eb223a64a56f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 15 Oct 2018 08:51:38 -0700 Subject: scsi: target/core: Use sg_alloc_table() instead of open-coding it The purpose of sg_alloc_table() is to allocate and initialize an sg-list. Use that function instead of open-coding it. This patch will make it easier to share code for caching sg-list allocations between the SCSI and NVMe target cores. Signed-off-by: Bart Van Assche Cc: Nicholas Bellinger Cc: Mike Christie Cc: Christoph Hellwig Cc: Hannes Reinecke Signed-off-by: Martin K. Petersen --- drivers/target/target_core_sbc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index f2c3b67dbb36..3d3568a037b1 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -447,7 +447,8 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes int *post_ret) { struct se_device *dev = cmd->se_dev; - struct scatterlist *write_sg = NULL, *sg; + struct sg_table write_tbl = { }; + struct scatterlist *write_sg, *sg; unsigned char *buf = NULL, *addr; struct sg_mapping_iter m; unsigned int offset = 0, len; @@ -488,14 +489,12 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes goto out; } - write_sg = kmalloc_array(cmd->t_data_nents, sizeof(*write_sg), - GFP_KERNEL); - if (!write_sg) { + if (sg_alloc_table(&write_tbl, cmd->t_data_nents, GFP_KERNEL) < 0) { pr_err("Unable to allocate compare_and_write sg\n"); ret = TCM_OUT_OF_RESOURCES; goto out; } - sg_init_table(write_sg, cmd->t_data_nents); + write_sg = write_tbl.sgl; /* * Setup verify and write data payloads from total NumberLBAs. */ @@ -591,7 +590,7 @@ out: * sbc_compare_and_write() before the original READ I/O submission. */ up(&dev->caw_sem); - kfree(write_sg); + sg_free_table(&write_tbl); kfree(buf); return ret; } -- cgit v1.2.3 From aa73237dcb2d96d7a3292af8ca943dd149fd39af Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 15 Oct 2018 08:51:39 -0700 Subject: scsi: target/core: Always call transport_complete_callback() upon failure COMPARE AND WRITE command execution starts with a call of sbc_compare_and_write(). That function locks the caw_sem member in the backend device data structure and submits a read request to the backend driver. Upon successful completion of the read compare_and_write_callback() gets called. That last function compares the data that has been read. If it matches transport_complete_callback is set to compare_and_write_post and a write request is submitted. compare_and_write_post() submits a write request to the backend driver. XDWRITEREAD command execution starts with sbc_execute_rw() submitting a read to the backend device. Upon successful completion of the read the xdreadwrite_callback() gets called. That function xors the data that has been read with the data in the data-out buffer and stores the result in the data-in buffer. Call transport_complete_callback() not only if COMPARE AND WRITE fails but also if XDWRITEREAD fails. This makes the code more systematic. Make sure that the callback functions handle (cmd, false, NULL) argument triples fine. Reviewed-by: Christoph Hellwig Reviewed-by: Nicholas Bellinger Cc: Mike Christie Cc: Hannes Reinecke Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/target/target_core_sbc.c | 6 +++++- drivers/target/target_core_transport.c | 11 +++-------- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 3d3568a037b1..1ac1f7d2e6c9 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -360,6 +360,10 @@ static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd, bool success, unsigned int offset; sense_reason_t ret = TCM_NO_SENSE; int i, count; + + if (!success) + return 0; + /* * From sbc3r22.pdf section 5.48 XDWRITEREAD (10) command * @@ -426,7 +430,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success, sense_reason_t ret = TCM_NO_SENSE; spin_lock_irq(&cmd->t_state_lock); - if (cmd->transport_state & CMD_T_SENT) { + if (success) { *post_ret = 1; if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 16c774b868be..4cf33e2cc705 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1778,7 +1778,7 @@ EXPORT_SYMBOL(target_submit_tmr); void transport_generic_request_failure(struct se_cmd *cmd, sense_reason_t sense_reason) { - int ret = 0, post_ret = 0; + int ret = 0; pr_debug("-----[ Storage Engine Exception; sense_reason %d\n", sense_reason); @@ -1789,13 +1789,8 @@ void transport_generic_request_failure(struct se_cmd *cmd, */ transport_complete_task_attr(cmd); - /* - * Handle special case for COMPARE_AND_WRITE failure, where the - * callback is expected to drop the per device ->caw_sem. - */ - if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) && - cmd->transport_complete_callback) - cmd->transport_complete_callback(cmd, false, &post_ret); + if (cmd->transport_complete_callback) + cmd->transport_complete_callback(cmd, false, NULL); if (transport_check_aborted_status(cmd, 1)) return; -- cgit v1.2.3