From: Himanshu Madhani <himanshu.madhani@qlogic.com> To: target-devel@vger.kernel.org, nab@linux-iscsi.org Cc: giridhar.malavali@qlogic.com, linux-scsi@vger.kernel.org, himanshu.madhani@qlogic.com Subject: [PATCH 12/20] target/tmr: LUN reset cause cmd premature free. Date: Mon, 7 Dec 2015 19:48:59 -0500 [thread overview] Message-ID: <1449535747-2850-13-git-send-email-himanshu.madhani@qlogic.com> (raw) In-Reply-To: <1449535747-2850-1-git-send-email-himanshu.madhani@qlogic.com> From: Quinn Tran <quinn.tran@qlogic.com> During LUN/Target reset, the TMR code attempt to intercept cmds and try to aborted them. Current code assume cmds are always intercepted at the back end device. The cleanup code would issue a "queue_status() & check_stop_free()" to terminate the command. However, when a cmd is intercepted at the front end/Fabric layer, current code introduce premature free or cause Fabric to double free. When command is intercepted at Fabric layer, it means a check_stop_free(cmd_kref--) has been called. The extra check_stop_free in the Lun Reset cleanup code causes early free. When a cmd in the Fabric layer is completed, the normal free code adds another another free which introduce a double free. To fix the issue: - add a new flag/CMD_T_SENT_STATUS to track command that have made it down to fabric layer after back end good/bad completion. - if cmd reach Fabric Layer at Lun Reset time, add an extra cmd_kref count to prevent premature free. Signed-off-by: Quinn Tran <quinn.tran@qlogic.com> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com> --- drivers/target/target_core_tmr.c | 33 +++++++++++++++++++++++++++++++- drivers/target/target_core_transport.c | 30 +++++++++++++++++++++++++++++ include/target/target_core_base.h | 1 + 3 files changed, 63 insertions(+), 1 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 28fb301..41f8b57 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -243,7 +243,9 @@ static void core_tmr_drain_state_list( { LIST_HEAD(drain_task_list); struct se_cmd *cmd, *next; - unsigned long flags; + unsigned long flags, flags2; + int rmkref; + struct se_session *se_sess; /* * Complete outstanding commands with TASK_ABORTED SAM status. @@ -282,6 +284,16 @@ static void core_tmr_drain_state_list( if (prout_cmd == cmd) continue; + se_sess = cmd->se_sess; + /* take an extra kref to prevent cmd free race condition. */ + spin_lock_irqsave(&se_sess->sess_cmd_lock, flags2); + if (!kref_get_unless_zero(&cmd->cmd_kref)) { + /* cmd is already in the free process */ + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags2); + continue; + } + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags2); + list_move_tail(&cmd->state_list, &drain_task_list); cmd->state_active = false; } @@ -320,9 +332,28 @@ static void core_tmr_drain_state_list( target_stop_cmd(cmd, &flags); cmd->transport_state |= CMD_T_ABORTED; + + /* CMD_T_SENT_STATUS: cmd is down in fabric layer. + * A check stop has been called. Keep the extra kref + * from above because core_tmr_handle_tas_abort will + * generate another check_stop. + * + * !CMD_T_SENT_STATUS: cmd intercepted at back end. + * Remove the extra kref from above because only + * 1 check_stop is required or generated by + * core_tmr_handle_tas_abort() + */ + rmkref = 0; + if (!((cmd->t_state == TRANSPORT_COMPLETE) && + (cmd->transport_state & CMD_T_SENT_STATUS))) + rmkref = 1; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); core_tmr_handle_tas_abort(tmr_nacl, cmd, tas); + + if (rmkref) + target_put_sess_cmd(cmd); } } diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 4fdcee2..cdd18bf 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -639,9 +639,14 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) static void target_complete_failure_work(struct work_struct *work) { struct se_cmd *cmd = container_of(work, struct se_cmd, work); + unsigned long flags; transport_generic_request_failure(cmd, TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE); + + spin_lock_irqsave(&cmd->t_state_lock, flags); + cmd->transport_state |= CMD_T_SENT_STATUS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); } /* @@ -1659,6 +1664,7 @@ void transport_generic_request_failure(struct se_cmd *cmd, sense_reason_t sense_reason) { int ret = 0, post_ret = 0; + unsigned long flags; pr_debug("-----[ Storage Engine Exception for cmd: %p ITT: 0x%08llx" " CDB: 0x%02x\n", cmd, cmd->tag, cmd->t_task_cdb[0]); @@ -1670,6 +1676,10 @@ void transport_generic_request_failure(struct se_cmd *cmd, (cmd->transport_state & CMD_T_STOP) != 0, (cmd->transport_state & CMD_T_SENT) != 0); + spin_lock_irqsave(&cmd->t_state_lock, flags); + cmd->transport_state |= CMD_T_SENT_STATUS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + /* * For SAM Task Attribute emulation for failed struct se_cmd */ @@ -1951,6 +1961,7 @@ static void transport_complete_task_attr(struct se_cmd *cmd) static void transport_complete_qf(struct se_cmd *cmd) { int ret = 0; + unsigned long flags; transport_complete_task_attr(cmd); @@ -1986,6 +1997,10 @@ out: } transport_lun_remove_cmd(cmd); transport_cmd_check_stop_to_fabric(cmd); + + spin_lock_irqsave(&cmd->t_state_lock, flags); + cmd->transport_state |= CMD_T_SENT_STATUS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); } static void transport_handle_queue_full( @@ -2032,6 +2047,7 @@ static void target_complete_ok_work(struct work_struct *work) { struct se_cmd *cmd = container_of(work, struct se_cmd, work); int ret; + unsigned long flags; /* * Check if we need to move delayed/dormant tasks from cmds on the @@ -2060,6 +2076,10 @@ static void target_complete_ok_work(struct work_struct *work) transport_lun_remove_cmd(cmd); transport_cmd_check_stop_to_fabric(cmd); + + spin_lock_irqsave(&cmd->t_state_lock, flags); + cmd->transport_state |= CMD_T_SENT_STATUS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); return; } /* @@ -2086,6 +2106,11 @@ static void target_complete_ok_work(struct work_struct *work) transport_lun_remove_cmd(cmd); transport_cmd_check_stop_to_fabric(cmd); + + spin_lock_irqsave(&cmd->t_state_lock, flags); + cmd->transport_state |= CMD_T_SENT_STATUS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + return; } } @@ -2136,6 +2161,7 @@ queue_rsp: ret = cmd->se_tfo->queue_status(cmd); if (ret == -EAGAIN || ret == -ENOMEM) goto queue_full; + break; default: break; @@ -2143,6 +2169,10 @@ queue_rsp: transport_lun_remove_cmd(cmd); transport_cmd_check_stop_to_fabric(cmd); + + spin_lock_irqsave(&cmd->t_state_lock, flags); + cmd->transport_state |= CMD_T_SENT_STATUS; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); return; queue_full: diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index aabf0ac..efccd71 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -490,6 +490,7 @@ struct se_cmd { #define CMD_T_DEV_ACTIVE (1 << 7) #define CMD_T_REQUEST_STOP (1 << 8) #define CMD_T_BUSY (1 << 9) +#define CMD_T_SENT_STATUS (1 << 10) spinlock_t t_state_lock; struct kref cmd_kref; struct completion t_transport_stop_comp; -- 1.7.7
next prev parent reply other threads:[~2015-12-08 0:48 UTC|newest] Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-12-08 0:48 [PATCH 00/20] qla2xxx: Patches for target-pending branch Himanshu Madhani 2015-12-08 0:48 ` [PATCH 01/20] qla2xxx: Enable Extended Login support Himanshu Madhani 2015-12-08 15:51 ` Hannes Reinecke 2015-12-08 19:35 ` Himanshu Madhani 2015-12-08 0:48 ` [PATCH 02/20] qla2xxx: Enable Exchange offload support Himanshu Madhani 2015-12-08 15:52 ` Hannes Reinecke 2015-12-08 0:48 ` [PATCH 03/20] qla2xxx: Enable Target counters in DebugFS Himanshu Madhani 2015-12-08 15:52 ` Hannes Reinecke 2015-12-08 0:48 ` [PATCH 04/20] qla2xxx: Add FW resource count " Himanshu Madhani 2015-12-08 15:53 ` Hannes Reinecke 2015-12-08 0:48 ` [PATCH 05/20] qla2xxx: Added interface to send ELS commands from driver Himanshu Madhani 2015-12-08 2:10 ` kbuild test robot 2015-12-08 2:10 ` [PATCH] qla2xxx: fix ifnullfree.cocci warnings kbuild test robot 2015-12-08 15:54 ` [PATCH 05/20] qla2xxx: Added interface to send ELS commands from driver Hannes Reinecke 2015-12-08 0:48 ` [PATCH 06/20] qla2xxx: Delete session if initiator is gone from FW Himanshu Madhani 2015-12-08 1:41 ` kbuild test robot 2015-12-08 15:58 ` Hannes Reinecke 2015-12-08 0:48 ` [PATCH 07/20] qla2xxx: Wait for all conflicts before ack'ing PLOGI Himanshu Madhani 2015-12-08 16:00 ` Hannes Reinecke 2015-12-08 0:48 ` [PATCH 08/20] qla2xxx: Replace QLA_TGT_STATE_ABORTED with a bit Himanshu Madhani 2015-12-08 16:01 ` Hannes Reinecke 2015-12-08 0:48 ` [PATCH 09/20] qla2xxx: Change check_stop_free to always return 1 Himanshu Madhani 2015-12-08 2:33 ` Christoph Hellwig 2015-12-09 6:56 ` Hannes Reinecke 2015-12-10 1:06 ` Quinn Tran 2015-12-08 0:48 ` [PATCH 10/20] qla2xxx: Fix interaction issue between qla2xxx and Target Core Module Himanshu Madhani 2015-12-08 2:37 ` Christoph Hellwig 2015-12-09 22:07 ` Quinn Tran 2015-12-14 10:34 ` Christoph Hellwig 2015-12-14 21:59 ` Quinn Tran 2015-12-09 7:01 ` Hannes Reinecke 2015-12-09 22:41 ` Quinn Tran 2015-12-08 0:48 ` [PATCH 11/20] qla2xxx: Add TAS detection for kernel 3.15 n newer Himanshu Madhani 2015-12-08 2:48 ` Christoph Hellwig 2015-12-09 20:24 ` Quinn Tran 2015-12-14 10:37 ` Christoph Hellwig 2015-12-14 22:00 ` Quinn Tran 2015-12-09 7:02 ` Hannes Reinecke 2015-12-08 0:48 ` Himanshu Madhani [this message] 2015-12-08 2:48 ` [PATCH 12/20] target/tmr: LUN reset cause cmd premature free Christoph Hellwig 2015-12-09 20:11 ` Quinn Tran 2016-01-04 7:44 ` Bart Van Assche 2015-12-09 7:03 ` Hannes Reinecke 2015-12-08 0:49 ` [PATCH 13/20] qla2xxx: Remove dependency on hardware_lock to reduce lock contention Himanshu Madhani 2015-12-08 0:49 ` [PATCH 14/20] qla2xxx: Add irq affinity notification Himanshu Madhani 2015-12-08 0:49 ` [PATCH 15/20] qla2xxx: Add selective command queuing Himanshu Madhani 2015-12-08 0:49 ` [PATCH 16/20] qla2xxx: Move atioq to a different lock to reduce lock contention Himanshu Madhani 2015-12-08 0:49 ` [PATCH 17/20] qla2xxx: Disable ZIO at start time Himanshu Madhani 2015-12-08 0:49 ` [PATCH 18/20] qla2xxx: Set all queues to 4k Himanshu Madhani 2015-12-08 0:49 ` [PATCH 19/20] qla2xxx: Add bulk send for atio & ctio completion paths Himanshu Madhani 2015-12-08 0:49 ` [PATCH 20/20] qla2xxx: Check for online flag instead of active reset when transmitting responses Himanshu Madhani
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1449535747-2850-13-git-send-email-himanshu.madhani@qlogic.com \ --to=himanshu.madhani@qlogic.com \ --cc=giridhar.malavali@qlogic.com \ --cc=linux-scsi@vger.kernel.org \ --cc=nab@linux-iscsi.org \ --cc=target-devel@vger.kernel.org \ --subject='Re: [PATCH 12/20] target/tmr: LUN reset cause cmd premature free.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.