All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Jens Axboe <axboe@kernel.dk>,
	James Bottomley <JBottomley@Parallels.com>,
	Nicholas Bellinger <nab@linux-iscsi.org>
Cc: linux-scsi@vger.kernel.org
Subject: [PATCH 16/17] scsi: convert device_busy to atomic_t
Date: Wed, 05 Feb 2014 04:39:46 -0800	[thread overview]
Message-ID: <20140205124021.925960454@bombadil.infradead.org> (raw)
In-Reply-To: 20140205123930.150608699@bombadil.infradead.org

[-- Attachment #1: 0016-scsi-convert-device_busy-to-atomic_t.patch --]
[-- Type: text/plain, Size: 7391 bytes --]

Avoid taking the queue_lock to check the per-device queue limit.  Instead
we do an atomic_inc_return early on to grab our slot in the queue,
and if nessecary decrement it after finishing all checks.

Unlike the host and target busy counters this doesn't allow us to avoid the
queue_lock in the request_fn due to the way the interface works, but it'll
allow us to prepare for using the blk-mq code, which doesn't use the
queue_lock at all, and it at least avoids a queue_lock rountrip in
scsi_device_unbusy, which is still important given how busy the queue_lock
is.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/message/fusion/mptsas.c |    2 +-
 drivers/scsi/scsi_lib.c         |   50 ++++++++++++++++++++++-----------------
 drivers/scsi/scsi_sysfs.c       |   10 +++++++-
 drivers/scsi/sg.c               |    2 +-
 include/scsi/scsi_device.h      |    4 +---
 5 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 00d339c..ea3f253 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -3765,7 +3765,7 @@ mptsas_send_link_status_event(struct fw_event_work *fw_event)
 						printk(MYIOC_s_DEBUG_FMT
 						"SDEV OUTSTANDING CMDS"
 						"%d\n", ioc->name,
-						sdev->device_busy));
+						atomic_read(&sdev->device_busy)));
 				}
 
 			}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8929345..20dfbb5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -291,9 +291,7 @@ void scsi_device_unbusy(struct scsi_device *sdev)
 		spin_unlock_irqrestore(shost->host_lock, flags);
 	}
 
-	spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
-	sdev->device_busy--;
-	spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);
+	atomic_dec(&sdev->device_busy);
 }
 
 /*
@@ -344,9 +342,10 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
 
 static inline int scsi_device_is_busy(struct scsi_device *sdev)
 {
-	if (sdev->device_busy >= sdev->queue_depth || sdev->device_blocked)
+	if (atomic_read(&sdev->device_busy) >= sdev->queue_depth)
+		return 1;
+	if (sdev->device_blocked)
 		return 1;
-
 	return 0;
 }
 
@@ -1268,7 +1267,7 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 		 * queue must be restarted, so we schedule a callback to happen
 		 * shortly.
 		 */
-		if (sdev->device_busy == 0)
+		if (atomic_read(&sdev->device_busy) == 0)
 			blk_delay_queue(q, SCSI_QUEUE_DELAY);
 		break;
 	default:
@@ -1299,26 +1298,32 @@ EXPORT_SYMBOL(scsi_prep_fn);
 static inline int scsi_dev_queue_ready(struct request_queue *q,
 				  struct scsi_device *sdev)
 {
-	if (sdev->device_busy == 0 && sdev->device_blocked) {
+	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) {
-			SCSI_LOG_MLQUEUE(3,
-				   sdev_printk(KERN_INFO, sdev,
-				   "unblocking device at zero depth\n"));
-		} else {
+		if (--sdev->device_blocked != 0) {
 			blk_delay_queue(q, SCSI_QUEUE_DELAY);
-			return 0;
+			goto out_dec;
 		}
+		SCSI_LOG_MLQUEUE(3, sdev_printk(KERN_INFO, sdev,
+				   "unblocking device at zero depth\n"));
 	}
-	if (scsi_device_is_busy(sdev))
-		return 0;
+
+	if (busy >= sdev->queue_depth)
+		goto out_dec;
+	if (sdev->device_blocked)
+		goto out_dec;
 
 	return 1;
+out_dec:
+	atomic_dec(&sdev->device_busy);
+	return 0;
 }
 
-
 /*
  * scsi_target_queue_ready: checks if there we can send commands to target
  * @sdev: scsi device on starget to check.
@@ -1488,7 +1493,7 @@ static void scsi_kill_request(struct request *req, struct request_queue *q)
 	 * bump busy counts.  To bump the counters, we need to dance
 	 * with the locks as normal issue path does.
 	 */
-	sdev->device_busy++;
+	atomic_inc(&sdev->device_busy);
 	atomic_inc(&shost->host_busy);
 	atomic_inc(&starget->target_busy);
 
@@ -1567,7 +1572,7 @@ static void scsi_request_fn(struct request_queue *q)
 		 * accept it.
 		 */
 		req = blk_peek_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
 			break;
 
 		if (unlikely(!scsi_device_online(sdev))) {
@@ -1577,13 +1582,14 @@ static void scsi_request_fn(struct request_queue *q)
 			continue;
 		}
 
+		if (!scsi_dev_queue_ready(q, sdev))
+			break;
 
 		/*
 		 * Remove the request from the request list.
 		 */
 		if (!(blk_queue_tagged(q) && !blk_queue_start_tag(q, req)))
 			blk_start_request(req);
-		sdev->device_busy++;
 
 		spin_unlock_irq(q->queue_lock);
 		cmd = req->special;
@@ -1649,9 +1655,9 @@ static void scsi_request_fn(struct request_queue *q)
 	 */
 	spin_lock_irq(q->queue_lock);
 	blk_requeue_request(q, req);
-	sdev->device_busy--;
+	atomic_dec(&sdev->device_busy);
 out_delay:
-	if (sdev->device_busy == 0)
+	if (atomic_read(&sdev->device_busy) == 0)
 		blk_delay_queue(q, SCSI_QUEUE_DELAY);
 }
 
@@ -2390,7 +2396,7 @@ scsi_device_quiesce(struct scsi_device *sdev)
 		return err;
 
 	scsi_run_queue(sdev->request_queue);
-	while (sdev->device_busy) {
+	while (atomic_read(&sdev->device_busy)) {
 		msleep_interruptible(200);
 		scsi_run_queue(sdev->request_queue);
 	}
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 658ee82..c59e146 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -587,13 +587,21 @@ static int scsi_sdev_check_buf_bit(const char *buf)
  */
 sdev_rd_attr (device_blocked, "%d\n");
 sdev_rd_attr (queue_depth, "%d\n");
-sdev_rd_attr (device_busy, "%d\n");
 sdev_rd_attr (type, "%d\n");
 sdev_rd_attr (scsi_level, "%d\n");
 sdev_rd_attr (vendor, "%.8s\n");
 sdev_rd_attr (model, "%.16s\n");
 sdev_rd_attr (rev, "%.4s\n");
 
+static ssize_t
+sdev_show_device_busy(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_busy));
+}
+static DEVICE_ATTR(device_busy, S_IRUGO, sdev_show_device_busy, NULL);
+
 /*
  * TODO: can we make these symlinks to the block layer ones?
  */
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index df5e961..999ce04 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2489,7 +2489,7 @@ static int sg_proc_seq_show_dev(struct seq_file *s, void *v)
 			      scsidp->id, scsidp->lun, (int) scsidp->type,
 			      1,
 			      (int) scsidp->queue_depth,
