From 72ca9f805593d69c054e18ef925a0a828df30f02 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Tue, 10 Nov 2015 03:29:11 +0000 Subject: [PATCH] Fixing race leading to lost external unblock requests git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6650 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_lib.c | 35 +++++++++++++++++++++++------------ scst/src/scst_priv.h | 6 ++++-- scst/src/scst_sysfs.c | 6 +++--- scst/src/scst_tg.c | 14 ++++---------- 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 849bdadcf..e87c90b39 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -7055,7 +7055,7 @@ void scst_stpg_del_unblock_next(struct scst_cmd *cmd) EXTRACHECKS_BUG_ON(!cmd->cmd_on_global_stpg_list); - TRACE_DBG("STPG cmd %p: unblocking next", cmd); + TRACE_DBG("STPG cmd %p done: unblocking next", cmd); list_del(&cmd->global_stpg_list_entry); cmd->cmd_on_global_stpg_list = 0; @@ -14171,15 +14171,15 @@ static void scst_sync_ext_blocking_done(struct scst_device *dev, return; } -int scst_ext_block_dev(struct scst_device *dev, bool sync, - ext_blocker_done_fn_t done_fn, const uint8_t *priv, int priv_len) +int scst_ext_block_dev(struct scst_device *dev, ext_blocker_done_fn_t done_fn, + const uint8_t *priv, int priv_len, int flags) { int res; struct scst_ext_blocker *b; TRACE_ENTRY(); - if (sync) + if (flags & SCST_EXT_BLOCK_SYNC) priv_len = sizeof(void *); b = kzalloc(sizeof(*b) + priv_len, GFP_KERNEL); @@ -14190,8 +14190,8 @@ int scst_ext_block_dev(struct scst_device *dev, bool sync, goto out; } - TRACE_MGMT_DBG("New (%d) %s ext blocker %p for dev %s", dev->ext_blocks_cnt+1, - sync ? "sync" : "async", b, dev->virt_name); + TRACE_MGMT_DBG("New %d ext blocker %p for dev %s (flags %x)", + dev->ext_blocks_cnt+1, b, dev->virt_name, flags); spin_lock_bh(&dev->dev_lock); @@ -14207,10 +14207,15 @@ int scst_ext_block_dev(struct scst_device *dev, bool sync, } else scst_block_dev(dev); + if (flags & SCST_EXT_BLOCK_STPG) { + WARN_ON(dev->stpg_ext_blocked); + dev->stpg_ext_blocked = 1; + } + dev->ext_blocks_cnt++; TRACE_DBG("ext_blocks_cnt %d", dev->ext_blocks_cnt); - if (sync && (dev->on_dev_cmd_count == 0)) { + if ((flags & SCST_EXT_BLOCK_SYNC) && (dev->on_dev_cmd_count == 0)) { TRACE_DBG("No commands to wait for sync blocking (dev %s)", dev->virt_name); spin_unlock_bh(&dev->dev_lock); @@ -14220,7 +14225,7 @@ int scst_ext_block_dev(struct scst_device *dev, bool sync, list_add_tail(&b->ext_blockers_list_entry, &dev->ext_blockers_list); dev->ext_blocking_pending = 1; - if (sync) { + if (flags & SCST_EXT_BLOCK_SYNC) { DECLARE_WAIT_QUEUE_HEAD_ONSTACK(w); b->ext_blocker_done_fn = scst_sync_ext_blocking_done; @@ -14254,7 +14259,7 @@ out: return res; out_free_success: - sBUG_ON(!sync); + sBUG_ON(!(flags & SCST_EXT_BLOCK_SYNC)); kfree(b); goto out_success; } @@ -14271,8 +14276,14 @@ void scst_ext_unblock_dev(struct scst_device *dev, bool stpg) } if ((dev->ext_blocks_cnt == 1) && dev->stpg_ext_blocked && !stpg) { - TRACE_DBG("Can not unblock internal STPG ext block (dev %s)", - dev->virt_name); + /* + * User space is sending too many unblock calls during + * STPG processing + */ + TRACE_MGMT_DBG("Can not unblock internal STPG ext block " + "(dev %s, ext_blocks_cnt %d, stpg_ext_blocked %d, stpg %d)", + dev->virt_name, dev->ext_blocks_cnt, + dev->stpg_ext_blocked, stpg); goto out_unlock; } @@ -14287,7 +14298,7 @@ void scst_ext_unblock_dev(struct scst_device *dev, bool stpg) spin_unlock_bh(&dev->dev_lock); TRACE_DBG("Ext unblock (dev %s): still pending...", dev->virt_name); - rc = scst_ext_block_dev(dev, true, NULL, NULL, 0); + rc = scst_ext_block_dev(dev, NULL, NULL, 0, SCST_EXT_BLOCK_SYNC); if (rc != 0) { /* Oops, have to poll */ PRINT_WARNING("scst_ext_block_dev(dev %s) failed, " diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 0fe533b40..5d9234364 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -718,8 +718,10 @@ bool __scst_check_blocked_dev(struct scst_cmd *cmd); void __scst_check_unblock_dev(struct scst_cmd *cmd); void scst_check_unblock_dev(struct scst_cmd *cmd); -int scst_ext_block_dev(struct scst_device *dev, bool sync, - ext_blocker_done_fn_t done_fn, const uint8_t *priv, int priv_len); +#define SCST_EXT_BLOCK_SYNC 1 +#define SCST_EXT_BLOCK_STPG 2 +int scst_ext_block_dev(struct scst_device *dev, ext_blocker_done_fn_t done_fn, + const uint8_t *priv, int priv_len, int flags); void scst_ext_unblock_dev(struct scst_device *dev, bool stpg); void __scst_ext_blocking_done(struct scst_device *dev); void scst_ext_blocking_done(struct scst_device *dev); diff --git a/scst/src/scst_sysfs.c b/scst/src/scst_sysfs.c index 7e12e175f..9b7336dd8 100644 --- a/scst/src/scst_sysfs.c +++ b/scst/src/scst_sysfs.c @@ -3560,10 +3560,10 @@ static ssize_t scst_dev_block_store(struct kobject *kobj, "data_len %d)", dev->virt_name, sync, data_start, data_len); if (sync) - res = scst_ext_block_dev(dev, true, NULL, NULL, 0); + res = scst_ext_block_dev(dev, NULL, NULL, 0, SCST_EXT_BLOCK_SYNC); else - res = scst_ext_block_dev(dev, false, scst_sysfs_ext_blocking_done, - data_start, data_len); + res = scst_ext_block_dev(dev, scst_sysfs_ext_blocking_done, + data_start, data_len, 0); if (res != 0) goto out; diff --git a/scst/src/scst_tg.c b/scst/src/scst_tg.c index e9f818095..00f6b0d2e 100644 --- a/scst/src/scst_tg.c +++ b/scst/src/scst_tg.c @@ -1852,17 +1852,11 @@ int scst_tg_set_group_info(struct scst_cmd *cmd) atomic_inc(&wait->stpg_wait_left); - spin_lock_bh(&dev->dev_lock); - WARN_ON(dgd->dev->stpg_ext_blocked); - dgd->dev->stpg_ext_blocked = 1; - spin_unlock_bh(&dev->dev_lock); - - rc = scst_ext_block_dev(dgd->dev, false, - scst_stpg_ext_blocking_done, (uint8_t *)&wait, - sizeof(wait)); + rc = scst_ext_block_dev(dgd->dev, scst_stpg_ext_blocking_done, + (uint8_t *)&wait, sizeof(wait), SCST_EXT_BLOCK_STPG); if (rc != 0) { - TRACE_DBG("scst_ext_block_dev() returned %d, " - "stepping back (cmd %p)", rc, cmd); + TRACE_DBG("scst_ext_block_dev() failed " + "with %d, reverting (cmd %p)", rc, cmd); wait->status = rc; wait->dg = dg; atomic_dec(&wait->stpg_wait_left);