From 47a5c6072c586a17ccd00d3073d9b007607485dc Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 10 Jul 2010 18:27:59 +0000 Subject: [PATCH] Fixed a bug in the command abortion code. E.g. the following kernel message could be generated when unloading ib_srpt while I/O was ongoing: ------------[ cut here ]------------ WARNING: at /home/bart/software/scst/srpt/src/ib_srpt.c:1063 srpt_reset_ioctx+0x15b/0x170 [ib_srpt]() Hardware name: P5Q DELUXE Modules linked in: [ ... ] Pid: 20709, comm: disk011_4 Tainted: G W 2.6.34-scst #1 Call Trace: [] warn_slowpath_common+0x7b/0xc0 [] warn_slowpath_null+0x14/0x20 [] srpt_reset_ioctx+0x15b/0x170 [ib_srpt] [] ? warn_slowpath_common+0x8f/0xc0 [] srpt_on_free_cmd+0x33/0x60 [ib_srpt] [] scst_free_cmd+0xab/0x460 [scst] [] scst_finish_cmd+0x178/0x300 [scst] [] scst_process_active_cmd+0x134/0x640 [scst] [] scst_process_redirect_cmd+0x2bc/0x3f0 [scst] [] scst_tgt_cmd_done+0x6a/0x100 [scst] [] srpt_xmit_response+0x11c/0x210 [ib_srpt] [] scst_xmit_response+0x13c/0x560 [scst] [] scst_process_active_cmd+0x15b/0x640 [scst] [] scst_do_job_active+0xbd/0x180 [scst] [] scst_cmd_thread+0x1c6/0x4b0 [scst] [] ? default_wake_function+0x0/0x20 [] ? scst_cmd_thread+0x0/0x4b0 [scst] [] kthread+0x96/0xa0 [] kernel_thread_helper+0x4/0x10 [] ? finish_task_switch+0x51/0xb0 [] ? _raw_spin_unlock_irq+0x1c/0x40 [] ? restore_args+0x0/0x30 [] ? kthread+0x0/0xa0 [] ? kernel_thread_helper+0x0/0x10 ---[ end trace 84b1e00ea2ac6651 ]--- git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@1812 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 65 ++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index 2b0d58782..47e06d80d 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -1107,21 +1107,21 @@ static void srpt_abort_scst_cmd(struct srpt_ioctx *ioctx, BUG_ON(!ioctx); /* - * Change the state of the command into SRPT_STATE_DONE, except when - * the current state is SRPT_STATE_NEW or SRPT_STATE_NEED_DATA. For - * these last two states, change the state into SRPT_STATE_DATA_IN. - * Doing so ensures that srpt_xmit_response() will call this function - * a second time and will invoke scst_tgt_cmd_done(). Unfortunately it - * is not allowed to invoke scst_tgt_cmd_done() for commands that have - * one of the states SRPT_STATE_NEW or SRPT_STATE_NEED_DATA + * If the command is in a state where the SCST core is waiting for the + * ib_srpt driver, change the state to the next state. Changing the + * state of the command from SRPT_NEED_DATA to SRPT_STATE_DATA_IN + * ensures that srpt_xmit_response() will call this function a second + * time. */ - state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEW, + state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA, SRPT_STATE_DATA_IN); - if (state != SRPT_STATE_NEW) { - state = srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA, - SRPT_STATE_DATA_IN); - if (state != SRPT_STATE_NEED_DATA) - state = srpt_set_cmd_state(ioctx, SRPT_STATE_DONE); + if (state != SRPT_STATE_NEED_DATA) { + state = srpt_test_and_set_cmd_state(ioctx, + SRPT_STATE_CMD_RSP_SENT, SRPT_STATE_DONE); + if (state != SRPT_STATE_CMD_RSP_SENT) + state = srpt_test_and_set_cmd_state(ioctx, + SRPT_STATE_MGMT_RSP_SENT, + SRPT_STATE_DONE); } if (state == SRPT_STATE_DONE) goto out; @@ -1140,19 +1140,26 @@ static void srpt_abort_scst_cmd(struct srpt_ioctx *ioctx, switch (state) { case SRPT_STATE_NEW: - scst_set_delivery_status(scmnd, SCST_CMD_DELIVERY_ABORTED); - break; - case SRPT_STATE_NEED_DATA: + WARN_ON("This code should never be reached."); scst_rx_data(scmnd, SCST_RX_STATUS_ERROR, context); break; + case SRPT_STATE_NEED_DATA: + /* RDMA read error or RDMA read timeout. */ case SRPT_STATE_DATA_IN: - case SRPT_STATE_CMD_RSP_SENT: - scst_set_delivery_status(scmnd, SCST_CMD_DELIVERY_ABORTED); - scst_tgt_cmd_done(scmnd, context); + /* + * SCST command abort flag has been set after the RDMA read + * started and before srpt_xmit_response() has been invoked. + */ + scst_rx_data(scmnd, SCST_RX_STATUS_ERROR, context); break; + case SRPT_STATE_CMD_RSP_SENT: + /* + * SRP_RSP sending failed or the SRP_RSP send completion has + * not been received in time. + */ case SRPT_STATE_MGMT_RSP_SENT: - WARN_ON("ERROR: srpt_abort_scst_cmd() has been called for" - " a management command."); + /* Management command response sending failed. */ + scst_set_delivery_status(scmnd, SCST_CMD_DELIVERY_ABORTED); scst_tgt_cmd_done(scmnd, context); break; default: @@ -2852,7 +2859,6 @@ static int srpt_xmit_response(struct scst_cmd *scmnd) int ret; int dir; int resp_len; - enum srpt_command_state prev_state; EXTRACHECKS_BUG_ON(scst_cmd_atomic(scmnd)); @@ -2862,19 +2868,10 @@ static int srpt_xmit_response(struct scst_cmd *scmnd) ch = scst_sess_get_tgt_priv(scst_cmd_get_session(scmnd)); BUG_ON(!ch); - prev_state = srpt_get_cmd_state(ioctx); if (unlikely(scst_cmd_aborted(scmnd))) { - TRACE_DBG("cmd with tag %lld has been aborted (SRPT state" - " %d -> %d; SCST state %d)", - scst_cmd_get_tag(scmnd), prev_state, - srpt_get_cmd_state(ioctx), - scmnd->state); - srpt_abort_scst_cmd(ioctx, SCST_CONTEXT_SAME); - if (scmnd->state != SCST_CMD_STATE_FINISHED) - PRINT_INFO("cmd with tag %lld has been aborted" - " and has now SCST state %d - session" - " unregistration will wait.", - scst_cmd_get_tag(scmnd), scmnd->state); + WARN_ON(ioctx->mapped_sg_count); + srpt_set_cmd_state(ioctx, SRPT_STATE_DONE); + scst_set_delivery_status(scmnd, SCST_CMD_DELIVERY_ABORTED); ret = SCST_TGT_RES_SUCCESS; goto out; }