-			      (int) scsidp->device_busy,
+			      (int) atomic_read(&scsidp->device_busy),
 			      (int) scsi_device_online(scsidp));
 	else
 		seq_printf(s, "-1\t-1\t-1\t-1\t-1\t-1\t-1\t-1\t-1\n");
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index f85b1a2..7bb39e4 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -81,9 +81,7 @@ struct scsi_device {
 	struct list_head    siblings;   /* list of all devices on this host */
 	struct list_head    same_target_siblings; /* just the devices sharing same target id */
 
-	/* this is now protected by the request_queue->queue_lock */
-	unsigned int device_busy;	/* commands actually active on
-					 * low-level. protected by queue_lock. */
+	atomic_t device_busy;		/* commands actually active on LLDD */
 	spinlock_t list_lock;
 	struct list_head cmd_list;	/* queue of in use SCSI Command structures */
 	struct list_head starved_entry;
-- 
1.7.10.4



  parent reply	other threads:[~2014-02-05 12:40 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-05 12:39 [PATCH 00/17] SCSI data path micro-optimizations Christoph Hellwig
2014-02-05 12:39 ` [PATCH 01/17] scsi: handle command allocation failure in scsi_reset_provider Christoph Hellwig
2014-02-05 12:39 ` [PATCH 02/17] megaraid: simplify internal command handling Christoph Hellwig
2014-02-06 16:40   ` Christoph Hellwig
2014-02-05 12:39 ` [PATCH 03/17] scsi: remove scsi_allocate_command/scsi_free_command Christoph Hellwig
2014-02-05 12:39 ` [PATCH 04/17] scsi: avoid useless free_list lock roundtrips Christoph Hellwig
2014-02-05 23:44   ` James Bottomley
2014-02-06 16:22     ` Christoph Hellwig
2014-02-07  9:05   ` Paolo Bonzini
2014-02-05 12:39 ` [PATCH 05/17] scsi: simplify command allocation and freeing a bit Christoph Hellwig
2014-02-05 23:51   ` James Bottomley
2014-02-06 16:21     ` Christoph Hellwig
2014-02-05 12:39 ` [PATCH 06/17] scsi: add support for per-host cmd pools Christoph Hellwig
2014-02-07  9:13   ` Paolo Bonzini
2014-02-07 12:44     ` Christoph Hellwig
2014-02-07  9:35   ` Mike Christie
2014-02-07 12:46     ` Christoph Hellwig
2014-02-07 21:43       ` Mike Christie
2014-02-10 12:20         ` Christoph Hellwig
2014-02-05 12:39 ` [PATCH 07/17] virtio_scsi: use cmd_size Christoph Hellwig
2014-02-07  9:13   ` Paolo Bonzini
2014-02-05 12:39 ` [PATCH 08/17] scsi: do not manipulate device reference counts in scsi_get/put_command Christoph Hellwig
2014-02-05 12:39 ` [PATCH 09/17] scsi: micro-optimize scsi_request_fn() Christoph Hellwig
2014-02-05 12:39 ` [PATCH 10/17] scsi: micro-optimize scsi_next_command() Christoph Hellwig
2014-02-05 12:39 ` [PATCH 11/17] scsi: micro-optimize scsi_requeue_command() Christoph Hellwig
2014-02-05 12:39 ` [PATCH 12/17] scsi: avoid taking host_lock in scsi_run_queue unless nessecary Christoph Hellwig
2014-02-05 23:54   ` James Bottomley
2014-02-06 16:19     ` Christoph Hellwig
2014-02-05 12:39 ` [PATCH 13/17] scsi: push host_lock down into scsi_{host,target}_queue_ready Christoph Hellwig
2014-02-06 16:56   ` James Bottomley
2014-02-06 17:10     ` Bart Van Assche
2014-02-06 18:41       ` James Bottomley
2014-02-07 10:42         ` Bart Van Assche
2014-02-06 21:58       ` Nicholas A. Bellinger
2014-02-07 10:32         ` Bart Van Assche
2014-02-07 19:30           ` Nicholas A. Bellinger
2014-02-08 11:00             ` Bart Van Assche
2014-02-09  8:26               ` Nicholas A. Bellinger
2014-02-10 12:09                 ` Christoph Hellwig
2014-02-10 19:53                   ` Nicholas A. Bellinger
2014-02-10 11:39     ` Christoph Hellwig
2014-02-10 20:09       ` Jens Axboe
2014-02-17  9:36         ` Bart Van Assche
2014-02-17 22:00           ` Christoph Hellwig
2014-02-26 15:39             ` Bart Van Assche
2014-02-10 21:10       ` James Bottomley
2014-02-05 12:39 ` [PATCH 14/17] scsi: convert target_busy to an atomic_t Christoph Hellwig
2014-02-05 12:39 ` [PATCH 15/17] scsi: convert host_busy to atomic_t Christoph Hellwig
2014-02-05 12:39 ` Christoph Hellwig [this message]
2014-02-05 12:39 ` [PATCH 17/17] scsi: fix the {host,target,device}_blocked counter mess Christoph Hellwig
2014-02-05 23:41 ` [PATCH 00/17] SCSI data path micro-optimizations James Bottomley
2014-02-06 16:29   ` 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=20140205124021.925960454@bombadil.infradead.org \
    --to=hch@infradead.org \
    --cc=JBottomley@Parallels.com \
    --cc=axboe@kernel.dk \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@linux-iscsi.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.