From 1249be6247f0ae409b27a5a9af48db788d01aa96 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Thu, 24 Apr 2014 04:03:51 +0000 Subject: [PATCH] Fix for TARGET RESET race It can happen, when a device added after blocking, so unblocking then will make dev->block_count of the new device negative. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@5468 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst.h | 1 + scst/src/scst_lib.c | 2 +- scst/src/scst_main.c | 4 ++-- scst/src/scst_targ.c | 57 ++++++++++++++++++++++++++++++++++++-------- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index 518e434a5..70ef6b486 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -2311,6 +2311,7 @@ struct scst_mgmt_cmd { unsigned int needs_unblocking:1; unsigned int lun_set:1; /* set, if lun field is valid */ unsigned int cmd_sn_set:1; /* set, if cmd_sn field is valid */ + unsigned int scst_get_called:1; /* set, if scst_get() was called */ /* Set if dev handler's task_mgmt_fn_received was called */ unsigned int task_mgmt_fn_received_called:1; unsigned int mcmd_dropped:1; /* set if mcmd was dropped */ diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 017538c2a..206dab174 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -5884,7 +5884,7 @@ void scst_free_mgmt_cmd(struct scst_mgmt_cmd *mcmd) scst_sess_put(mcmd->sess); - if (mcmd->mcmd_tgt_dev != NULL) + if ((mcmd->mcmd_tgt_dev != NULL) || mcmd->scst_get_called) scst_put(mcmd->cpu_cmd_counter); mempool_free(mcmd, scst_mgmt_mempool); diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index d3e99cbac..d3f24e383 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -960,8 +960,8 @@ int scst_suspend_activity(unsigned long timeout) set_bit(SCST_FLAG_SUSPENDED, &scst_flags); /* * Assignment of SCST_FLAG_SUSPENDING and SCST_FLAG_SUSPENDED must be - * ordered with cpu_cmd_count in scst_get(). Otherwise lockless logic in - * scst_translate_lun() and scst_mgmt_translate_lun() won't work. + * ordered with cpu_cmd_count in scst_get(). Otherwise, lockless logic + * of scst_get() users won't work. */ smp_mb__after_set_bit(); diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 83b4ef0f6..d879354a4 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -4926,21 +4926,16 @@ void scst_cmd_tasklet(long p) } /* - * Returns 0 on success, < 0 if there is no device handler or - * > 0 if SCST_FLAG_SUSPENDED set and SCST_FLAG_SUSPENDING - not. - * No locks, protection is done by the suspended activity. + * Returns 0 on success, or > 0 if SCST_FLAG_SUSPENDED set and + * SCST_FLAG_SUSPENDING - not. No locks, protection is done by the + * suspended activity. */ -static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) +static int scst_get_mgmt(struct scst_mgmt_cmd *mcmd) { - struct scst_tgt_dev *tgt_dev; - struct list_head *head; - int res = -1; + int res = 0; TRACE_ENTRY(); - TRACE_DBG("Finding tgt_dev for mgmt cmd %p (lun %lld)", mcmd, - (long long unsigned int)mcmd->lun); - mcmd->cpu_cmd_counter = scst_get(); if (unlikely(test_bit(SCST_FLAG_SUSPENDED, &scst_flags) && @@ -4951,6 +4946,33 @@ static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) goto out; } +out: + TRACE_EXIT_HRES(res); + return res; +} + +/* + * Returns 0 on success, < 0 if there is no device handler or + * > 0 if SCST_FLAG_SUSPENDED set and SCST_FLAG_SUSPENDING - not. + * No locks, protection is done by the suspended activity. + */ +static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) +{ + struct scst_tgt_dev *tgt_dev; + struct list_head *head; + int res; + + TRACE_ENTRY(); + + TRACE_DBG("Finding tgt_dev for mgmt cmd %p (lun %lld)", mcmd, + (long long unsigned int)mcmd->lun); + + res = scst_get_mgmt(mcmd); + if (unlikely(res != 0)) + goto out; + + res = -1; + head = &mcmd->sess->sess_tgt_dev_list[SESS_TGT_DEV_LIST_HASH_FN(mcmd->lun)]; list_for_each_entry(tgt_dev, head, sess_tgt_dev_list_entry) { if (tgt_dev->lun == mcmd->lun) { @@ -5762,6 +5784,21 @@ static int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd) } case SCST_TARGET_RESET: + /* + * Needed to protect against race, when a device added after + * blocking, so unblocking then will make dev->block_count + * of the new device negative. + */ + rc = scst_get_mgmt(mcmd); + if (rc == 0) { + mcmd->state = SCST_MCMD_STATE_EXEC; + mcmd->scst_get_called = 1; + } else { + EXTRACHECKS_BUG_ON(rc < 0); + res = rc; + } + break; + case SCST_NEXUS_LOSS_SESS: case SCST_ABORT_ALL_TASKS_SESS: case SCST_NEXUS_LOSS: