From: Christoph Hellwig <hch@infradead.org>
To: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 23/39] scsi: fix the {host,target,device}_blocked counter mess
Date: Mon, 17 Mar 2014 06:27:56 -0700 [thread overview]
Message-ID: <20140317133134.819366684@bombadil.infradead.org> (raw)
In-Reply-To: 20140317132733.789623766@bombadil.infradead.org
[-- Attachment #1: 0023-scsi-fix-the-host-target-device-_blocked-counter-mes.patch --]
[-- Type: text/plain, Size: 10205 bytes --]
Seems like these counters are missing any sort of synchronization for
updates, as a over 10 year old comment from me noted. Fix this by
using atomic counters, and while we're at it also make sure they are
in the same cacheline as the _busy counters and not needlessly stored
to in every I/O completion.
With the new model the _busy counters can temporarily go negative,
so all the readers are updated to check for > 0 values. Longer
term every successful I/O completion will reset the counters to zero,
so the temporarily negative values will not cause any harm.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/scsi.c | 21 ++++++------
drivers/scsi/scsi_lib.c | 82 +++++++++++++++++++++-----------------------
drivers/scsi/scsi_sysfs.c | 10 +++++-
include/scsi/scsi_device.h | 7 ++--
include/scsi/scsi_host.h | 7 ++--
5 files changed, 64 insertions(+), 63 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index a0b5e6f..0e8a077 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -752,17 +752,16 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
scsi_device_unbusy(sdev);
- /*
- * Clear the flags which say that the device/host is no longer
- * capable of accepting new commands. These are set in scsi_queue.c
- * for both the queue full condition on a device, and for a
- * host full condition on the host.
- *
- * XXX(hch): What about locking?
- */
- shost->host_blocked = 0;
- starget->target_blocked = 0;
- sdev->device_blocked = 0;
+ /*
+ * Clear the flags which say that the device/target/host is no longer
+ * capable of accepting new commands.
+ */
+ if (atomic_read(&shost->host_blocked))
+ atomic_set(&shost->host_blocked, 0);
+ if (atomic_read(&starget->target_blocked))
+ atomic_set(&starget->target_blocked, 0);
+ if (atomic_read(&sdev->device_blocked))
+ atomic_set(&sdev->device_blocked, 0);
/*
* If we have valid sense information, then some kind of recovery
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6281595..a23e8c3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -113,14 +113,16 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
*/
switch (reason) {
case SCSI_MLQUEUE_HOST_BUSY:
- host->host_blocked = host->max_host_blocked;
+ atomic_set(&host->host_blocked, host->max_host_blocked);
break;
case SCSI_MLQUEUE_DEVICE_BUSY:
case SCSI_MLQUEUE_EH_RETRY:
- device->device_blocked = device->max_device_blocked;
+ atomic_set(&device->device_blocked,
+ device->max_device_blocked);
break;
case SCSI_MLQUEUE_TARGET_BUSY:
- starget->target_blocked = starget->max_target_blocked;
+ atomic_set(&starget->target_blocked,
+ starget->max_target_blocked);
break;
}
@@ -340,30 +342,39 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
spin_unlock_irqrestore(shost->host_lock, flags);
}
-static inline int scsi_device_is_busy(struct scsi_device *sdev)
+static inline bool scsi_device_is_busy(struct scsi_device *sdev)
{
if (atomic_read(&sdev->device_busy) >= sdev->queue_depth)
- return 1;
- if (sdev->device_blocked)
- return 1;
+ return true;
+ if (atomic_read(&sdev->device_blocked) > 0)
+ return true;
return 0;
}
-static inline int scsi_target_is_busy(struct scsi_target *starget)
+static inline bool scsi_target_is_busy(struct scsi_target *starget)
{
- return ((starget->can_queue > 0 &&
- atomic_read(&starget->target_busy) >= starget->can_queue) ||
- starget->target_blocked);
+ if (starget->can_queue > 0) {
+ if (atomic_read(&starget->target_busy) >= starget->can_queue)
+ return true;
+ if (atomic_read(&starget->target_blocked) > 0)
+ return true;
+ }
+
+ return false;
}
-static inline int scsi_host_is_busy(struct Scsi_Host *shost)
+static inline bool scsi_host_is_busy(struct Scsi_Host *shost)
{
- if ((shost->can_queue > 0 &&
- atomic_read(&shost->host_busy) >= shost->can_queue) ||
- shost->host_blocked || shost->host_self_blocked)
- return 1;
+ if (shost->can_queue > 0) {
+ if (atomic_read(&shost->host_busy) >= shost->can_queue)
+ return true;
+ if (atomic_read(&shost->host_blocked) > 0)
+ return true;
+ if (shost->host_self_blocked)
+ return true;
+ }
- return 0;
+ return false;
}
static void scsi_starved_list_run(struct Scsi_Host *shost)
@@ -1256,11 +1267,8 @@ static inline int scsi_dev_queue_ready(struct request_queue *q,
unsigned int busy;
busy = atomic_inc_return(&sdev->device_busy) - 1;
- if (busy == 0 && sdev->device_blocked) {
- /*
- * unblock after device_blocked iterates to zero
- */
- if (--sdev->device_blocked != 0) {
+ if (busy == 0 && atomic_read(&sdev->device_blocked) > 0) {
+ if (atomic_dec_return(&sdev->device_blocked) > 0) {
blk_delay_queue(q, SCSI_QUEUE_DELAY);
goto out_dec;
}
@@ -1270,7 +1278,7 @@ static inline int scsi_dev_queue_ready(struct request_queue *q,
if (busy >= sdev->queue_depth)
goto out_dec;
- if (sdev->device_blocked)
+ if (atomic_read(&sdev->device_blocked) > 0)
goto out_dec;
return 1;
@@ -1301,16 +1309,9 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
}
busy = atomic_inc_return(&starget->target_busy) - 1;
- if (busy == 0 && starget->target_blocked) {
- /*
- * unblock after target_blocked iterates to zero
- */
- spin_lock_irq(shost->host_lock);
- if (--starget->target_blocked != 0) {
- spin_unlock_irq(shost->host_lock);
+ if (busy == 0 && atomic_read(&starget->target_blocked) > 0) {
+ if (atomic_dec_return(&starget->target_blocked) > 0)
goto out_dec;
- }
- spin_unlock_irq(shost->host_lock);
SCSI_LOG_MLQUEUE(3, starget_printk(KERN_INFO, starget,
"unblocking target at zero depth\n"));
@@ -1318,7 +1319,7 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
if (starget->can_queue > 0 && busy >= starget->can_queue)
goto starved;
- if (starget->target_blocked)
+ if (atomic_read(&starget->target_blocked) > 0)
goto starved;
return 1;
@@ -1347,16 +1348,9 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
return 0;
busy = atomic_inc_return(&shost->host_busy) - 1;
- if (busy == 0 && shost->host_blocked) {
- /*
- * unblock after host_blocked iterates to zero
- */
- spin_lock_irq(shost->host_lock);
- if (--shost->host_blocked != 0) {
- spin_unlock_irq(shost->host_lock);
+ if (busy == 0 && atomic_read(&shost->host_blocked) > 0) {
+ if (atomic_dec_return(&shost->host_blocked) > 0)
goto out_dec;
- }
- spin_unlock_irq(shost->host_lock);
SCSI_LOG_MLQUEUE(3,
printk("scsi%d unblocking host at zero depth\n",
@@ -1365,7 +1359,9 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
if (shost->can_queue > 0 && busy >= shost->can_queue)
goto starved;
- if (shost->host_blocked || shost->host_self_blocked)
+ if (atomic_read(&shost->host_blocked) > 0)
+ goto starved;
+ if (shost->host_self_blocked)
goto starved;
/* We're OK to process the command, so we can't be starved */
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index c59e146..707a10c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -585,7 +585,6 @@ static int scsi_sdev_check_buf_bit(const char *buf)
/*
* Create the actual show/store functions and data structures.
*/
-sdev_rd_attr (device_blocked, "%d\n");
sdev_rd_attr (queue_depth, "%d\n");
sdev_rd_attr (type, "%d\n");
sdev_rd_attr (scsi_level, "%d\n");
@@ -602,6 +601,15 @@ sdev_show_device_busy(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR(device_busy, S_IRUGO, sdev_show_device_busy, NULL);
+static ssize_t
+sdev_show_device_blocked(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ return snprintf(buf, 20, "%d\n", atomic_read(&sdev->device_blocked));
+}
+static DEVICE_ATTR(device_blocked, S_IRUGO, sdev_show_device_blocked, NULL);
+
/*
* TODO: can we make these symlinks to the block layer ones?
*/
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 7bb39e4..d72c842 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -82,6 +82,8 @@ struct scsi_device {
struct list_head same_target_siblings; /* just the devices sharing same target id */
atomic_t device_busy; /* commands actually active on LLDD */
+ atomic_t device_blocked; /* Device returned QUEUE_FULL. */
+
spinlock_t list_lock;
struct list_head cmd_list; /* queue of in use SCSI Command structures */
struct list_head starved_entry;
@@ -173,8 +175,6 @@ struct scsi_device {
struct list_head event_list; /* asserted events */
struct work_struct event_work;
- unsigned int device_blocked; /* Device returned QUEUE_FULL. */
-
unsigned int max_device_blocked; /* what device_blocked counts down from */
#define SCSI_DEFAULT_DEVICE_BLOCKED 3
@@ -272,12 +272,13 @@ struct scsi_target {
* the same target will also. */
/* commands actually active on LLD. */
atomic_t target_busy;
+ atomic_t target_blocked;
+
/*
* LLDs should set this in the slave_alloc host template callout.
* If set to zero then there is not limit.
*/
unsigned int can_queue;
- unsigned int target_blocked;
unsigned int max_target_blocked;
#define SCSI_DEFAULT_TARGET_BLOCKED 3
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 590e1af..c4e4875 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -604,6 +604,8 @@ struct Scsi_Host {
struct blk_queue_tag *bqt;
atomic_t host_busy; /* commands actually active on low-level */
+ atomic_t host_blocked;
+
unsigned int host_failed; /* commands that failed.
protected by host_lock */
unsigned int host_eh_scheduled; /* EH scheduled without command */
@@ -703,11 +705,6 @@ struct Scsi_Host {
struct workqueue_struct *tmf_work_q;
/*
- * Host has rejected a command because it was busy.
- */
- unsigned int host_blocked;
-
- /*
* Value host_blocked counts down from
*/
unsigned int max_host_blocked;
--
1.7.10.4
next prev parent reply other threads:[~2014-03-17 13:41 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
2014-03-17 13:27 ` [PATCH 01/39] block: fix q->flush_rq NULL pointer crash on dm-mpath flush Christoph Hellwig
2014-03-17 13:27 ` [PATCH 02/39] block: change flush sequence list addition back to front add Christoph Hellwig
2014-03-17 13:27 ` [PATCH 03/39] blk-mq: fix blk_mq_end_io_partial Christoph Hellwig
2014-03-17 13:27 ` [PATCH 04/39] blk-mq: initialize resid_len Christoph Hellwig
2014-03-17 13:27 ` [PATCH 05/39] blk-mq: replace blk_mq_init_commands with a ->init_request method Christoph Hellwig
2014-03-17 13:27 ` [PATCH 06/39] blk-mq: add a exit_request method Christoph Hellwig
2014-03-17 13:27 ` [PATCH 07/39] scsi: avoid useless free_list lock roundtrips Christoph Hellwig
2014-03-17 13:27 ` [PATCH 08/39] scsi: avoid taking host_lock in scsi_run_queue unless nessecary Christoph Hellwig
2014-03-17 13:27 ` [PATCH 09/39] scsi: do not manipulate device reference counts in scsi_get/put_command Christoph Hellwig
2014-03-17 13:27 ` [PATCH 10/39] scsi: remove a useless get/put_device pair in scsi_request_fn Christoph Hellwig
2014-03-17 13:27 ` [PATCH 11/39] scsi: remove a useless get/put_device pair in scsi_next_command Christoph Hellwig
2014-03-17 13:27 ` [PATCH 12/39] scsi: remove a useless get/put_device pair in scsi_requeue_command Christoph Hellwig
2014-03-17 13:27 ` [PATCH 13/39] megaraid: simplify internal command handling Christoph Hellwig
2014-03-21 1:12 ` adam radford
2014-03-17 13:27 ` [PATCH 14/39] scsi: simplify command allocation and freeing a bit Christoph Hellwig
2014-03-17 13:27 ` [PATCH 15/39] scsi: add support for per-host cmd pools Christoph Hellwig
2014-03-17 13:27 ` [PATCH 16/39] virtio_scsi: use cmd_size Christoph Hellwig
2014-03-25 15:31 ` Christoph Hellwig
2014-03-25 15:36 ` Paolo Bonzini
2014-03-27 15:28 ` James Bottomley
2014-03-17 13:27 ` [PATCH 17/39] scsi: explicitly release bidi buffers Christoph Hellwig
2014-03-17 13:27 ` [PATCH 18/39] scsi: remove scsi_end_request Christoph Hellwig
2014-03-17 13:27 ` [PATCH 19/39] scsi: push host_lock down into scsi_{host,target}_queue_ready Christoph Hellwig
2014-03-17 13:27 ` [PATCH 20/39] scsi: convert target_busy to an atomic_t Christoph Hellwig
2014-03-17 13:27 ` [PATCH 21/39] scsi: convert host_busy to atomic_t Christoph Hellwig
2014-03-17 13:27 ` [PATCH 22/39] scsi: convert device_busy " Christoph Hellwig
2014-03-17 13:27 ` Christoph Hellwig [this message]
2014-03-17 13:27 ` [PATCH 24/39] blk-mq: add blk_mq_requeue_request Christoph Hellwig
2014-03-17 13:27 ` [PATCH 25/39] blk-mq: add async paramter to blk_mq_start_stopped_hw_queues Christoph Hellwig
2014-03-17 13:27 ` [PATCH 26/39] HACK: support blk_delay_queue for blk-mq Christoph Hellwig
2014-03-17 13:28 ` [PATCH 27/39] blk-mq: export blk_mq_insert_request Christoph Hellwig
2014-03-17 13:28 ` [PATCH 28/39] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
2014-03-26 9:46 ` Christoph Hellwig
2014-03-17 13:28 ` [PATCH 29/39] block: remove unprep_rq_fn Christoph Hellwig
2014-03-17 13:28 ` [PATCH 30/39] scsi: centralize command re-queueing in scsi_dispatch_fn Christoph Hellwig
2014-03-17 13:28 ` [PATCH 31/39] scsi: split __scsi_queue_insert Christoph Hellwig
2014-03-17 13:28 ` [PATCH 32/39] scsi: unwind blk_end_request_all and blk_end_request_err calls Christoph Hellwig
2014-03-17 13:28 ` [PATCH 33/39] scsi: initial blk-mq support Christoph Hellwig
2014-03-17 13:28 ` [PATCH 34/39] scsi: partially stub out scsi_adjust_queue_depth when using blk-mq Christoph Hellwig
2014-03-17 13:28 ` [PATCH 35/39] virtio_scsi: use blk_mq Christoph Hellwig
2014-03-17 13:28 ` [PATCH 36/39] iscsi_tcp: " Christoph Hellwig
2014-03-17 13:28 ` [PATCH 37/39] ata_piix: " Christoph Hellwig
2014-03-17 13:28 ` [PATCH 38/39] blk-mq: make blk_mq_start_stopped_hw_queues run a queue even if not stopped Christoph Hellwig
2014-03-17 13:28 ` [PATCH 39/39] scsi: implement ->init_request and ->exit_request Christoph Hellwig
2014-03-17 13:33 ` [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
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=20140317133134.819366684@bombadil.infradead.org \
--to=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).