Rework iSCSI aborts to make them more correct and remove recent workarounds (one more post-IET cleanup)

git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@3444 d57e44dd-8a1f-0410-8b47-8ef2f437770f
This commit is contained in:
Vladislav Bolkhovitin
2011-05-09 23:07:23 +00:00
parent 1ee4416700
commit 70eb596f60
7 changed files with 144 additions and 107 deletions

View File

@@ -620,9 +620,9 @@ void iscsi_check_tm_data_wait_timeouts(struct iscsi_conn *conn, bool force)
TRACE_ENTRY();
TRACE_DBG_FLAG(force ? TRACE_CONN_OC_DBG : TRACE_MGMT_DEBUG,
"conn %p, read_cmnd %p, read_state %d, j %ld (TIMEOUT %d, "
"force %d)", conn, conn->read_cmnd, conn->read_state, j,
TRACE_DBG_FLAG(TRACE_MGMT_DEBUG, "conn %p, read_cmnd %p, read_state "
"%d, j %ld (TIMEOUT %d, force %d)", conn, conn->read_cmnd,
conn->read_state, j,
ISCSI_TM_DATA_WAIT_TIMEOUT + ISCSI_ADD_SCHED_TIME, force);
iscsi_extracheck_is_rd_thread(conn);
@@ -635,23 +635,19 @@ again:
list_for_each_entry(cmnd, &conn->write_timeout_list,
write_timeout_list_entry) {
if (test_bit(ISCSI_CMD_ABORTED, &cmnd->prelim_compl_flags)) {
TRACE_DBG_FLAG(force ? TRACE_CONN_OC_DBG : TRACE_MGMT_DEBUG,
"Checking aborted cmnd %p (scst_state %d, "
"on_write_timeout_list %d, write_start %ld, "
"r2t_len_to_receive %d)", cmnd, cmnd->scst_state,
TRACE_DBG_FLAG(TRACE_MGMT_DEBUG, "Checking aborted "
"cmnd %p (scst_state %d, on_write_timeout_list "
"%d, write_start %ld, r2t_len_to_receive %d)",
cmnd, cmnd->scst_state,
cmnd->on_write_timeout_list, cmnd->write_start,
cmnd->r2t_len_to_receive);
if (cmnd == conn->read_cmnd) {
TRACE_DBG_FLAG(force ? TRACE_CONN_OC_DBG : TRACE_MGMT_DEBUG,
TRACE_DBG_FLAG(TRACE_MGMT_DEBUG,
"cmnd %p is read cmd", cmnd);
sBUG_ON(force);
if ((conn->read_state == RX_INIT_BHS) ||
(conn->read_state == RX_BHS)) {
TRACE_MGMT_DBG("Unabort not yet received cmnd %p",
cmnd);
clear_bit(ISCSI_CMD_ABORTED, &cmnd->prelim_compl_flags);
continue;
} else if (cmnd->scst_state == ISCSI_CMD_STATE_RX_CMD) {
sBUG_ON((conn->read_state == RX_INIT_BHS) ||
(conn->read_state == RX_BHS));
if (cmnd->scst_state == ISCSI_CMD_STATE_RX_CMD) {
TRACE_MGMT_DBG("Aborted cmnd %p is RX_CMD, "
"keep waiting", cmnd);
goto cont;
@@ -661,7 +657,8 @@ again:
(time_after_eq(j, cmnd->write_start + ISCSI_TM_DATA_WAIT_TIMEOUT) ||
force)) {
if (cmnd == conn->read_cmnd) {
TRACE_MGMT_DBG("Clearing read_cmnd for conn %p", conn);
TRACE_MGMT_DBG("Clearing read_cmnd for "
"conn %p", conn);
conn->read_cmnd = NULL;
conn->read_state = RX_INIT_BHS;
}

View File

@@ -61,7 +61,6 @@ static void iscsi_send_task_mgmt_resp(struct iscsi_cmnd *req, int status);
static void iscsi_check_send_delayed_tm_resp(struct iscsi_session *sess);
static void req_cmnd_release(struct iscsi_cmnd *req);
static int cmnd_insert_data_wait_hash(struct iscsi_cmnd *cmnd);
static void __cmnd_abort(struct iscsi_cmnd *cmnd);
static void iscsi_cmnd_init_write(struct iscsi_cmnd *rsp, int flags);
static void iscsi_set_resid_no_scst_cmd(struct iscsi_cmnd *rsp);
static void iscsi_set_resid(struct iscsi_cmnd *rsp);
@@ -2250,7 +2249,7 @@ static void __cmnd_abort(struct iscsi_cmnd *cmnd)
}
/* Must be called from the read or conn close thread */
static int cmnd_abort(struct iscsi_cmnd *req, int *status)
static int cmnd_abort_pre_checks(struct iscsi_cmnd *req, int *status)
{
struct iscsi_task_mgt_hdr *req_hdr =
(struct iscsi_task_mgt_hdr *)&req->pdu.bhs;
@@ -2268,7 +2267,6 @@ static int cmnd_abort(struct iscsi_cmnd *req, int *status)
cmnd = cmnd_find_itt_get(req->conn, req_hdr->rtt);
if (cmnd) {
struct iscsi_conn *conn = cmnd->conn;
struct iscsi_scsi_cmd_hdr *hdr = cmnd_hdr(cmnd);
if (req_hdr->lun != hdr->lun) {
@@ -2310,10 +2308,6 @@ static int cmnd_abort(struct iscsi_cmnd *req, int *status)
goto out_put;
}
spin_lock_bh(&conn->cmd_list_lock);
__cmnd_abort(cmnd);
spin_unlock_bh(&conn->cmd_list_lock);
cmnd_put(cmnd);
res = 0;
} else {
@@ -2351,69 +2345,86 @@ out_put:
goto out;
}
/* Must be called from the read or conn close thread */
static int target_abort(struct iscsi_cmnd *req, int all)
struct iscsi_cmnd_abort_params {
struct work_struct iscsi_cmnd_abort_work;
struct scst_cmd *scst_cmd;
};
static mempool_t *iscsi_cmnd_abort_mempool;
static void iscsi_cmnd_abort_fn(struct work_struct *work)
{
struct iscsi_target *target = req->conn->session->target;
struct iscsi_task_mgt_hdr *req_hdr =
(struct iscsi_task_mgt_hdr *)&req->pdu.bhs;
struct iscsi_session *session;
struct iscsi_cmnd_abort_params *params = container_of(work,
struct iscsi_cmnd_abort_params, iscsi_cmnd_abort_work);
struct scst_cmd *scst_cmd = params->scst_cmd;
struct iscsi_session *session = scst_sess_get_tgt_priv(scst_cmd->sess);
struct iscsi_conn *conn;
struct iscsi_cmnd *cmnd;
struct iscsi_cmnd *cmnd = scst_cmd_get_tgt_priv(scst_cmd);
bool done = false;
mutex_lock(&target->target_mutex);
TRACE_ENTRY();
list_for_each_entry(session, &target->session_list,
session_list_entry) {
list_for_each_entry(conn, &session->conn_list,
conn_list_entry) {
spin_lock_bh(&conn->cmd_list_lock);
list_for_each_entry(cmnd, &conn->cmd_list,
cmd_list_entry) {
if (cmnd == req)
continue;
if (all)
__cmnd_abort(cmnd);
else if (req_hdr->lun == cmnd_hdr(cmnd)->lun)
__cmnd_abort(cmnd);
}
spin_unlock_bh(&conn->cmd_list_lock);
}
}
TRACE_MGMT_DBG("Checking aborted scst_cmd %p (cmnd %p)", scst_cmd, cmnd);
mutex_unlock(&target->target_mutex);
return 0;
}
/* Must be called from the read or conn close thread */
static void task_set_abort(struct iscsi_cmnd *req)
{
struct iscsi_session *session = req->conn->session;
struct iscsi_task_mgt_hdr *req_hdr =
(struct iscsi_task_mgt_hdr *)&req->pdu.bhs;
struct iscsi_target *target = session->target;
struct iscsi_conn *conn;
struct iscsi_cmnd *cmnd;
mutex_lock(&target->target_mutex);
mutex_lock(&session->target->target_mutex);
/*
* cmnd pointer is valid only under cmd_list_lock, but we can't know the
* corresponding conn without dereferencing cmnd at first, so let's
* check all conns and cmnds to find out if our cmnd is still valid
* under lock.
*/
list_for_each_entry(conn, &session->conn_list, conn_list_entry) {
struct iscsi_cmnd *c;
spin_lock_bh(&conn->cmd_list_lock);
list_for_each_entry(cmnd, &conn->cmd_list, cmd_list_entry) {
struct iscsi_scsi_cmd_hdr *hdr = cmnd_hdr(cmnd);
if (cmnd == req)
continue;
if (req_hdr->lun != hdr->lun)
continue;
if (before(req_hdr->cmd_sn, hdr->cmd_sn) ||
req_hdr->cmd_sn == hdr->cmd_sn)
continue;
__cmnd_abort(cmnd);
list_for_each_entry(c, &conn->cmd_list, cmd_list_entry) {
if (c == cmnd) {
__cmnd_abort(cmnd);
done = true;
break;
}
}
spin_unlock_bh(&conn->cmd_list_lock);
if (done)
break;
}
mutex_unlock(&target->target_mutex);
mutex_unlock(&session->target->target_mutex);
scst_cmd_put(scst_cmd);
mempool_free(params, iscsi_cmnd_abort_mempool);
TRACE_EXIT();
return;
}
static void iscsi_on_abort_cmd(struct scst_cmd *scst_cmd)
{
struct iscsi_cmnd_abort_params *params;
TRACE_ENTRY();
params = mempool_alloc(iscsi_cmnd_abort_mempool, GFP_ATOMIC);
if (params == NULL) {
PRINT_CRIT_ERROR("Unable to create iscsi_cmnd_abort_params, "
"iSCSI cmnd for scst_cmd %p may not be aborted",
scst_cmd);
goto out;
}
memset(params, 0, sizeof(*params));
INIT_WORK(&params->iscsi_cmnd_abort_work, iscsi_cmnd_abort_fn);
params->scst_cmd = scst_cmd;
scst_cmd_get(scst_cmd);
TRACE_MGMT_DBG("Scheduling abort check for scst_cmd %p", scst_cmd);
schedule_work(&params->iscsi_cmnd_abort_work);
out:
TRACE_EXIT();
return;
}
@@ -2516,7 +2527,7 @@ static void execute_task_management(struct iscsi_cmnd *req)
switch (function) {
case ISCSI_FUNCTION_ABORT_TASK:
rc = cmnd_abort(req, &status);
rc = cmnd_abort_pre_checks(req, &status);
if (rc == 0) {
params.fn = SCST_ABORT_TASK;
params.tag = (__force u32)req_hdr->rtt;
@@ -2532,7 +2543,6 @@ static void execute_task_management(struct iscsi_cmnd *req)
}
break;
case ISCSI_FUNCTION_ABORT_TASK_SET:
task_set_abort(req);
params.fn = SCST_ABORT_TASK_SET;
params.lun = (uint8_t *)&req_hdr->lun;
params.lun_len = sizeof(req_hdr->lun);
@@ -2544,7 +2554,6 @@ static void execute_task_management(struct iscsi_cmnd *req)
status = ISCSI_RESPONSE_FUNCTION_REJECTED;
break;
case ISCSI_FUNCTION_CLEAR_TASK_SET:
task_set_abort(req);
params.fn = SCST_CLEAR_TASK_SET;
params.lun = (uint8_t *)&req_hdr->lun;
params.lun_len = sizeof(req_hdr->lun);
@@ -2568,7 +2577,6 @@ static void execute_task_management(struct iscsi_cmnd *req)
break;
case ISCSI_FUNCTION_TARGET_COLD_RESET:
case ISCSI_FUNCTION_TARGET_WARM_RESET:
target_abort(req, 1);
params.fn = SCST_TARGET_RESET;
params.cmd_sn = req_hdr->cmd_sn;
params.cmd_sn_set = 1;
@@ -2577,7 +2585,6 @@ static void execute_task_management(struct iscsi_cmnd *req)
status = ISCSI_RESPONSE_FUNCTION_REJECTED;
break;
case ISCSI_FUNCTION_LOGICAL_UNIT_RESET:
target_abort(req, 0);
params.fn = SCST_LUN_RESET;
params.lun = (uint8_t *)&req_hdr->lun;
params.lun_len = sizeof(req_hdr->lun);
@@ -3827,6 +3834,7 @@ struct scst_tgt_template iscsi_template = {
.pre_exec = iscsi_pre_exec,
.task_mgmt_affected_cmds_done = iscsi_task_mgmt_affected_cmds_done,
.task_mgmt_fn_done = iscsi_task_mgmt_fn_done,
.on_abort_cmd = iscsi_on_abort_cmd,
.report_aen = iscsi_report_aen,
.get_initiator_port_transport_id = iscsi_get_initiator_port_transport_id,
.get_scsi_transport_version = iscsi_get_scsi_transport_version,
@@ -4016,12 +4024,19 @@ static int __init iscsi_init(void)
sg_init_table(&dummy_sg, 1);
sg_set_page(&dummy_sg, dummy_page, PAGE_SIZE, 0);
iscsi_cmnd_abort_mempool = mempool_create_kmalloc_pool(2500,
sizeof(struct iscsi_cmnd_abort_params));
if (iscsi_cmnd_abort_mempool == NULL) {
err = -ENOMEM;
goto out_free_dummy;
}
#if defined(CONFIG_TCP_ZERO_COPY_TRANSFER_COMPLETION_NOTIFICATION)
err = net_set_get_put_page_callbacks(iscsi_get_page_callback,
iscsi_put_page_callback);
if (err != 0) {
PRINT_INFO("Unable to set page callbackes: %d", err);
goto out_free_dummy;
goto out_destroy_mempool;
}
#else
#ifndef GENERATING_UPSTREAM_PATCH
@@ -4092,6 +4107,9 @@ out_callb:
#if defined(CONFIG_TCP_ZERO_COPY_TRANSFER_COMPLETION_NOTIFICATION)
net_set_get_put_page_callbacks(NULL, NULL);
out_destroy_mempool:
mempool_destroy(iscsi_cmnd_abort_mempool);
out_free_dummy:
#endif
__free_pages(dummy_page, 0);
@@ -4119,6 +4137,8 @@ static void __exit iscsi_exit(void)
net_set_get_put_page_callbacks(NULL, NULL);
#endif
mempool_destroy(iscsi_cmnd_abort_mempool);
__free_pages(dummy_page, 0);
return;
}

View File

@@ -672,7 +672,7 @@ static inline void iscsi_cmnd_set_length(struct iscsi_pdu *pdu)
extern struct scst_tgt_template iscsi_template;
/*
* Skip this command if result is not 0. Must be called under
* Skip this command if result is true. Must be called under
* corresponding lock.
*/
static inline bool cmnd_get_check(struct iscsi_cmnd *cmnd)

View File

@@ -19,7 +19,6 @@
#include <linux/sched.h>
#include <linux/file.h>
#include <linux/kthread.h>
#include <asm/ioctls.h>
#include <linux/delay.h>
#include <net/tcp.h>
@@ -839,17 +838,10 @@ static int process_read_io(struct iscsi_conn *conn, int *closed)
res = do_recv(conn);
if (res == 0) {
/*
* Clear aborted status if this command was
* accidentally aborted with other commands of
* this connection. This command not yet
* received on the aborted time, so shouldn't be
* affected by the abort.
* This command not yet received on the aborted
* time, so shouldn't be affected by any abort.
*/
if (cmnd->prelim_compl_flags != 0)
TRACE_MGMT_DBG("Unabort not yet "
"received cmnd %p (flags %lx)",
cmnd, cmnd->prelim_compl_flags);
cmnd->prelim_compl_flags = 0;
EXTRACHECKS_BUG_ON(cmnd->prelim_compl_flags != 0);
iscsi_cmnd_get_length(&cmnd->pdu);

View File

@@ -810,6 +810,15 @@ struct scst_tgt_template {
*/
void (*task_mgmt_fn_done) (struct scst_mgmt_cmd *mgmt_cmd);
/*
* Called to notify target driver that the command is being aborted.
* If target driver wants to redirect processing to some outside
* processing, it should get it using scst_cmd_get().
*
* OPTIONAL
*/
void (*on_abort_cmd) (struct scst_cmd *cmd);
/*
* This function should detect the target adapters that
* are present in the system. The function should return a value
@@ -1232,7 +1241,13 @@ struct scst_dev_type {
* - SCST_DEV_TM_NOT_COMPLETED - regular standard actions for the
* command should be done
*
* Called without any locks held from a thread context.
* Can be called under many internal SCST locks, including under
* disabled IRQs, so dev handler should be careful with locking and,
* if necessary, pass processing somewhere outside (in a work, e.g.)
*
* But at the moment it's called under disabled IRQs only for
* SCST_ABORT_TASK, however dev handler using it should add a BUG_ON
* trap to catch if it's changed in future.
*
* OPTIONAL
*/
@@ -1862,7 +1877,10 @@ struct scst_cmd {
/* Set if cmd is done */
unsigned int done:1;
/* Set if cmd is finished */
/*
* Set if cmd is finished. Used under sess_list_lock to sync
* between scst_finish_cmd() and scst_abort_cmd()
*/
unsigned int finished:1;
#ifdef CONFIG_SCST_DEBUG_TM

View File

@@ -2408,6 +2408,7 @@ again:
return;
}
/* Can be called under some spinlock and IRQs off */
static int dev_user_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
struct scst_tgt_dev *tgt_dev)
{
@@ -2458,7 +2459,7 @@ static int dev_user_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
dev_user_abort_ready_commands(dev);
/* We can't afford missing TM command due to memory shortage */
ucmd = dev_user_alloc_ucmd(dev, GFP_KERNEL|__GFP_NOFAIL);
ucmd = dev_user_alloc_ucmd(dev, GFP_ATOMIC|__GFP_NOFAIL);
ucmd->user_cmd_payload_len =
offsetof(struct scst_user_get_cmd, tm_cmd) +

View File

@@ -508,7 +508,7 @@ int scst_pre_parse(struct scst_cmd *cmd)
TRACE_DBG("op_name <%s> (cmd %p), direction=%d "
"(expected %d, set %s), bufflen=%d, out_bufflen=%d (expected "
"len %d, out expected len %d), flags=%d", cmd->op_name, cmd,
"len %d, out expected len %d), flags=0x%x", cmd->op_name, cmd,
cmd->data_direction, cmd->expected_data_direction,
scst_cmd_is_expected_set(cmd) ? "yes" : "no",
cmd->bufflen, cmd->out_bufflen, cmd->expected_transfer_len,
@@ -3641,10 +3641,13 @@ static int scst_finish_cmd(struct scst_cmd *cmd)
list_del(&cmd->sess_cmd_list_entry);
spin_unlock_irq(&sess->sess_list_lock);
/*
* Done under sess_list_lock to sync with scst_abort_cmd() without
* using extra barrier.
*/
cmd->finished = 1;
smp_mb(); /* to sync with scst_abort_cmd() */
spin_unlock_irq(&sess->sess_list_lock);
if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))) {
TRACE_MGMT_DBG("Aborted cmd %p finished (cmd_ref %d)",
@@ -4613,7 +4616,6 @@ static int scst_call_dev_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
if (h->task_mgmt_fn) {
TRACE_MGMT_DBG("Calling dev handler %s task_mgmt_fn(fn=%d)",
h->name, mcmd->fn);
EXTRACHECKS_BUG_ON(in_irq() || irqs_disabled());
res = h->task_mgmt_fn(mcmd, tgt_dev);
TRACE_MGMT_DBG("Dev handler %s task_mgmt_fn() returned %d",
h->name, res);
@@ -4639,7 +4641,10 @@ static inline int scst_is_strict_mgmt_fn(int mgmt_fn)
}
}
/* Might be called under sess_list_lock and IRQ off + BHs also off */
/*
* Must be called under sess_list_lock to sync with finished flag assignment in
* scst_finish_cmd()
*/
void scst_abort_cmd(struct scst_cmd *cmd, struct scst_mgmt_cmd *mcmd,
bool other_ini, bool call_dev_task_mgmt_fn)
{
@@ -4684,9 +4689,9 @@ void scst_abort_cmd(struct scst_cmd *cmd, struct scst_mgmt_cmd *mcmd,
spin_unlock_irqrestore(&other_ini_lock, flags);
/*
* To sync with cmd->finished/done set in
* scst_finish_cmd()/scst_pre_xmit_response() and with setting UA for
* aborted cmd in scst_set_pending_UA().
* To sync with setting cmd->done in scst_pre_xmit_response() (with
* scst_finish_cmd() we synced by using sess_list_lock) and with
* setting UA for aborted cmd in scst_set_pending_UA().
*/
smp_mb__after_set_bit();
@@ -4697,10 +4702,8 @@ void scst_abort_cmd(struct scst_cmd *cmd, struct scst_mgmt_cmd *mcmd,
wake_up(&scst_init_cmd_list_waitQ);
}
if (call_dev_task_mgmt_fn && (cmd->tgt_dev != NULL)) {
EXTRACHECKS_BUG_ON(irqs_disabled());
if (!cmd->finished && call_dev_task_mgmt_fn && (cmd->tgt_dev != NULL))
scst_call_dev_task_mgmt_fn(mcmd, cmd->tgt_dev, 1);
}
spin_lock_irqsave(&scst_mcmd_lock, flags);
if ((mcmd != NULL) && !cmd->finished) {
@@ -4766,6 +4769,9 @@ void scst_abort_cmd(struct scst_cmd *cmd, struct scst_mgmt_cmd *mcmd,
/* We don't need to wait for this cmd */
mempool_free(mstb, scst_mgmt_stub_mempool);
}
if (cmd->tgtt->on_abort_cmd)
cmd->tgtt->on_abort_cmd(cmd);
}
unlock:
@@ -5495,7 +5501,10 @@ static int scst_abort_task(struct scst_mgmt_cmd *mcmd)
cmd->tgt_sn, (long long unsigned int)mcmd->tag);
mcmd->status = SCST_MGMT_STATUS_REJECTED;
} else {
spin_lock_irq(&cmd->sess->sess_list_lock);
scst_abort_cmd(cmd, mcmd, 0, 1);
spin_unlock_irq(&cmd->sess->sess_list_lock);
scst_unblock_aborted_cmds(0);
}