[1/2] target: Fix LUN_RESET active I/O handling for ACK_KREF

Message ID	1452594245-921-2-git-send-email-nab@daterainc.com (mailing list archive)
State	Not Applicable, archived
Headers	show Return-Path: <linux-scsi-owner@kernel.org> From: "Nicholas A. Bellinger" <nab@daterainc.com> To: target-devel <target-devel@vger.kernel.org> Cc: linux-scsi <linux-scsi@vger.kernel.org>, Quinn Tran <quinn.tran@qlogic.com>, Himanshu Madhani <himanshu.madhani@qlogic.com>, Sagi Grimberg <sagig@mellanox.com>, Christoph Hellwig <hch@lst.de>, Hannes Reinecke <hare@suse.de>, Andy Grover <agrover@redhat.com>, Nicholas Bellinger <nab@linux-iscsi.org> Subject: [PATCH 1/2] target: Fix LUN_RESET active I/O handling for ACK_KREF Date: Tue, 12 Jan 2016 10:24:04 +0000 Message-Id: <1452594245-921-2-git-send-email-nab@daterainc.com> In-Reply-To: <1452594245-921-1-git-send-email-nab@daterainc.com> References: <1452594245-921-1-git-send-email-nab@daterainc.com> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 7137042..af4adb6 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -107,6 +107,29 @@ static int target_check_cdb_and_preempt(struct list_head *list, return 1; } +/* + * Called with se_session->sess_cmd_lock held with irq disabled + */ +static bool __target_check_io_state(struct se_cmd *se_cmd) +{ + struct se_session *sess = se_cmd->se_sess; + + if (!sess) + return false; + + spin_lock(&se_cmd->t_state_lock); + if (se_cmd->transport_state & CMD_T_COMPLETE) { + printk("Attempted to abort io tag: %llu already complete," + " skipping\n", se_cmd->tag); + spin_unlock(&se_cmd->t_state_lock); + return false; + } + se_cmd->transport_state |= CMD_T_ABORTED; + spin_unlock(&se_cmd->t_state_lock); + + return kref_get_unless_zero(&se_cmd->cmd_kref); +} + void core_tmr_abort_task( struct se_device *dev, struct se_tmr_req *tmr, @@ -132,27 +155,23 @@ void core_tmr_abort_task( printk("ABORT_TASK: Found referenced %s task_tag: %llu\n", se_cmd->se_tfo->get_fabric_name(), ref_tag); - - spin_lock(&se_cmd->t_state_lock); - if (se_cmd->transport_state & CMD_T_COMPLETE) { - printk("ABORT_TASK: ref_tag: %llu already complete," - " skipping\n", ref_tag); - spin_unlock(&se_cmd->t_state_lock); + /* + * Obtain cmd_kref and move to list for shutdown processing + * if se_cmd->cmd_kref is still active, the command has not + * already reached CMD_T_COMPLETE + */ + if (!__target_check_io_state(se_cmd)) { spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); goto out; } - se_cmd->transport_state |= CMD_T_ABORTED; - spin_unlock(&se_cmd->t_state_lock); - list_del_init(&se_cmd->se_cmd_list); - kref_get(&se_cmd->cmd_kref); spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); cancel_work_sync(&se_cmd->work); transport_wait_for_tasks(se_cmd); - target_put_sess_cmd(se_cmd); transport_cmd_finish_abort(se_cmd, true); + target_put_sess_cmd(se_cmd); printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" " ref_tag: %llu\n", ref_tag); @@ -237,8 +256,10 @@ static void core_tmr_drain_state_list( struct list_head *preempt_and_abort_list) { LIST_HEAD(drain_task_list); + struct se_session *sess; struct se_cmd *cmd, *next; unsigned long flags; + int rc; /* * Complete outstanding commands with TASK_ABORTED SAM status. @@ -277,6 +298,24 @@ static void core_tmr_drain_state_list( if (prout_cmd == cmd) continue; + sess = cmd->se_sess; + if (!sess) { + pr_err("NULL cmd->sess for tag: %llu\n", cmd->tag); + dump_stack(); + continue; + } + /* + * Obtain cmd_kref and move to list for shutdown processing + * if se_cmd->cmd_kref is still active, the command has not + * already reached CMD_T_COMPLETE + */ + spin_lock(&sess->sess_cmd_lock); + rc = __target_check_io_state(cmd); + spin_unlock(&sess->sess_cmd_lock); + if (!rc) { + printk("LUN_RESET I/O: non-zero kref_get_unless_zero\n"); + continue; + } list_move_tail(&cmd->state_list, &drain_task_list); cmd->state_active = false; } @@ -308,16 +347,11 @@ static void core_tmr_drain_state_list( * loop above, but we do it down here given that * cancel_work_sync may block. */ - if (cmd->t_state == TRANSPORT_COMPLETE) - cancel_work_sync(&cmd->work); - - spin_lock_irqsave(&cmd->t_state_lock, flags); - target_stop_cmd(cmd, &flags); - - cmd->transport_state |= CMD_T_ABORTED; - spin_unlock_irqrestore(&cmd->t_state_lock, flags); + cancel_work_sync(&cmd->work); + transport_wait_for_tasks(cmd); core_tmr_handle_tas_abort(tmr_nacl, cmd, tas); + target_put_sess_cmd(cmd); } } diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index c5035b9..f2c596b 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -626,6 +626,8 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd) void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) { + bool ack_kref = (cmd->se_cmd_flags & SCF_ACK_KREF); + if (cmd->se_cmd_flags & SCF_SE_LUN_CMD) transport_lun_remove_cmd(cmd); /* @@ -637,7 +639,7 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) if (transport_cmd_check_stop_to_fabric(cmd)) return; - if (remove) + if (remove && ack_kref) transport_put_cmd(cmd); } @@ -2219,20 +2221,14 @@ static inline void transport_free_pages(struct se_cmd *cmd) } /** - * transport_release_cmd - free a command - * @cmd: command to free + * transport_put_cmd - release a reference to a command + * @cmd: command to release * - * This routine unconditionally frees a command, and reference counting - * or list removal must be done in the caller. + * This routine releases our reference to the command and frees it if possible. */ -static int transport_release_cmd(struct se_cmd *cmd) +static int transport_put_cmd(struct se_cmd *cmd) { BUG_ON(!cmd->se_tfo); - - if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) - core_tmr_release_req(cmd->se_tmr_req); - if (cmd->t_task_cdb != cmd->__t_task_cdb) - kfree(cmd->t_task_cdb); /* * If this cmd has been setup with target_get_sess_cmd(), drop * the kref and call ->release_cmd() in kref callback. @@ -2240,18 +2236,6 @@ static int transport_release_cmd(struct se_cmd *cmd) return target_put_sess_cmd(cmd); } -/** - * transport_put_cmd - release a reference to a command - * @cmd: command to release - * - * This routine releases our reference to the command and frees it if possible. - */ -static int transport_put_cmd(struct se_cmd *cmd) -{ - transport_free_pages(cmd); - return transport_release_cmd(cmd); -} - void *transport_kmap_data_sg(struct se_cmd *cmd) { struct scatterlist *sg = cmd->t_data_sg; @@ -2456,7 +2440,7 @@ int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) if (wait_for_tasks && (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) transport_wait_for_tasks(cmd); - ret = transport_release_cmd(cmd); + ret = transport_put_cmd(cmd); } else { if (wait_for_tasks) transport_wait_for_tasks(cmd); @@ -2495,8 +2479,10 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref) * fabric acknowledgement that requires two target_put_sess_cmd() * invocations before se_cmd descriptor release. */ - if (ack_kref) + if (ack_kref) { + se_cmd->se_cmd_flags |= SCF_ACK_KREF; kref_get(&se_cmd->cmd_kref); + } spin_lock_irqsave(&se_sess->sess_cmd_lock, flags); if (se_sess->sess_tearing_down) { @@ -2514,6 +2500,16 @@ out: } EXPORT_SYMBOL(target_get_sess_cmd); +static void target_free_cmd_mem(struct se_cmd *cmd) +{ + transport_free_pages(cmd); + + if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) + core_tmr_release_req(cmd->se_tmr_req); + if (cmd->t_task_cdb != cmd->__t_task_cdb) + kfree(cmd->t_task_cdb); +} + static void target_release_cmd_kref(struct kref *kref) __releases(&se_cmd->se_sess->sess_cmd_lock) { @@ -2522,17 +2518,20 @@ static void target_release_cmd_kref(struct kref *kref) if (list_empty(&se_cmd->se_cmd_list)) { spin_unlock(&se_sess->sess_cmd_lock); + target_free_cmd_mem(se_cmd); se_cmd->se_tfo->release_cmd(se_cmd); return; } if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) { spin_unlock(&se_sess->sess_cmd_lock); + target_free_cmd_mem(se_cmd); complete(&se_cmd->cmd_wait_comp); return; } list_del(&se_cmd->se_cmd_list); spin_unlock(&se_sess->sess_cmd_lock); + target_free_cmd_mem(se_cmd); se_cmd->se_tfo->release_cmd(se_cmd); } diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index a4bed07..1060c52 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -140,6 +140,7 @@ enum se_cmd_flags_table { SCF_COMPARE_AND_WRITE = 0x00080000, SCF_COMPARE_AND_WRITE_POST = 0x00100000, SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC = 0x00200000, + SCF_ACK_KREF = 0x00400000, }; /* struct se_dev_entry->lun_flags and struct se_lun->lun_access */

[1/2] target: Fix LUN_RESET active I/O handling for ACK_KREF

Commit Message

Comments

Patch