public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/39] [WIP] scsi multiqueue
@ 2014-03-17 13:27 Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 01/39] block: fix q->flush_rq NULL pointer crash on dm-mpath flush Christoph Hellwig
                   ` (39 more replies)
  0 siblings, 40 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

I'd like to repost the current state of the scsi multiqueue work.  This
version has shown stable under various loads and is getting close to
feature complete.  So far we see it improving the IOPS over the old code
up to 30% and maxing out the current test hardware, so we will have to
test on bigger hardware for more numbers.  More impressive is the CPU
usage reduction of over 200% on multi-lun tests where we can now mostly
avoid the host lock contention.

The big remaining TODO item is to look into the queue limit and tag
allocation in blk-mq to make it work for all existing SCSI drivers.

Feel free to test it with the existing drivers that set use_blk_mq as well
as with any other driver that doesn't use scsi_init_shared_tag_map.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH 01/39] block: fix q->flush_rq NULL pointer crash on dm-mpath flush
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 02/39] block: change flush sequence list addition back to front add Christoph Hellwig
                   ` (38 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Hannes Reinecke, Mike Snitzer, Jens Axboe

[-- Attachment #1: 0001-block-fix-q-flush_rq-NULL-pointer-crash-on-dm-mpath-.patch --]
[-- Type: text/plain, Size: 4033 bytes --]

Commit 1874198 ("blk-mq: rework flush sequencing logic") switched
->flush_rq from being an embedded member of the request_queue structure
to being dynamically allocated in blk_init_queue_node().

Request-based DM multipath doesn't use blk_init_queue_node(), instead it
uses blk_alloc_queue_node() + blk_init_allocated_queue().  Because
commit 1874198 placed the dynamic allocation of ->flush_rq in
blk_init_queue_node() any flush issued to a dm-mpath device would crash
with a NULL pointer, e.g.:

BUG: unable to handle kernel NULL pointer dereference at           (null)
IP: [<ffffffff8125037e>] blk_rq_init+0x1e/0xb0
PGD bb3c7067 PUD bb01d067 PMD 0
Oops: 0002 [#1] SMP
...
CPU: 5 PID: 5028 Comm: dt Tainted: G        W  O 3.14.0-rc3.snitm+ #10
...
task: ffff88032fb270e0 ti: ffff880079564000 task.ti: ffff880079564000
RIP: 0010:[<ffffffff8125037e>]  [<ffffffff8125037e>] blk_rq_init+0x1e/0xb0
RSP: 0018:ffff880079565c98  EFLAGS: 00010046
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000030
RDX: ffff880260c74048 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffff880079565ca8 R08: ffff880260aa1e98 R09: 0000000000000001
R10: ffff88032fa78500 R11: 0000000000000246 R12: 0000000000000000
R13: ffff880260aa1de8 R14: 0000000000000650 R15: 0000000000000000
FS:  00007f8d36a2a700(0000) GS:ffff88033fca0000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000079b36000 CR4: 00000000000007e0
Stack:
 0000000000000000 ffff880260c74048 ffff880079565cd8 ffffffff81257a47
 ffff880260aa1de8 ffff880260c74048 0000000000000001 0000000000000000
 ffff880079565d08 ffffffff81257c2d 0000000000000000 ffff880260aa1de8
Call Trace:
 [<ffffffff81257a47>] blk_flush_complete_seq+0x2d7/0x2e0
 [<ffffffff81257c2d>] blk_insert_flush+0x1dd/0x210
 [<ffffffff8124ec59>] __elv_add_request+0x1f9/0x320
 [<ffffffff81250681>] ? blk_account_io_start+0x111/0x190
 [<ffffffff81253a4b>] blk_queue_bio+0x25b/0x330
 [<ffffffffa0020bf5>] dm_request+0x35/0x40 [dm_mod]
 [<ffffffff812530c0>] generic_make_request+0xc0/0x100
 [<ffffffff81253173>] submit_bio+0x73/0x140
 [<ffffffff811becdd>] submit_bio_wait+0x5d/0x80
 [<ffffffff81257528>] blkdev_issue_flush+0x78/0xa0
 [<ffffffff811c1f6f>] blkdev_fsync+0x3f/0x60
 [<ffffffff811b7fde>] vfs_fsync_range+0x1e/0x20
 [<ffffffff811b7ffc>] vfs_fsync+0x1c/0x20
 [<ffffffff811b81f1>] do_fsync+0x41/0x80
 [<ffffffff8118874e>] ? SyS_lseek+0x7e/0x80
 [<ffffffff811b8260>] SyS_fsync+0x10/0x20
 [<ffffffff8154c2d2>] system_call_fastpath+0x16/0x1b

Fix this by moving the ->flush_rq allocation from blk_init_queue_node()
to blk_init_allocated_queue().  blk_init_queue_node() also calls
blk_init_allocated_queue() so this change is functionality equivalent
for all blk_init_queue_node() callers.

Reported-by: Hannes Reinecke <hare@suse.de>
Reported-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
---
 block/blk-core.c |   17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 853f927..4cd5ffc 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -693,20 +693,11 @@ blk_init_queue_node(request_fn_proc *rfn, spinlock_t *lock, int node_id)
 	if (!uninit_q)
 		return NULL;
 
-	uninit_q->flush_rq = kzalloc(sizeof(struct request), GFP_KERNEL);
-	if (!uninit_q->flush_rq)
-		goto out_cleanup_queue;
-
 	q = blk_init_allocated_queue(uninit_q, rfn, lock);
 	if (!q)
-		goto out_free_flush_rq;
-	return q;
+		blk_cleanup_queue(uninit_q);
 
-out_free_flush_rq:
-	kfree(uninit_q->flush_rq);
-out_cleanup_queue:
-	blk_cleanup_queue(uninit_q);
-	return NULL;
+	return q;
 }
 EXPORT_SYMBOL(blk_init_queue_node);
 
@@ -717,6 +708,10 @@ blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
 	if (!q)
 		return NULL;
 
+	q->flush_rq = kzalloc(sizeof(struct request), GFP_KERNEL);
+	if (!q->flush_rq)
+		return NULL;
+
 	if (blk_init_rl(&q->root_rl, q, GFP_KERNEL))
 		return NULL;
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 02/39] block: change flush sequence list addition back to front add
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 03/39] blk-mq: fix blk_mq_end_io_partial Christoph Hellwig
                   ` (37 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Mike Snitzer, Jens Axboe

[-- Attachment #1: 0002-block-change-flush-sequence-list-addition-back-to-fr.patch --]
[-- Type: text/plain, Size: 1655 bytes --]

Commit 18741986 inadvertently changed the rq flush insertion
from a head to a tail insertion. Fix that back up.

Signed-off-by: Mike Snitzer <msnitzer@redhat.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
---
 block/blk-flush.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index f598f79..43e6b47 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -140,14 +140,17 @@ static void mq_flush_run(struct work_struct *work)
 	blk_mq_insert_request(rq, false, true, false);
 }
 
-static bool blk_flush_queue_rq(struct request *rq)
+static bool blk_flush_queue_rq(struct request *rq, bool add_front)
 {
 	if (rq->q->mq_ops) {
 		INIT_WORK(&rq->mq_flush_work, mq_flush_run);
 		kblockd_schedule_work(rq->q, &rq->mq_flush_work);
 		return false;
 	} else {
-		list_add_tail(&rq->queuelist, &rq->q->queue_head);
+		if (add_front)
+			list_add(&rq->queuelist, &rq->q->queue_head);
+		else
+			list_add_tail(&rq->queuelist, &rq->q->queue_head);
 		return true;
 	}
 }
@@ -193,7 +196,7 @@ static bool blk_flush_complete_seq(struct request *rq, unsigned int seq,
 
 	case REQ_FSEQ_DATA:
 		list_move_tail(&rq->flush.list, &q->flush_data_in_flight);
-		queued = blk_flush_queue_rq(rq);
+		queued = blk_flush_queue_rq(rq, true);
 		break;
 
 	case REQ_FSEQ_DONE:
@@ -326,7 +329,7 @@ static bool blk_kick_flush(struct request_queue *q)
 	q->flush_rq->rq_disk = first_rq->rq_disk;
 	q->flush_rq->end_io = flush_end_io;
 
-	return blk_flush_queue_rq(q->flush_rq);
+	return blk_flush_queue_rq(q->flush_rq, false);
 }
 
 static void flush_data_end_io(struct request *rq, int error)
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 03/39] blk-mq: fix blk_mq_end_io_partial
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 04/39] blk-mq: initialize resid_len Christoph Hellwig
                   ` (36 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0003-blk-mq-fix-blk_mq_end_io_partial.patch --]
[-- Type: text/plain, Size: 629 bytes --]

We need to pass the actual nr_bytes instead of all bytes in the request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 883f720..d81dc8b 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -285,7 +285,7 @@ void blk_mq_free_request(struct request *rq)
 
 bool blk_mq_end_io_partial(struct request *rq, int error, unsigned int nr_bytes)
 {
-	if (blk_update_request(rq, error, blk_rq_bytes(rq)))
+	if (blk_update_request(rq, error, nr_bytes))
 		return true;
 
 	blk_account_io_done(rq);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 04/39] blk-mq: initialize resid_len
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (2 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 03/39] blk-mq: fix blk_mq_end_io_partial Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 05/39] blk-mq: replace blk_mq_init_commands with a ->init_request method Christoph Hellwig
                   ` (35 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0004-blk-mq-initialize-resid_len.patch --]
[-- Type: text/plain, Size: 534 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index d81dc8b..e3284f6 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -350,6 +350,8 @@ static void blk_mq_start_request(struct request *rq, bool last)
 
 	trace_block_rq_issue(q, rq);
 
+	rq->resid_len = blk_rq_bytes(rq);
+
 	/*
 	 * Just mark start time and set the started bit. Due to memory
 	 * ordering, we know we'll see the correct deadline as long as
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 05/39] blk-mq: replace blk_mq_init_commands with a ->init_request method
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (3 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 04/39] blk-mq: initialize resid_len Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 06/39] blk-mq: add a exit_request method Christoph Hellwig
                   ` (34 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0005-blk-mq-replace-blk_mq_init_commands-with-a-init_requ.patch --]
[-- Type: text/plain, Size: 6687 bytes --]

Bedides a simpler and cleared interface this also allows to initialize the
flush_rq properly.  The interface for that is still a bit messy as we don't
have a hw_ctx available for the flush request, but that's something that
should be fixable with a little work once the demand arises.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c             |   65 +++++++++++++++++++++-----------------------
 drivers/block/virtio_blk.c |   22 +++++++--------
 include/linux/blk-mq.h     |    9 +++++-
 3 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index e3284f6..c2ce99b 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -996,33 +996,6 @@ static void blk_mq_hctx_notify(void *data, unsigned long action,
 	blk_mq_put_ctx(ctx);
 }
 
-static void blk_mq_init_hw_commands(struct blk_mq_hw_ctx *hctx,
-				    void (*init)(void *, struct blk_mq_hw_ctx *,
-					struct request *, unsigned int),
-				    void *data)
-{
-	unsigned int i;
-
-	for (i = 0; i < hctx->queue_depth; i++) {
-		struct request *rq = hctx->rqs[i];
-
-		init(data, hctx, rq, i);
-	}
-}
-
-void blk_mq_init_commands(struct request_queue *q,
-			  void (*init)(void *, struct blk_mq_hw_ctx *,
-					struct request *, unsigned int),
-			  void *data)
-{
-	struct blk_mq_hw_ctx *hctx;
-	unsigned int i;
-
-	queue_for_each_hw_ctx(q, hctx, i)
-		blk_mq_init_hw_commands(hctx, init, data);
-}
-EXPORT_SYMBOL(blk_mq_init_commands);
-
 static void blk_mq_free_rq_map(struct blk_mq_hw_ctx *hctx)
 {
 	struct page *page;
@@ -1050,10 +1023,12 @@ static size_t order_to_size(unsigned int order)
 }
 
 static int blk_mq_init_rq_map(struct blk_mq_hw_ctx *hctx,
-			      unsigned int reserved_tags, int node)
+		struct blk_mq_reg *reg, void *driver_data, int node)
 {
+	unsigned int reserved_tags = reg->reserved_tags;
 	unsigned int i, j, entries_per_page, max_order = 4;
 	size_t rq_size, left;
+	int error;
 
 	INIT_LIST_HEAD(&hctx->page_list);
 
@@ -1102,14 +1077,23 @@ static int blk_mq_init_rq_map(struct blk_mq_hw_ctx *hctx,
 		for (j = 0; j < to_do; j++) {
 			hctx->rqs[i] = p;
 			blk_mq_rq_init(hctx, hctx->rqs[i]);
+			if (reg->ops->init_request) {
+				error = reg->ops->init_request(driver_data,
+						hctx, hctx->rqs[i], i);
+				if (error)
+					goto err_rq_map;
+			}
+
 			p += rq_size;
 			i++;
 		}
 	}
 
-	if (i < (reserved_tags + BLK_MQ_TAG_MIN))
+	if (i < (reserved_tags + BLK_MQ_TAG_MIN)) {
+		error = -ENOMEM;
 		goto err_rq_map;
-	else if (i != hctx->queue_depth) {
+	}
+	if (i != hctx->queue_depth) {
 		hctx->queue_depth = i;
 		pr_warn("%s: queue depth set to %u because of low memory\n",
 					__func__, i);
@@ -1117,12 +1101,14 @@ static int blk_mq_init_rq_map(struct blk_mq_hw_ctx *hctx,
 
 	hctx->tags = blk_mq_init_tags(hctx->queue_depth, reserved_tags, node);
 	if (!hctx->tags) {
-err_rq_map:
-		blk_mq_free_rq_map(hctx);
-		return -ENOMEM;
+		error = -ENOMEM;
+		goto err_rq_map;
 	}
 
 	return 0;
+err_rq_map:
+	blk_mq_free_rq_map(hctx);
+	return error;
 }
 
 static int blk_mq_init_hw_queues(struct request_queue *q,
@@ -1155,7 +1141,7 @@ static int blk_mq_init_hw_queues(struct request_queue *q,
 						blk_mq_hctx_notify, hctx);
 		blk_mq_register_cpu_notifier(&hctx->cpu_notifier);
 
-		if (blk_mq_init_rq_map(hctx, reg->reserved_tags, node))
+		if (blk_mq_init_rq_map(hctx, reg, driver_data, node))
 			break;
 
 		/*
@@ -1334,6 +1320,17 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 	if (!q->flush_rq)
 		goto err_hw;
 
+	if (reg->ops->init_request) {
+		blk_rq_init(q, q->flush_rq);
+		if (reg->cmd_size)
+			q->flush_rq->special =
+				blk_mq_rq_to_pdu(q->flush_rq);
+
+		if (reg->ops->init_request(driver_data,
+				NULL, q->flush_rq, -1))
+			goto err_flush_rq;
+	}
+
 	if (blk_mq_init_hw_queues(q, reg, driver_data))
 		goto err_flush_rq;
 
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b1cb3f4..a2e925b 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -474,11 +474,22 @@ static const struct device_attribute dev_attr_cache_type_rw =
 	__ATTR(cache_type, S_IRUGO|S_IWUSR,
 	       virtblk_cache_type_show, virtblk_cache_type_store);
 
+static int virtblk_init_request(void *data, struct blk_mq_hw_ctx *hctx,
+		struct request *rq, unsigned int nr)
+{
+	struct virtio_blk *vblk = data;
+	struct virtblk_req *vbr = rq->special;
+
+	sg_init_table(vbr->sg, vblk->sg_elems);
+	return 0;
+}
+
 static struct blk_mq_ops virtio_mq_ops = {
 	.queue_rq	= virtio_queue_rq,
 	.map_queue	= blk_mq_map_queue,
 	.alloc_hctx	= blk_mq_alloc_single_hw_queue,
 	.free_hctx	= blk_mq_free_single_hw_queue,
+	.init_request	= virtblk_init_request,
 	.complete	= virtblk_request_done,
 };
 
@@ -490,15 +501,6 @@ static struct blk_mq_reg virtio_mq_reg = {
 	.flags		= BLK_MQ_F_SHOULD_MERGE,
 };
 
-static void virtblk_init_vbr(void *data, struct blk_mq_hw_ctx *hctx,
-			     struct request *rq, unsigned int nr)
-{
-	struct virtio_blk *vblk = data;
-	struct virtblk_req *vbr = rq->special;
-
-	sg_init_table(vbr->sg, vblk->sg_elems);
-}
-
 static int virtblk_probe(struct virtio_device *vdev)
 {
 	struct virtio_blk *vblk;
@@ -562,8 +564,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 		goto out_put_disk;
 	}
 
-	blk_mq_init_commands(q, virtblk_init_vbr, vblk);
-
 	q->queuedata = vblk;
 
 	virtblk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 2ff2e8d..cac10e1 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -66,6 +66,8 @@ typedef struct blk_mq_hw_ctx *(alloc_hctx_fn)(struct blk_mq_reg *,unsigned int);
 typedef void (free_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
 typedef int (init_hctx_fn)(struct blk_mq_hw_ctx *, void *, unsigned int);
 typedef void (exit_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
+typedef int (init_request_fn)(void *, struct blk_mq_hw_ctx *,
+		struct request *, unsigned int);
 
 struct blk_mq_ops {
 	/*
@@ -98,6 +100,12 @@ struct blk_mq_ops {
 	 */
 	init_hctx_fn		*init_hctx;
 	exit_hctx_fn		*exit_hctx;
+
+	/*
+	 * Called for every command allocated by the block layer to allow
+	 * the driver to set up driver specific data.
+	 */
+	init_request_fn		*init_request;
 };
 
 enum {
@@ -117,7 +125,6 @@ enum {
 struct request_queue *blk_mq_init_queue(struct blk_mq_reg *, void *);
 int blk_mq_register_disk(struct gendisk *);
 void blk_mq_unregister_disk(struct gendisk *);
-void blk_mq_init_commands(struct request_queue *, void (*init)(void *data, struct blk_mq_hw_ctx *, struct request *, unsigned int), void *data);
 
 void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule);
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 06/39] blk-mq: add a exit_request method
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (4 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 07/39] scsi: avoid useless free_list lock roundtrips Christoph Hellwig
                   ` (33 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0006-blk-mq-add-a-exit_request-method.patch --]
[-- Type: text/plain, Size: 1591 bytes --]

This gives drivers an easy way to free any ressources allocated in
->init_request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c2ce99b..c7e723e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1000,6 +1000,16 @@ static void blk_mq_free_rq_map(struct blk_mq_hw_ctx *hctx)
 {
 	struct page *page;
 
+	if (hctx->rqs && hctx->queue->mq_ops->exit_request) {
+		int i;
+
+		for (i = 0; i < hctx->queue_depth; i++) {
+			if (!hctx->rqs[i])
+				continue;
+			hctx->queue->mq_ops->exit_request(hctx, hctx->rqs[i]);
+		}
+	}
+
 	while (!list_empty(&hctx->page_list)) {
 		page = list_first_entry(&hctx->page_list, struct page, lru);
 		list_del_init(&page->lru);
@@ -1332,7 +1342,7 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 	}
 
 	if (blk_mq_init_hw_queues(q, reg, driver_data))
-		goto err_flush_rq;
+		goto err_flush_rq_init;
 
 	blk_mq_map_swqueue(q);
 
@@ -1342,6 +1352,9 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 
 	return q;
 
+err_flush_rq_init:
+	if (reg->ops->exit_request)
+		reg->ops->exit_request(NULL, q->flush_rq);
 err_flush_rq:
 	kfree(q->flush_rq);
 err_hw:
@@ -1387,6 +1400,9 @@ void blk_mq_free_queue(struct request_queue *q)
 	mutex_lock(&all_q_mutex);
 	list_del_init(&q->all_q_node);
 	mutex_unlock(&all_q_mutex);
+
+	if (q->mq_ops->exit_request)
+		q->mq_ops->exit_request(NULL, q->flush_rq);
 }
 
 /* Basically redo blk_mq_init_queue with queue frozen */
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 07/39] scsi: avoid useless free_list lock roundtrips
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (5 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 06/39] blk-mq: add a exit_request method Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 08/39] scsi: avoid taking host_lock in scsi_run_queue unless nessecary Christoph Hellwig
                   ` (32 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Hannes Reinecke

[-- Attachment #1: 0007-scsi-avoid-useless-free_list-lock-roundtrips.patch --]
[-- Type: text/plain, Size: 1136 bytes --]

Avoid hitting the host-wide free_list lock unless we need to put a command
back onto the freelist.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d8afec8..fb86479 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -320,13 +320,14 @@ void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd,
 {
 	unsigned long flags;
 
-	/* changing locks here, don't need to restore the irq state */
-	spin_lock_irqsave(&shost->free_list_lock, flags);
 	if (unlikely(list_empty(&shost->free_list))) {
-		list_add(&cmd->list, &shost->free_list);
-		cmd = NULL;
+		spin_lock_irqsave(&shost->free_list_lock, flags);
+		if (list_empty(&shost->free_list)) {
+			list_add(&cmd->list, &shost->free_list);
+			cmd = NULL;
+		}
+		spin_unlock_irqrestore(&shost->free_list_lock, flags);
 	}
-	spin_unlock_irqrestore(&shost->free_list_lock, flags);
 
 	if (likely(cmd != NULL))
 		scsi_pool_free_command(shost->cmd_pool, cmd);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 08/39] scsi: avoid taking host_lock in scsi_run_queue unless nessecary
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (6 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 07/39] scsi: avoid useless free_list lock roundtrips Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 09/39] scsi: do not manipulate device reference counts in scsi_get/put_command Christoph Hellwig
                   ` (31 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0008-scsi-avoid-taking-host_lock-in-scsi_run_queue-unless.patch --]
[-- Type: text/plain, Size: 2111 bytes --]

If we don't have starved devices we don't need to take the host lock
to iterate over them.  Also split the function up to be more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |   43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 62ec84b..aa6a581 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -385,29 +385,12 @@ static inline int scsi_host_is_busy(struct Scsi_Host *shost)
 	return 0;
 }
 
-/*
- * Function:	scsi_run_queue()
- *
- * Purpose:	Select a proper request queue to serve next
- *
- * Arguments:	q	- last request's queue
- *
- * Returns:     Nothing
- *
- * Notes:	The previous command was completely finished, start
- *		a new one if possible.
- */
-static void scsi_run_queue(struct request_queue *q)
+static void scsi_starved_list_run(struct Scsi_Host *shost)
 {
-	struct scsi_device *sdev = q->queuedata;
-	struct Scsi_Host *shost;
 	LIST_HEAD(starved_list);
+	struct scsi_device *sdev;
 	unsigned long flags;
 
-	shost = sdev->host;
-	if (scsi_target(sdev)->single_lun)
-		scsi_single_lun_run(sdev);
-
 	spin_lock_irqsave(shost->host_lock, flags);
 	list_splice_init(&shost->starved_list, &starved_list);
 
@@ -459,6 +442,28 @@ static void scsi_run_queue(struct request_queue *q)
 	/* put any unprocessed entries back */
 	list_splice(&starved_list, &shost->starved_list);
 	spin_unlock_irqrestore(shost->host_lock, flags);
+}
+
+/*
+ * Function:   scsi_run_queue()
+ *
+ * Purpose:    Select a proper request queue to serve next
+ *
+ * Arguments:  q       - last request's queue
+ *
+ * Returns:     Nothing
+ *
+ * Notes:      The previous command was completely finished, start
+ *             a new one if possible.
+ */
+static void scsi_run_queue(struct request_queue *q)
+{
+	struct scsi_device *sdev = q->queuedata;
+
+	if (scsi_target(sdev)->single_lun)
+		scsi_single_lun_run(sdev);
+	if (!list_empty(&sdev->host->starved_list))
+		scsi_starved_list_run(sdev->host);
 
 	blk_run_queue(q);
 }
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 09/39] scsi: do not manipulate device reference counts in scsi_get/put_command
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (7 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 10/39] scsi: remove a useless get/put_device pair in scsi_request_fn Christoph Hellwig
                   ` (30 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0009-scsi-do-not-manipulate-device-reference-counts-in-sc.patch --]
[-- Type: text/plain, Size: 6467 bytes --]

Many callers won't need this and we can optimize them away.  In addition
the handling in the __-prefixed variants was inconsistant to start with.

Based on an earlier patch from Bart Van Assche.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi.c         |   37 ++++++++++++-------------------------
 drivers/scsi/scsi_error.c   |    6 ++++++
 drivers/scsi/scsi_lib.c     |   12 +++++++++++-
 drivers/scsi/scsi_tgt_lib.c |    3 ++-
 include/scsi/scsi_cmnd.h    |    3 +--
 5 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index fb86479..2b12983 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -284,27 +284,19 @@ EXPORT_SYMBOL_GPL(__scsi_get_command);
  */
 struct scsi_cmnd *scsi_get_command(struct scsi_device *dev, gfp_t gfp_mask)
 {
-	struct scsi_cmnd *cmd;
+	struct scsi_cmnd *cmd = __scsi_get_command(dev->host, gfp_mask);
+	unsigned long flags;
 
-	/* Bail if we can't get a reference to the device */
-	if (!get_device(&dev->sdev_gendev))
+	if (unlikely(cmd == NULL))
 		return NULL;
 
-	cmd = __scsi_get_command(dev->host, gfp_mask);
-
-	if (likely(cmd != NULL)) {
-		unsigned long flags;
-
-		cmd->device = dev;
-		INIT_LIST_HEAD(&cmd->list);
-		INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
-		spin_lock_irqsave(&dev->list_lock, flags);
-		list_add_tail(&cmd->list, &dev->cmd_list);
-		spin_unlock_irqrestore(&dev->list_lock, flags);
-		cmd->jiffies_at_alloc = jiffies;
-	} else
-		put_device(&dev->sdev_gendev);
-
+	cmd->device = dev;
+	INIT_LIST_HEAD(&cmd->list);
+	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
+	spin_lock_irqsave(&dev->list_lock, flags);
+	list_add_tail(&cmd->list, &dev->cmd_list);
+	spin_unlock_irqrestore(&dev->list_lock, flags);
+	cmd->jiffies_at_alloc = jiffies;
 	return cmd;
 }
 EXPORT_SYMBOL(scsi_get_command);
@@ -313,10 +305,8 @@ EXPORT_SYMBOL(scsi_get_command);
  * __scsi_put_command - Free a struct scsi_cmnd
  * @shost: dev->host
  * @cmd: Command to free
- * @dev: parent scsi device
  */
-void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd,
-			struct device *dev)
+void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 {
 	unsigned long flags;
 
@@ -331,8 +321,6 @@ void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd,
 
 	if (likely(cmd != NULL))
 		scsi_pool_free_command(shost->cmd_pool, cmd);
-
-	put_device(dev);
 }
 EXPORT_SYMBOL(__scsi_put_command);
 
@@ -346,7 +334,6 @@ EXPORT_SYMBOL(__scsi_put_command);
  */
 void scsi_put_command(struct scsi_cmnd *cmd)
 {
-	struct scsi_device *sdev = cmd->device;
 	unsigned long flags;
 
 	/* serious error if the command hasn't come from a device list */
@@ -357,7 +344,7 @@ void scsi_put_command(struct scsi_cmnd *cmd)
 
 	cancel_delayed_work(&cmd->abort_work);
 
-	__scsi_put_command(cmd->device->host, cmd, &sdev->sdev_gendev);
+	__scsi_put_command(cmd->device->host, cmd);
 }
 EXPORT_SYMBOL(scsi_put_command);
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 78b004d..771c16b 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2288,6 +2288,11 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
 	if (scsi_autopm_get_host(shost) < 0)
 		return FAILED;
 
+	if (!get_device(&dev->sdev_gendev)) {
+		rtn = FAILED;
+		goto out_put_autopm_host;
+	}
+
 	scmd = scsi_get_command(dev, GFP_KERNEL);
 	blk_rq_init(NULL, &req);
 	scmd->request = &req;
@@ -2345,6 +2350,7 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
 	scsi_run_host_queues(shost);
 
 	scsi_next_command(scmd);
+out_put_autopm_host:
 	scsi_autopm_put_host(shost);
 	return rtn;
 }
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index aa6a581..f8a77d8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -95,6 +95,7 @@ static void scsi_unprep_request(struct request *req)
 	req->special = NULL;
 
 	scsi_put_command(cmd);
+	put_device(&cmd->device->sdev_gendev);
 }
 
 /**
@@ -529,6 +530,7 @@ void scsi_next_command(struct scsi_cmnd *cmd)
 	get_device(&sdev->sdev_gendev);
 
 	scsi_put_command(cmd);
+	put_device(&sdev->sdev_gendev);
 	scsi_run_queue(q);
 
 	/* ok to remove device now */
@@ -1116,6 +1118,7 @@ err_exit:
 	scsi_release_buffers(cmd);
 	cmd->request->special = NULL;
 	scsi_put_command(cmd);
+	put_device(&cmd->device->sdev_gendev);
 	return error;
 }
 EXPORT_SYMBOL(scsi_init_io);
@@ -1126,9 +1129,15 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
 	struct scsi_cmnd *cmd;
 
 	if (!req->special) {
+		/* Bail if we can't get a reference to the device */
+		if (!get_device(&sdev->sdev_gendev))
+			return NULL;
+
 		cmd = scsi_get_command(sdev, GFP_ATOMIC);
-		if (unlikely(!cmd))
+		if (unlikely(!cmd)) {
+			put_device(&sdev->sdev_gendev);
 			return NULL;
+		}
 		req->special = cmd;
 	} else {
 		cmd = req->special;
@@ -1291,6 +1300,7 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 			struct scsi_cmnd *cmd = req->special;
 			scsi_release_buffers(cmd);
 			scsi_put_command(cmd);
+			put_device(&cmd->device->sdev_gendev);
 			req->special = NULL;
 		}
 		break;
diff --git a/drivers/scsi/scsi_tgt_lib.c b/drivers/scsi/scsi_tgt_lib.c
index 84a1fdf..e51add0 100644
--- a/drivers/scsi/scsi_tgt_lib.c
+++ b/drivers/scsi/scsi_tgt_lib.c
@@ -155,7 +155,8 @@ void scsi_host_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 	__blk_put_request(q, rq);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 
-	__scsi_put_command(shost, cmd, &shost->shost_gendev);
+	__scsi_put_command(shost, cmd);
+	put_device(&shost->shost_gendev);
 }
 EXPORT_SYMBOL_GPL(scsi_host_put_command);
 
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 91558a1..414edf9 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -142,8 +142,7 @@ static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
 extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t);
 extern struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *, gfp_t);
 extern void scsi_put_command(struct scsi_cmnd *);
-extern void __scsi_put_command(struct Scsi_Host *, struct scsi_cmnd *,
-			       struct device *);
+extern void __scsi_put_command(struct Scsi_Host *, struct scsi_cmnd *);
 extern void scsi_finish_command(struct scsi_cmnd *cmd);
 
 extern void *scsi_kmap_atomic_sg(struct scatterlist *sg, int sg_count,
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 10/39] scsi: remove a useless get/put_device pair in scsi_request_fn
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (8 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 11/39] scsi: remove a useless get/put_device pair in scsi_next_command Christoph Hellwig
                   ` (29 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel
  Cc: Bart Van Assche, Tejun Heo, Hannes Reinecke, Mike Christie

[-- Attachment #1: 0010-scsi-remove-a-useless-get-put_device-pair-in-scsi_re.patch --]
[-- Type: text/plain, Size: 2081 bytes --]

SCSI devices may only be removed by calling scsi_remove_device().
That function must invoke blk_cleanup_queue() before the final put
of sdev->sdev_gendev. Since blk_cleanup_queue() waits for the
block queue to drain and then tears it down, scsi_request_fn cannot
be active anymore after blk_cleanup_queue() has returned and hence
the get_device()/put_device() pair in scsi_request_fn is unnecessary.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c |   14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f8a77d8..a6d5525 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1558,16 +1558,14 @@ static void scsi_softirq_done(struct request *rq)
  * Lock status: IO request lock assumed to be held when called.
  */
 static void scsi_request_fn(struct request_queue *q)
+	__releases(q->queue_lock)
+	__acquires(q->queue_lock)
 {
 	struct scsi_device *sdev = q->queuedata;
 	struct Scsi_Host *shost;
 	struct scsi_cmnd *cmd;
 	struct request *req;
 
-	if(!get_device(&sdev->sdev_gendev))
-		/* We must be tearing the block queue down already */
-		return;
-
 	/*
 	 * To start with, we keep looping until the queue is empty, or until
 	 * the host is no longer able to accept any more requests.
@@ -1656,7 +1654,7 @@ static void scsi_request_fn(struct request_queue *q)
 			goto out_delay;
 	}
 
-	goto out;
+	return;
 
  not_ready:
 	spin_unlock_irq(shost->host_lock);
@@ -1675,12 +1673,6 @@ static void scsi_request_fn(struct request_queue *q)
 out_delay:
 	if (sdev->device_busy == 0)
 		blk_delay_queue(q, SCSI_QUEUE_DELAY);
-out:
-	/* must be careful here...if we trigger the ->remove() function
-	 * we cannot be holding the q lock */
-	spin_unlock_irq(q->queue_lock);
-	put_device(&sdev->sdev_gendev);
-	spin_lock_irq(q->queue_lock);
 }
 
 u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 11/39] scsi: remove a useless get/put_device pair in scsi_next_command
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (9 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 12/39] scsi: remove a useless get/put_device pair in scsi_requeue_command Christoph Hellwig
                   ` (28 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Bart Van Assche, Hannes Reinecke

[-- Attachment #1: 0011-scsi-remove-a-useless-get-put_device-pair-in-scsi_ne.patch --]
[-- Type: text/plain, Size: 1005 bytes --]

Eliminate a get_device() / put_device() pair from scsi_next_command().
Both are atomic operations hence removing these slightly improves
performance.

[hch: slight changes due to different context]

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Acked-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a6d5525..6866943 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -526,14 +526,9 @@ void scsi_next_command(struct scsi_cmnd *cmd)
 	struct scsi_device *sdev = cmd->device;
 	struct request_queue *q = sdev->request_queue;
 
-	/* need to hold a reference on the device before we let go of the cmd */
-	get_device(&sdev->sdev_gendev);
-
 	scsi_put_command(cmd);
-	put_device(&sdev->sdev_gendev);
 	scsi_run_queue(q);
 
-	/* ok to remove device now */
 	put_device(&sdev->sdev_gendev);
 }
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 12/39] scsi: remove a useless get/put_device pair in scsi_requeue_command
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (10 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 13/39] megaraid: simplify internal command handling Christoph Hellwig
                   ` (27 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Hannes Reinecke

[-- Attachment #1: 0012-scsi-remove-a-useless-get-put_device-pair-in-scsi_re.patch --]
[-- Type: text/plain, Size: 1907 bytes --]

Avoid a spurious device get/put pair by cleaning up scsi_requeue_command
and folding scsi_unprep_request into it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_lib.c |   35 +++--------------------------------
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6866943..debe30a 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -75,29 +75,6 @@ struct kmem_cache *scsi_sdb_cache;
  */
 #define SCSI_QUEUE_DELAY	3
 
-/*
- * Function:	scsi_unprep_request()
- *
- * Purpose:	Remove all preparation done for a request, including its
- *		associated scsi_cmnd, so that it can be requeued.
- *
- * Arguments:	req	- request to unprepare
- *
- * Lock status:	Assumed that no locks are held upon entry.
- *
- * Returns:	Nothing.
- */
-static void scsi_unprep_request(struct request *req)
-{
-	struct scsi_cmnd *cmd = req->special;
-
-	blk_unprep_request(req);
-	req->special = NULL;
-
-	scsi_put_command(cmd);
-	put_device(&cmd->device->sdev_gendev);
-}
-
 /**
  * __scsi_queue_insert - private queue insertion
  * @cmd: The SCSI command being requeued
@@ -503,16 +480,10 @@ static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
 	struct request *req = cmd->request;
 	unsigned long flags;
 
-	/*
-	 * We need to hold a reference on the device to avoid the queue being
-	 * killed after the unlock and before scsi_run_queue is invoked which
-	 * may happen because scsi_unprep_request() puts the command which
-	 * releases its reference on the device.
-	 */
-	get_device(&sdev->sdev_gendev);
-
 	spin_lock_irqsave(q->queue_lock, flags);
-	scsi_unprep_request(req);
+	blk_unprep_request(req);
+	req->special = NULL;
+	scsi_put_command(cmd);
 	blk_requeue_request(q, req);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 13/39] megaraid: simplify internal command handling
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (11 preceding siblings ...)
  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 ` 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
                   ` (26 subsequent siblings)
  39 siblings, 1 reply; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0013-megaraid-simplify-internal-command-handling.patch --]
[-- Type: text/plain, Size: 9741 bytes --]

We don't use the passed in scsi command for anything, so just add a adapter-
wide internal status to go along with the internal scb that is used unter
int_mtx to pass back the return value and get rid of all the complexities
and abuse of the scsi_cmnd structure.

This gets rid of the only user of scsi_allocate_command/scsi_free_command,
which can now be removed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Cc: Sumit.Saxena@lsi.com
Cc: kashyap.desai@lsi.com
Cc: megaraidlinux@lsi.com
---
 drivers/scsi/megaraid.c  |  120 ++++++++++++----------------------------------
 drivers/scsi/megaraid.h  |    3 +-
 drivers/scsi/scsi.c      |   56 ----------------------
 include/scsi/scsi_cmnd.h |    3 --
 4 files changed, 32 insertions(+), 150 deletions(-)

diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 816db12..8bca30f 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -531,13 +531,6 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
 	int	target = 0;
 	int	ldrv_num = 0;   /* logical drive number */
 
-
-	/*
-	 * filter the internal and ioctl commands
-	 */
-	if((cmd->cmnd[0] == MEGA_INTERNAL_CMD))
-		return (scb_t *)cmd->host_scribble;
-
 	/*
 	 * We know what channels our logical drives are on - mega_find_card()
 	 */
@@ -1439,19 +1432,22 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status)
 
 		cmdid = completed[i];
 
-		if( cmdid == CMDID_INT_CMDS ) { /* internal command */
+		/*
+		 * Only free SCBs for the commands coming down from the
+		 * mid-layer, not for which were issued internally
+		 *
+		 * For internal command, restore the status returned by the
+		 * firmware so that user can interpret it.
+		 */
+		if (cmdid == CMDID_INT_CMDS) {
 			scb = &adapter->int_scb;
-			cmd = scb->cmd;
-			mbox = (mbox_t *)scb->raw_mbox;
 
-			/*
-			 * Internal command interface do not fire the extended
-			 * passthru or 64-bit passthru
-			 */
-			pthru = scb->pthru;
+			list_del_init(&scb->list);
+			scb->state = SCB_FREE;
 
-		}
-		else {
+			adapter->int_status = status;
+			complete(&adapter->int_waitq);
+		} else {
 			scb = &adapter->scb_list[cmdid];
 
 			/*
@@ -1640,25 +1636,7 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status)
 				cmd->result |= (DID_BAD_TARGET << 16)|status;
 		}
 
-		/*
-		 * Only free SCBs for the commands coming down from the
-		 * mid-layer, not for which were issued internally
-		 *
-		 * For internal command, restore the status returned by the
-		 * firmware so that user can interpret it.
-		 */
-		if( cmdid == CMDID_INT_CMDS ) { /* internal command */
-			cmd->result = status;
-
-			/*
-			 * Remove the internal command from the pending list
-			 */
-			list_del_init(&scb->list);
-			scb->state = SCB_FREE;
-		}
-		else {
-			mega_free_scb(adapter, scb);
-		}
+		mega_free_scb(adapter, scb);
 
 		/* Add Scsi_Command to end of completed queue */
 		list_add_tail(SCSI_LIST(cmd), &adapter->completed_list);
@@ -4133,23 +4111,15 @@ mega_internal_dev_inquiry(adapter_t *adapter, u8 ch, u8 tgt,
  * The last argument is the address of the passthru structure if the command
  * to be fired is a passthru command
  *
- * lockscope specifies whether the caller has already acquired the lock. Of
- * course, the caller must know which lock we are talking about.
- *
  * Note: parameter 'pthru' is null for non-passthru commands.
  */
 static int
 mega_internal_command(adapter_t *adapter, megacmd_t *mc, mega_passthru *pthru)
 {
-	Scsi_Cmnd	*scmd;
-	struct	scsi_device *sdev;
+	unsigned long flags;
 	scb_t	*scb;
 	int	rval;
 
-	scmd = scsi_allocate_command(GFP_KERNEL);
-	if (!scmd)
-		return -ENOMEM;
-
 	/*
 	 * The internal commands share one command id and hence are
 	 * serialized. This is so because we want to reserve maximum number of
@@ -4160,73 +4130,45 @@ mega_internal_command(adapter_t *adapter, megacmd_t *mc, mega_passthru *pthru)
 	scb = &adapter->int_scb;
 	memset(scb, 0, sizeof(scb_t));
 
-	sdev = kzalloc(sizeof(struct scsi_device), GFP_KERNEL);
-	scmd->device = sdev;
-
-	memset(adapter->int_cdb, 0, sizeof(adapter->int_cdb));
-	scmd->cmnd = adapter->int_cdb;
-	scmd->device->host = adapter->host;
-	scmd->host_scribble = (void *)scb;
-	scmd->cmnd[0] = MEGA_INTERNAL_CMD;
-
-	scb->state |= SCB_ACTIVE;
-	scb->cmd = scmd;
+	scb->idx = CMDID_INT_CMDS;
+	scb->state |= SCB_ACTIVE | SCB_PENDQ;
 
 	memcpy(scb->raw_mbox, mc, sizeof(megacmd_t));
 
 	/*
 	 * Is it a passthru command
 	 */
-	if( mc->cmd == MEGA_MBOXCMD_PASSTHRU ) {
-
+	if (mc->cmd == MEGA_MBOXCMD_PASSTHRU)
 		scb->pthru = pthru;
-	}
-
-	scb->idx = CMDID_INT_CMDS;
 
-	megaraid_queue_lck(scmd, mega_internal_done);
+	spin_lock_irqsave(&adapter->lock, flags);
+	list_add_tail(&scb->list, &adapter->pending_list);
+	/*
+	 * Check if the HBA is in quiescent state, e.g., during a
+	 * delete logical drive opertion. If it is, don't run
+	 * the pending_list.
+	 */
+	if (atomic_read(&adapter->quiescent) == 0)
+		mega_runpendq(adapter);
+	spin_unlock_irqrestore(&adapter->lock, flags);
 
 	wait_for_completion(&adapter->int_waitq);
 
-	rval = scmd->result;
-	mc->status = scmd->result;
-	kfree(sdev);
+	mc->status = rval = adapter->int_status;
 
 	/*
 	 * Print a debug message for all failed commands. Applications can use
 	 * this information.
 	 */
-	if( scmd->result && trace_level ) {
+	if( rval && trace_level ) {
 		printk("megaraid: cmd [%x, %x, %x] status:[%x]\n",
-			mc->cmd, mc->opcode, mc->subopcode, scmd->result);
+			mc->cmd, mc->opcode, mc->subopcode, rval);
 	}
 
 	mutex_unlock(&adapter->int_mtx);
-
-	scsi_free_command(GFP_KERNEL, scmd);
-
 	return rval;
 }
 
-
-/**
- * mega_internal_done()
- * @scmd - internal scsi command
- *
- * Callback routine for internal commands.
- */
-static void
-mega_internal_done(Scsi_Cmnd *scmd)
-{
-	adapter_t	*adapter;
-
-	adapter = (adapter_t *)scmd->device->host->hostdata;
-
-	complete(&adapter->int_waitq);
-
-}
-
-
 static struct scsi_host_template megaraid_template = {
 	.module				= THIS_MODULE,
 	.name				= "MegaRAID",
diff --git a/drivers/scsi/megaraid.h b/drivers/scsi/megaraid.h
index 4d0ce4e..8f2e026 100644
--- a/drivers/scsi/megaraid.h
+++ b/drivers/scsi/megaraid.h
@@ -853,10 +853,10 @@ typedef struct {
 
 	u8	sglen;	/* f/w supported scatter-gather list length */
 
-	unsigned char int_cdb[MAX_COMMAND_SIZE];
 	scb_t			int_scb;
 	struct mutex		int_mtx;	/* To synchronize the internal
 						commands */
+	int			int_status; 	/* status of internal cmd */
 	struct completion	int_waitq;	/* wait queue for internal
 						 cmds */
 
@@ -1004,7 +1004,6 @@ static int mega_del_logdrv(adapter_t *, int);
 static int mega_do_del_logdrv(adapter_t *, int);
 static void mega_get_max_sgl(adapter_t *);
 static int mega_internal_command(adapter_t *, megacmd_t *, mega_passthru *);
-static void mega_internal_done(Scsi_Cmnd *);
 static int mega_support_cluster(adapter_t *);
 #endif
 
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2b12983..8b2bc06 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -403,62 +403,6 @@ static void scsi_put_host_cmd_pool(gfp_t gfp_mask)
 }
 
 /**
- * scsi_allocate_command - get a fully allocated SCSI command
- * @gfp_mask:	allocation mask
- *
- * This function is for use outside of the normal host based pools.
- * It allocates the relevant command and takes an additional reference
- * on the pool it used.  This function *must* be paired with
- * scsi_free_command which also has the identical mask, otherwise the
- * free pool counts will eventually go wrong and you'll trigger a bug.
- *
- * This function should *only* be used by drivers that need a static
- * command allocation at start of day for internal functions.
- */
-struct scsi_cmnd *scsi_allocate_command(gfp_t gfp_mask)
-{
-	struct scsi_host_cmd_pool *pool = scsi_get_host_cmd_pool(gfp_mask);
-
-	if (!pool)
-		return NULL;
-
-	return scsi_pool_alloc_command(pool, gfp_mask);
-}
-EXPORT_SYMBOL(scsi_allocate_command);
-
-/**
- * scsi_free_command - free a command allocated by scsi_allocate_command
- * @gfp_mask:	mask used in the original allocation
- * @cmd:	command to free
- *
- * Note: using the original allocation mask is vital because that's
- * what determines which command pool we use to free the command.  Any
- * mismatch will cause the system to BUG eventually.
- */
-void scsi_free_command(gfp_t gfp_mask, struct scsi_cmnd *cmd)
-{
-	struct scsi_host_cmd_pool *pool = scsi_get_host_cmd_pool(gfp_mask);
-
-	/*
-	 * this could trigger if the mask to scsi_allocate_command
-	 * doesn't match this mask.  Otherwise we're guaranteed that this
-	 * succeeds because scsi_allocate_command must have taken a reference
-	 * on the pool
-	 */
-	BUG_ON(!pool);
-
-	scsi_pool_free_command(pool, cmd);
-	/*
-	 * scsi_put_host_cmd_pool is called twice; once to release the
-	 * reference we took above, and once to release the reference
-	 * originally taken by scsi_allocate_command
-	 */
-	scsi_put_host_cmd_pool(gfp_mask);
-	scsi_put_host_cmd_pool(gfp_mask);
-}
-EXPORT_SYMBOL(scsi_free_command);
-
-/**
  * scsi_setup_command_freelist - Setup the command freelist for a scsi host.
  * @shost: host to allocate the freelist for.
  *
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 414edf9..dd7c998 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -155,9 +155,6 @@ extern void scsi_release_buffers(struct scsi_cmnd *cmd);
 extern int scsi_dma_map(struct scsi_cmnd *cmd);
 extern void scsi_dma_unmap(struct scsi_cmnd *cmd);
 
-struct scsi_cmnd *scsi_allocate_command(gfp_t gfp_mask);
-void scsi_free_command(gfp_t gfp_mask, struct scsi_cmnd *cmd);
-
 static inline unsigned scsi_sg_count(struct scsi_cmnd *cmd)
 {
 	return cmd->sdb.table.nents;
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 14/39] scsi: simplify command allocation and freeing a bit
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (12 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 13/39] megaraid: simplify internal command handling Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 15/39] scsi: add support for per-host cmd pools Christoph Hellwig
                   ` (25 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Paolo Bonzini

[-- Attachment #1: 0014-scsi-simplify-command-allocation-and-freeing-a-bit.patch --]
[-- Type: text/plain, Size: 3815 bytes --]

Just have one level of alloc/free functions that take a host instead
of two levels for the allocation and different calling conventions
for the free.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/scsi/scsi.c |   67 +++++++++++++++++++--------------------------------
 1 file changed, 25 insertions(+), 42 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 8b2bc06..391dd9f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -161,47 +161,20 @@ static struct scsi_host_cmd_pool scsi_cmd_dma_pool = {
 static DEFINE_MUTEX(host_cmd_pool_mutex);
 
 /**
- * scsi_pool_alloc_command - internal function to get a fully allocated command
- * @pool:	slab pool to allocate the command from
- * @gfp_mask:	mask for the allocation
- *
- * Returns a fully allocated command (with the allied sense buffer) or
- * NULL on failure
- */
-static struct scsi_cmnd *
-scsi_pool_alloc_command(struct scsi_host_cmd_pool *pool, gfp_t gfp_mask)
-{
-	struct scsi_cmnd *cmd;
-
-	cmd = kmem_cache_zalloc(pool->cmd_slab, gfp_mask | pool->gfp_mask);
-	if (!cmd)
-		return NULL;
-
-	cmd->sense_buffer = kmem_cache_alloc(pool->sense_slab,
-					     gfp_mask | pool->gfp_mask);
-	if (!cmd->sense_buffer) {
-		kmem_cache_free(pool->cmd_slab, cmd);
-		return NULL;
-	}
-
-	return cmd;
-}
-
-/**
- * scsi_pool_free_command - internal function to release a command
- * @pool:	slab pool to allocate the command from
+ * scsi_host_free_command - internal function to release a command
+ * @host:	host to free the command for
  * @cmd:	command to release
  *
  * the command must previously have been allocated by
- * scsi_pool_alloc_command.
+ * scsi_host_alloc_command.
  */
 static void
-scsi_pool_free_command(struct scsi_host_cmd_pool *pool,
-			 struct scsi_cmnd *cmd)
+scsi_host_free_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 {
+	struct scsi_host_cmd_pool *pool = shost->cmd_pool;
+
 	if (cmd->prot_sdb)
 		kmem_cache_free(scsi_sdb_cache, cmd->prot_sdb);
-
 	kmem_cache_free(pool->sense_slab, cmd->sense_buffer);
 	kmem_cache_free(pool->cmd_slab, cmd);
 }
@@ -217,22 +190,32 @@ scsi_pool_free_command(struct scsi_host_cmd_pool *pool,
 static struct scsi_cmnd *
 scsi_host_alloc_command(struct Scsi_Host *shost, gfp_t gfp_mask)
 {
+	struct scsi_host_cmd_pool *pool = shost->cmd_pool;
 	struct scsi_cmnd *cmd;
 
-	cmd = scsi_pool_alloc_command(shost->cmd_pool, gfp_mask);
+	cmd = kmem_cache_zalloc(pool->cmd_slab, gfp_mask | pool->gfp_mask);
 	if (!cmd)
-		return NULL;
+		goto fail;
+
+	cmd->sense_buffer = kmem_cache_alloc(pool->sense_slab,
+					     gfp_mask | pool->gfp_mask);
+	if (!cmd->sense_buffer)
+		goto fail_free_cmd;
 
 	if (scsi_host_get_prot(shost) >= SHOST_DIX_TYPE0_PROTECTION) {
 		cmd->prot_sdb = kmem_cache_zalloc(scsi_sdb_cache, gfp_mask);
-
-		if (!cmd->prot_sdb) {
-			scsi_pool_free_command(shost->cmd_pool, cmd);
-			return NULL;
-		}
+		if (!cmd->prot_sdb)
+			goto fail_free_sense;
 	}
 
 	return cmd;
+
+fail_free_sense:
+	kmem_cache_free(pool->sense_slab, cmd->sense_buffer);
+fail_free_cmd:
+	kmem_cache_free(pool->cmd_slab, cmd);
+fail:
+	return NULL;
 }
 
 /**
@@ -320,7 +303,7 @@ void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 	}
 
 	if (likely(cmd != NULL))
-		scsi_pool_free_command(shost->cmd_pool, cmd);
+		scsi_host_free_command(shost, cmd);
 }
 EXPORT_SYMBOL(__scsi_put_command);
 
@@ -456,7 +439,7 @@ void scsi_destroy_command_freelist(struct Scsi_Host *shost)
 
 		cmd = list_entry(shost->free_list.next, struct scsi_cmnd, list);
 		list_del_init(&cmd->list);
-		scsi_pool_free_command(shost->cmd_pool, cmd);
+		scsi_host_free_command(shost, cmd);
 	}
 	shost->cmd_pool = NULL;
 	scsi_put_host_cmd_pool(shost->unchecked_isa_dma ? GFP_DMA : GFP_KERNEL);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 15/39] scsi: add support for per-host cmd pools
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (13 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 14/39] scsi: simplify command allocation and freeing a bit Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 16/39] virtio_scsi: use cmd_size Christoph Hellwig
                   ` (24 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Paolo Bonzini

[-- Attachment #1: 0015-scsi-add-support-for-per-host-cmd-pools.patch --]
[-- Type: text/plain, Size: 5945 bytes --]

This allows drivers to specify the size of their per-command private
data in the host template and then get extra memory allocated for
each command instead of needing another allocation in ->queuecommand.

With the current SCSI code that already does multiple allocations for
each command this probably doesn't make a big performance impact, but
it allows to clean up the drivers, and prepare them for using the
blk-mq infrastructure where the common allocation will make a difference.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/scsi/scsi.c      |   96 +++++++++++++++++++++++++++++++++++++---------
 include/scsi/scsi_host.h |    7 ++++
 2 files changed, 84 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 391dd9f..eff189f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -331,46 +331,103 @@ void scsi_put_command(struct scsi_cmnd *cmd)
 }
 EXPORT_SYMBOL(scsi_put_command);
 
-static struct scsi_host_cmd_pool *scsi_get_host_cmd_pool(gfp_t gfp_mask)
+static struct scsi_host_cmd_pool *
+scsi_find_host_cmd_pool(struct Scsi_Host *shost)
 {
+	if (shost->hostt->cmd_size)
+		return shost->hostt->cmd_pool;
+	if (shost->unchecked_isa_dma)
+		return &scsi_cmd_dma_pool;
+	return &scsi_cmd_pool;
+}
+
+static void
+scsi_free_host_cmd_pool(struct scsi_host_cmd_pool *pool)
+{
+	kfree(pool->sense_name);
+	kfree(pool->cmd_name);
+	kfree(pool);
+}
+
+static struct scsi_host_cmd_pool *
+scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
+{
+	struct scsi_host_template *hostt = shost->hostt;
+	struct scsi_host_cmd_pool *pool;
+
+	pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+	if (!pool)
+		return NULL;
+
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	if (!pool->cmd_name || !pool->sense_name) {
+		scsi_free_host_cmd_pool(pool);
+		return NULL;
+	}
+
+	pool->slab_flags = SLAB_HWCACHE_ALIGN;
+	if (shost->unchecked_isa_dma) {
+		pool->slab_flags |= SLAB_CACHE_DMA;
+		pool->gfp_mask = __GFP_DMA;
+	}
+	return pool;
+}
+
+static struct scsi_host_cmd_pool *
+scsi_get_host_cmd_pool(struct Scsi_Host *shost)
+{
+	struct scsi_host_template *hostt = shost->hostt;
 	struct scsi_host_cmd_pool *retval = NULL, *pool;
+	size_t cmd_size = sizeof(struct scsi_cmnd) + hostt->cmd_size;
+
 	/*
 	 * Select a command slab for this host and create it if not
 	 * yet existent.
 	 */
 	mutex_lock(&host_cmd_pool_mutex);
-	pool = (gfp_mask & __GFP_DMA) ? &scsi_cmd_dma_pool :
-		&scsi_cmd_pool;
+	pool = scsi_find_host_cmd_pool(shost);
+	if (!pool) {
+		pool = scsi_alloc_host_cmd_pool(shost);
+		if (!pool)
+			goto out;
+	}
+
 	if (!pool->users) {
-		pool->cmd_slab = kmem_cache_create(pool->cmd_name,
-						   sizeof(struct scsi_cmnd), 0,
+		pool->cmd_slab = kmem_cache_create(pool->cmd_name, cmd_size, 0,
 						   pool->slab_flags, NULL);
 		if (!pool->cmd_slab)
-			goto fail;
+			goto out_free_pool;
 
 		pool->sense_slab = kmem_cache_create(pool->sense_name,
 						     SCSI_SENSE_BUFFERSIZE, 0,
 						     pool->slab_flags, NULL);
-		if (!pool->sense_slab) {
-			kmem_cache_destroy(pool->cmd_slab);
-			goto fail;
-		}
+		if (!pool->sense_slab)
+			goto out_free_slab;
 	}
 
 	pool->users++;
 	retval = pool;
- fail:
+out:
 	mutex_unlock(&host_cmd_pool_mutex);
 	return retval;
+
+out_free_slab:
+	kmem_cache_destroy(pool->cmd_slab);
+out_free_pool:
+	if (hostt->cmd_size)
+		scsi_free_host_cmd_pool(pool);
+	goto out;
 }
 
-static void scsi_put_host_cmd_pool(gfp_t gfp_mask)
+static void scsi_put_host_cmd_pool(struct Scsi_Host *shost)
 {
+	struct scsi_host_template *hostt = shost->hostt;
 	struct scsi_host_cmd_pool *pool;
 
 	mutex_lock(&host_cmd_pool_mutex);
-	pool = (gfp_mask & __GFP_DMA) ? &scsi_cmd_dma_pool :
-		&scsi_cmd_pool;
+	pool = scsi_find_host_cmd_pool(shost);
+
 	/*
 	 * This may happen if a driver has a mismatched get and put
 	 * of the command pool; the driver should be implicated in
@@ -381,6 +438,8 @@ static void scsi_put_host_cmd_pool(gfp_t gfp_mask)
 	if (!--pool->users) {
 		kmem_cache_destroy(pool->cmd_slab);
 		kmem_cache_destroy(pool->sense_slab);
+		if (hostt->cmd_size)
+			scsi_free_host_cmd_pool(pool);
 	}
 	mutex_unlock(&host_cmd_pool_mutex);
 }
@@ -397,14 +456,13 @@ static void scsi_put_host_cmd_pool(gfp_t gfp_mask)
  */
 int scsi_setup_command_freelist(struct Scsi_Host *shost)
 {
-	struct scsi_cmnd *cmd;
 	const gfp_t gfp_mask = shost->unchecked_isa_dma ? GFP_DMA : GFP_KERNEL;
+	struct scsi_cmnd *cmd;
 
 	spin_lock_init(&shost->free_list_lock);
 	INIT_LIST_HEAD(&shost->free_list);
 
-	shost->cmd_pool = scsi_get_host_cmd_pool(gfp_mask);
-
+	shost->cmd_pool = scsi_get_host_cmd_pool(shost);
 	if (!shost->cmd_pool)
 		return -ENOMEM;
 
@@ -413,7 +471,7 @@ int scsi_setup_command_freelist(struct Scsi_Host *shost)
 	 */
 	cmd = scsi_host_alloc_command(shost, gfp_mask);
 	if (!cmd) {
-		scsi_put_host_cmd_pool(gfp_mask);
+		scsi_put_host_cmd_pool(shost);
 		shost->cmd_pool = NULL;
 		return -ENOMEM;
 	}
@@ -442,7 +500,7 @@ void scsi_destroy_command_freelist(struct Scsi_Host *shost)
 		scsi_host_free_command(shost, cmd);
 	}
 	shost->cmd_pool = NULL;
-	scsi_put_host_cmd_pool(shost->unchecked_isa_dma ? GFP_DMA : GFP_KERNEL);
+	scsi_put_host_cmd_pool(shost);
 }
 
 #ifdef CONFIG_SCSI_LOGGING
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 53075e5..94844fc 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -15,6 +15,7 @@ struct completion;
 struct module;
 struct scsi_cmnd;
 struct scsi_device;
+struct scsi_host_cmd_pool;
 struct scsi_target;
 struct Scsi_Host;
 struct scsi_host_cmd_pool;
@@ -524,6 +525,12 @@ struct scsi_host_template {
 	 *   scsi_netlink.h
 	 */
 	u64 vendor_id;
+
+	/*
+	 * Additional per-command data allocated for the driver.
+	 */
+	unsigned int cmd_size;
+	struct scsi_host_cmd_pool *cmd_pool;
 };
 
 /*
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 16/39] virtio_scsi: use cmd_size
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (14 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 15/39] scsi: add support for per-host cmd pools Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-25 15:31   ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 17/39] scsi: explicitly release bidi buffers Christoph Hellwig
                   ` (23 subsequent siblings)
  39 siblings, 1 reply; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel; +Cc: Paolo Bonzini

[-- Attachment #1: 0016-virtio_scsi-use-cmd_size.patch --]
[-- Type: text/plain, Size: 3491 bytes --]

Taken almost entirely from Nicholas Bellinger's scsi-mq conversion.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/scsi/virtio_scsi.c |   25 +++++++------------------
 include/scsi/scsi_cmnd.h   |    9 +++++++++
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 16bfd50..5ec4a73 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -204,7 +204,6 @@ static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf)
 			set_driver_byte(sc, DRIVER_SENSE);
 	}
 
-	mempool_free(cmd, virtscsi_cmd_pool);
 	sc->scsi_done(sc);
 
 	atomic_dec(&tgt->reqs);
@@ -279,8 +278,6 @@ static void virtscsi_complete_free(struct virtio_scsi *vscsi, void *buf)
 
 	if (cmd->comp)
 		complete_all(cmd->comp);
-	else
-		mempool_free(cmd, virtscsi_cmd_pool);
 }
 
 static void virtscsi_ctrl_done(struct virtqueue *vq)
@@ -496,10 +493,9 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
 				 struct virtio_scsi_vq *req_vq,
 				 struct scsi_cmnd *sc)
 {
-	struct virtio_scsi_cmd *cmd;
-	int ret;
-
 	struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
+	struct virtio_scsi_cmd *cmd = scsi_cmd_priv(sc);
+
 	BUG_ON(scsi_sg_count(sc) > shost->sg_tablesize);
 
 	/* TODO: check feature bit and fail if unsupported?  */
@@ -508,11 +504,6 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
 	dev_dbg(&sc->device->sdev_gendev,
 		"cmd %p CDB: %#02x\n", sc, sc->cmnd[0]);
 
-	ret = SCSI_MLQUEUE_HOST_BUSY;
-	cmd = mempool_alloc(virtscsi_cmd_pool, GFP_ATOMIC);
-	if (!cmd)
-		goto out;
-
 	memset(cmd, 0, sizeof(*cmd));
 	cmd->sc = sc;
 	cmd->req.cmd = (struct virtio_scsi_cmd_req){
@@ -531,13 +522,9 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
 
 	if (virtscsi_kick_cmd(req_vq, cmd,
 			      sizeof cmd->req.cmd, sizeof cmd->resp.cmd,
-			      GFP_ATOMIC) == 0)
-		ret = 0;
-	else
-		mempool_free(cmd, virtscsi_cmd_pool);
-
-out:
-	return ret;
+			      GFP_ATOMIC) != 0)
+		return SCSI_MLQUEUE_HOST_BUSY;
+	return 0;
 }
 
 static int virtscsi_queuecommand_single(struct Scsi_Host *sh,
@@ -683,6 +670,7 @@ static struct scsi_host_template virtscsi_host_template_single = {
 	.name = "Virtio SCSI HBA",
 	.proc_name = "virtio_scsi",
 	.this_id = -1,
+	.cmd_size = sizeof(struct virtio_scsi_cmd),
 	.queuecommand = virtscsi_queuecommand_single,
 	.eh_abort_handler = virtscsi_abort,
 	.eh_device_reset_handler = virtscsi_device_reset,
@@ -699,6 +687,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
 	.name = "Virtio SCSI HBA",
 	.proc_name = "virtio_scsi",
 	.this_id = -1,
+	.cmd_size = sizeof(struct virtio_scsi_cmd),
 	.queuecommand = virtscsi_queuecommand_multi,
 	.eh_abort_handler = virtscsi_abort,
 	.eh_device_reset_handler = virtscsi_device_reset,
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index dd7c998..e016e2a 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -133,6 +133,15 @@ struct scsi_cmnd {
 	unsigned char tag;	/* SCSI-II queued command tag */
 };
 
+/*
+ * Return the driver private allocation behind the command.
+ * Only works if cmd_size is set in the host template.
+ */
+static inline void *scsi_cmd_priv(struct scsi_cmnd *cmd)
+{
+	return cmd + 1;
+}
+
 /* make sure not to use it with REQ_TYPE_BLOCK_PC commands */
 static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
 {
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 17/39] scsi: explicitly release bidi buffers
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (15 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 16/39] virtio_scsi: use cmd_size Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 18/39] scsi: remove scsi_end_request Christoph Hellwig
                   ` (22 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0017-scsi-explicitly-release-bidi-buffers.patch --]
[-- Type: text/plain, Size: 3434 bytes --]

Instead of trying to guess when we have a BIDI buffer in scsi_release_buffers
add a function to explicitly free the BIDI ressoures in the one place that
handles them.  This avoids needing a special __scsi_release_buffers for the
case where we already have freed the request as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |   47 ++++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index debe30a..3012700 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -511,8 +511,6 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }
 
-static void __scsi_release_buffers(struct scsi_cmnd *, int);
-
 /*
  * Function:    scsi_end_request()
  *
@@ -568,7 +566,7 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
 	 * This will goose the queue request function at the end, so we don't
 	 * need to worry about launching another command.
 	 */
-	__scsi_release_buffers(cmd, 0);
+	scsi_release_buffers(cmd);
 	scsi_next_command(cmd);
 	return NULL;
 }
@@ -624,30 +622,10 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb)
 	__sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, scsi_sg_free);
 }
 
-static void __scsi_release_buffers(struct scsi_cmnd *cmd, int do_bidi_check)
-{
-
-	if (cmd->sdb.table.nents)
-		scsi_free_sgtable(&cmd->sdb);
-
-	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
-
-	if (do_bidi_check && scsi_bidi_cmnd(cmd)) {
-		struct scsi_data_buffer *bidi_sdb =
-			cmd->request->next_rq->special;
-		scsi_free_sgtable(bidi_sdb);
-		kmem_cache_free(scsi_sdb_cache, bidi_sdb);
-		cmd->request->next_rq->special = NULL;
-	}
-
-	if (scsi_prot_sg_count(cmd))
-		scsi_free_sgtable(cmd->prot_sdb);
-}
-
 /*
  * Function:    scsi_release_buffers()
  *
- * Purpose:     Completion processing for block device I/O requests.
+ * Purpose:     Free resources allocate for a scsi_command.
  *
  * Arguments:   cmd	- command that we are bailing.
  *
@@ -658,15 +636,29 @@ static void __scsi_release_buffers(struct scsi_cmnd *cmd, int do_bidi_check)
  * Notes:       In the event that an upper level driver rejects a
  *		command, we must release resources allocated during
  *		the __init_io() function.  Primarily this would involve
- *		the scatter-gather table, and potentially any bounce
- *		buffers.
+ *		the scatter-gather table.
  */
 void scsi_release_buffers(struct scsi_cmnd *cmd)
 {
-	__scsi_release_buffers(cmd, 1);
+	if (cmd->sdb.table.nents)
+		scsi_free_sgtable(&cmd->sdb);
+
+	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
+
+	if (scsi_prot_sg_count(cmd))
+		scsi_free_sgtable(cmd->prot_sdb);
 }
 EXPORT_SYMBOL(scsi_release_buffers);
 
+static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
+{
+	struct scsi_data_buffer *bidi_sdb = cmd->request->next_rq->special;
+
+	scsi_free_sgtable(bidi_sdb);
+	kmem_cache_free(scsi_sdb_cache, bidi_sdb);
+	cmd->request->next_rq->special = NULL;
+}
+
 /**
  * __scsi_error_from_host_byte - translate SCSI error code into errno
  * @cmd:	SCSI command (unused)
@@ -799,6 +791,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			req->next_rq->resid_len = scsi_in(cmd)->resid;
 
 			scsi_release_buffers(cmd);
+			scsi_release_bidi_buffers(cmd);
 			blk_end_request_all(req, 0);
 
 			scsi_next_command(cmd);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 18/39] scsi: remove scsi_end_request
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (16 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 17/39] scsi: explicitly release bidi buffers Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 19/39] scsi: push host_lock down into scsi_{host,target}_queue_ready Christoph Hellwig
                   ` (21 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0018-scsi-remove-scsi_end_request.patch --]
[-- Type: text/plain, Size: 6332 bytes --]

By folding scsi_end_request into its only caller we can significantly clean
up the completion logic.  We can use simple goto labels now to only have
a single place to finish or requeue command there instead of the previous
convoluted logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |  114 +++++++++++++----------------------------------
 1 file changed, 32 insertions(+), 82 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 3012700..0bcb7c4 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -511,66 +511,6 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }
 
-/*
- * Function:    scsi_end_request()
- *
- * Purpose:     Post-processing of completed commands (usually invoked at end
- *		of upper level post-processing and scsi_io_completion).
- *
- * Arguments:   cmd	 - command that is complete.
- *              error    - 0 if I/O indicates success, < 0 for I/O error.
- *              bytes    - number of bytes of completed I/O
- *		requeue  - indicates whether we should requeue leftovers.
- *
- * Lock status: Assumed that lock is not held upon entry.
- *
- * Returns:     cmd if requeue required, NULL otherwise.
- *
- * Notes:       This is called for block device requests in order to
- *              mark some number of sectors as complete.
- * 
- *		We are guaranteeing that the request queue will be goosed
- *		at some point during this call.
- * Notes:	If cmd was requeued, upon return it will be a stale pointer.
- */
-static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
-					  int bytes, int requeue)
-{
-	struct request_queue *q = cmd->device->request_queue;
-	struct request *req = cmd->request;
-
-	/*
-	 * If there are blocks left over at the end, set up the command
-	 * to queue the remainder of them.
-	 */
-	if (blk_end_request(req, error, bytes)) {
-		/* kill remainder if no retrys */
-		if (error && scsi_noretry_cmd(cmd))
-			blk_end_request_all(req, error);
-		else {
-			if (requeue) {
-				/*
-				 * Bleah.  Leftovers again.  Stick the
-				 * leftovers in the front of the
-				 * queue, and goose the queue again.
-				 */
-				scsi_release_buffers(cmd);
-				scsi_requeue_command(q, cmd);
-				cmd = NULL;
-			}
-			return cmd;
-		}
-	}
-
-	/*
-	 * This will goose the queue request function at the end, so we don't
-	 * need to worry about launching another command.
-	 */
-	scsi_release_buffers(cmd);
-	scsi_next_command(cmd);
-	return NULL;
-}
-
 static inline unsigned int scsi_sgtable_index(unsigned short nents)
 {
 	unsigned int index;
@@ -716,16 +656,9 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
  *
  * Returns:     Nothing
  *
- * Notes:       This function is matched in terms of capabilities to
- *              the function that created the scatter-gather list.
- *              In other words, if there are no bounce buffers
- *              (the normal case for most drivers), we don't need
- *              the logic to deal with cleaning up afterwards.
- *
- *		We must call scsi_end_request().  This will finish off
- *		the specified number of sectors.  If we are done, the
- *		command block will be released and the queue function
- *		will be goosed.  If we are not done then we have to
+ * Notes:       We will finish off the specified number of sectors.  If we
+ *		are done, the command block will be released and the queue
+ *		function will be goosed.  If we are not done then we have to
  *		figure out what to do next:
  *
  *		a) We can call scsi_requeue_command().  The request
@@ -734,7 +667,7 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
  *		   be used if we made forward progress, or if we want
  *		   to switch from READ(10) to READ(6) for example.
  *
- *		b) We can call scsi_queue_insert().  The request will
+ *		b) We can call __scsi_queue_insert().  The request will
  *		   be put back on the queue and retried using the same
  *		   command as before, possibly after a delay.
  *
@@ -792,6 +725,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 
 			scsi_release_buffers(cmd);
 			scsi_release_bidi_buffers(cmd);
+
 			blk_end_request_all(req, 0);
 
 			scsi_next_command(cmd);
@@ -831,12 +765,25 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	}
 
 	/*
-	 * A number of bytes were successfully read.  If there
-	 * are leftovers and there is some kind of error
-	 * (result != 0), retry the rest.
+	 * If we finished all bytes in the request we are done now.
 	 */
-	if (scsi_end_request(cmd, error, good_bytes, result == 0) == NULL)
-		return;
+	if (!blk_end_request(req, error, good_bytes))
+		goto next_command;
+
+	/*
+	 * Kill remainder if no retrys.
+	 */
+	if (error && scsi_noretry_cmd(cmd)) {
+		blk_end_request_all(req, error);
+		goto next_command;
+	}
+
+	/*
+	 * If there had been no error, but we have leftover bytes in the
+	 * requeues just queue the command up again.
+	 */
+	if (result == 0)
+		goto requeue;
 
 	error = __scsi_error_from_host_byte(cmd, result);
 
@@ -958,7 +905,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	switch (action) {
 	case ACTION_FAIL:
 		/* Give up and fail the remainder of the request */
-		scsi_release_buffers(cmd);
 		if (!(req->cmd_flags & REQ_QUIET)) {
 			if (description)
 				scmd_printk(KERN_INFO, cmd, "%s\n",
@@ -968,12 +914,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 				scsi_print_sense("", cmd);
 			scsi_print_command(cmd);
 		}
-		if (blk_end_request_err(req, error))
-			scsi_requeue_command(q, cmd);
-		else
-			scsi_next_command(cmd);
-		break;
+		if (!blk_end_request_err(req, error))
+			goto next_command;
+		/*FALLTHRU*/
 	case ACTION_REPREP:
+	requeue:
 		/* Unprep the request and put it back at the head of the queue.
 		 * A new command will be prepared and issued.
 		 */
@@ -989,6 +934,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 		__scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY, 0);
 		break;
 	}
+	return;
+
+next_command:
+	scsi_release_buffers(cmd);
+	scsi_next_command(cmd);
 }
 
 static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb,
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 19/39] scsi: push host_lock down into scsi_{host,target}_queue_ready
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (17 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 18/39] scsi: remove scsi_end_request Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 20/39] scsi: convert target_busy to an atomic_t Christoph Hellwig
                   ` (20 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0019-scsi-push-host_lock-down-into-scsi_-host-target-_que.patch --]
[-- Type: text/plain, Size: 5087 bytes --]

Prepare for not taking a host-wide lock in the dispatch path by pushing
the lock down into the places that actually need it.  Note that this
patch is just a preparation step, as it will actually increase lock
roundtrips and thus decrease performance on its own.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |   75 ++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0bcb7c4..a549044 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1273,18 +1273,18 @@ static inline int scsi_dev_queue_ready(struct request_queue *q,
 /*
  * scsi_target_queue_ready: checks if there we can send commands to target
  * @sdev: scsi device on starget to check.
- *
- * Called with the host lock held.
  */
 static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
 					   struct scsi_device *sdev)
 {
 	struct scsi_target *starget = scsi_target(sdev);
+	int ret = 0;
 
+	spin_lock_irq(shost->host_lock);
 	if (starget->single_lun) {
 		if (starget->starget_sdev_user &&
 		    starget->starget_sdev_user != sdev)
-			return 0;
+			goto out;
 		starget->starget_sdev_user = sdev;
 	}
 
@@ -1292,57 +1292,66 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
 		/*
 		 * unblock after target_blocked iterates to zero
 		 */
-		if (--starget->target_blocked == 0) {
-			SCSI_LOG_MLQUEUE(3, starget_printk(KERN_INFO, starget,
-					 "unblocking target at zero depth\n"));
-		} else
-			return 0;
+		if (--starget->target_blocked != 0)
+			goto out;
+
+		SCSI_LOG_MLQUEUE(3, starget_printk(KERN_INFO, starget,
+				 "unblocking target at zero depth\n"));
 	}
 
 	if (scsi_target_is_busy(starget)) {
 		list_move_tail(&sdev->starved_entry, &shost->starved_list);
-		return 0;
+		goto out;
 	}
 
-	return 1;
+	scsi_target(sdev)->target_busy++;
+	ret = 1;
+out:
+	spin_unlock_irq(shost->host_lock);
+	return ret;
 }
 
 /*
  * scsi_host_queue_ready: if we can send requests to shost, return 1 else
  * return 0. We must end up running the queue again whenever 0 is
  * returned, else IO can hang.
- *
- * Called with host_lock held.
  */
 static inline int scsi_host_queue_ready(struct request_queue *q,
 				   struct Scsi_Host *shost,
 				   struct scsi_device *sdev)
 {
+	int ret = 0;
+
+	spin_lock_irq(shost->host_lock);
+
 	if (scsi_host_in_recovery(shost))
-		return 0;
+		goto out;
 	if (shost->host_busy == 0 && shost->host_blocked) {
 		/*
 		 * unblock after host_blocked iterates to zero
 		 */
-		if (--shost->host_blocked == 0) {
-			SCSI_LOG_MLQUEUE(3,
-				printk("scsi%d unblocking host at zero depth\n",
-					shost->host_no));
-		} else {
-			return 0;
-		}
+		if (--shost->host_blocked != 0)
+			goto out;
+
+		SCSI_LOG_MLQUEUE(3,
+			printk("scsi%d unblocking host at zero depth\n",
+				shost->host_no));
 	}
 	if (scsi_host_is_busy(shost)) {
 		if (list_empty(&sdev->starved_entry))
 			list_add_tail(&sdev->starved_entry, &shost->starved_list);
-		return 0;
+		goto out;
 	}
 
 	/* We're OK to process the command, so we can't be starved */
 	if (!list_empty(&sdev->starved_entry))
 		list_del_init(&sdev->starved_entry);
 
-	return 1;
+	shost->host_busy++;
+	ret = 1;
+out:
+	spin_unlock_irq(shost->host_lock);
+	return ret;
 }
 
 /*
@@ -1506,7 +1515,7 @@ static void scsi_request_fn(struct request_queue *q)
 			blk_start_request(req);
 		sdev->device_busy++;
 
-		spin_unlock(q->queue_lock);
+		spin_unlock_irq(q->queue_lock);
 		cmd = req->special;
 		if (unlikely(cmd == NULL)) {
 			printk(KERN_CRIT "impossible request in %s.\n"
@@ -1516,7 +1525,6 @@ static void scsi_request_fn(struct request_queue *q)
 			blk_dump_rq_flags(req, "foo");
 			BUG();
 		}
-		spin_lock(shost->host_lock);
 
 		/*
 		 * We hit this when the driver is using a host wide
@@ -1527,9 +1535,11 @@ static void scsi_request_fn(struct request_queue *q)
 		 * a run when a tag is freed.
 		 */
 		if (blk_queue_tagged(q) && !blk_rq_tagged(req)) {
+			spin_lock_irq(shost->host_lock);
 			if (list_empty(&sdev->starved_entry))
 				list_add_tail(&sdev->starved_entry,
 					      &shost->starved_list);
+			spin_unlock_irq(shost->host_lock);
 			goto not_ready;
 		}
 
@@ -1537,16 +1547,7 @@ static void scsi_request_fn(struct request_queue *q)
 			goto not_ready;
 
 		if (!scsi_host_queue_ready(q, shost, sdev))
-			goto not_ready;
-
-		scsi_target(sdev)->target_busy++;
-		shost->host_busy++;
-
-		/*
-		 * XXX(hch): This is rather suboptimal, scsi_dispatch_cmd will
-		 *		take the lock again.
-		 */
-		spin_unlock_irq(shost->host_lock);
+			goto host_not_ready;
 
 		/*
 		 * Finally, initialize any error handling parameters, and set up
@@ -1565,9 +1566,11 @@ static void scsi_request_fn(struct request_queue *q)
 
 	return;
 
- not_ready:
+ host_not_ready:
+	spin_lock_irq(shost->host_lock);
+	scsi_target(sdev)->target_busy--;
 	spin_unlock_irq(shost->host_lock);
-
+ not_ready:
 	/*
 	 * lock q, handle tag, requeue req, and decrement device_busy. We
 	 * must return with queue_lock held.
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 20/39] scsi: convert target_busy to an atomic_t
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (18 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 21/39] scsi: convert host_busy to atomic_t Christoph Hellwig
                   ` (19 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0020-scsi-convert-target_busy-to-an-atomic_t.patch --]
[-- Type: text/plain, Size: 4298 bytes --]

Avoid taking the host-wide host_lock to check the per-target 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.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c    |   52 ++++++++++++++++++++++++++------------------
 include/scsi/scsi_device.h |    4 ++--
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a549044..01a631e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -283,7 +283,7 @@ void scsi_device_unbusy(struct scsi_device *sdev)
 
 	spin_lock_irqsave(shost->host_lock, flags);
 	shost->host_busy--;
-	starget->target_busy--;
+	atomic_dec(&starget->target_busy);
 	if (unlikely(scsi_host_in_recovery(shost) &&
 		     (shost->host_failed || shost->host_eh_scheduled)))
 		scsi_eh_wakeup(shost);
@@ -350,7 +350,7 @@ static inline int scsi_device_is_busy(struct scsi_device *sdev)
 static inline int scsi_target_is_busy(struct scsi_target *starget)
 {
 	return ((starget->can_queue > 0 &&
-		 starget->target_busy >= starget->can_queue) ||
+		 atomic_read(&starget->target_busy) >= starget->can_queue) ||
 		 starget->target_blocked);
 }
 
@@ -1278,37 +1278,49 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
 					   struct scsi_device *sdev)
 {
 	struct scsi_target *starget = scsi_target(sdev);
-	int ret = 0;
+	unsigned int busy;
 
-	spin_lock_irq(shost->host_lock);
 	if (starget->single_lun) {
+		spin_lock_irq(shost->host_lock);
 		if (starget->starget_sdev_user &&
-		    starget->starget_sdev_user != sdev)
-			goto out;
+		    starget->starget_sdev_user != sdev) {
+			spin_unlock_irq(shost->host_lock);
+			return 0;
+		}
 		starget->starget_sdev_user = sdev;
+		spin_unlock_irq(shost->host_lock);
 	}
 
-	if (starget->target_busy == 0 && starget->target_blocked) {
+	busy = atomic_inc_return(&starget->target_busy) - 1;
+	if (busy == 0 && starget->target_blocked) {
 		/*
 		 * unblock after target_blocked iterates to zero
 		 */
-		if (--starget->target_blocked != 0)
-			goto out;
+		spin_lock_irq(shost->host_lock);
+		if (--starget->target_blocked != 0) {
+			spin_unlock_irq(shost->host_lock);
+			goto out_dec;
+		}
+		spin_unlock_irq(shost->host_lock);
 
 		SCSI_LOG_MLQUEUE(3, starget_printk(KERN_INFO, starget,
 				 "unblocking target at zero depth\n"));
 	}
 
-	if (scsi_target_is_busy(starget)) {
-		list_move_tail(&sdev->starved_entry, &shost->starved_list);
-		goto out;
-	}
+	if (starget->can_queue > 0 && busy >= starget->can_queue)
+		goto starved;
+	if (starget->target_blocked)
+		goto starved;
 
-	scsi_target(sdev)->target_busy++;
-	ret = 1;
-out:
+	return 1;
+
+starved:
+	spin_lock_irq(shost->host_lock);
+	list_move_tail(&sdev->starved_entry, &shost->starved_list);
 	spin_unlock_irq(shost->host_lock);
-	return ret;
+out_dec:
+	atomic_dec(&starget->target_busy);
+	return 0;
 }
 
 /*
@@ -1418,7 +1430,7 @@ static void scsi_kill_request(struct request *req, struct request_queue *q)
 	spin_unlock(sdev->request_queue->queue_lock);
 	spin_lock(shost->host_lock);
 	shost->host_busy++;
-	starget->target_busy++;
+	atomic_inc(&starget->target_busy);
 	spin_unlock(shost->host_lock);
 	spin_lock(sdev->request_queue->queue_lock);
 
@@ -1567,9 +1579,7 @@ static void scsi_request_fn(struct request_queue *q)
 	return;
 
  host_not_ready:
-	spin_lock_irq(shost->host_lock);
-	scsi_target(sdev)->target_busy--;
-	spin_unlock_irq(shost->host_lock);
+	atomic_dec(&scsi_target(sdev)->target_busy);
  not_ready:
 	/*
 	 * lock q, handle tag, requeue req, and decrement device_busy. We
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d65fbec..f85b1a2 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -272,8 +272,8 @@ struct scsi_target {
 	unsigned int		expecting_lun_change:1;	/* A device has reported
 						 * a 3F/0E UA, other devices on
 						 * the same target will also. */
-	/* commands actually active on LLD. protected by host lock. */
-	unsigned int		target_busy;
+	/* commands actually active on LLD. */
+	atomic_t		target_busy;
 	/*
 	 * LLDs should set this in the slave_alloc host template callout.
 	 * If set to zero then there is not limit.
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 21/39] scsi: convert host_busy to atomic_t
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (19 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 20/39] scsi: convert target_busy to an atomic_t Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 22/39] scsi: convert device_busy " Christoph Hellwig
                   ` (18 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0021-scsi-convert-host_busy-to-atomic_t.patch --]
[-- Type: text/plain, Size: 11452 bytes --]

Avoid taking the host-wide host_lock to check the per-host 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.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/advansys.c             |    4 +-
 drivers/scsi/libiscsi.c             |    4 +-
 drivers/scsi/libsas/sas_scsi_host.c |    5 ++-
 drivers/scsi/qlogicpti.c            |    2 +-
 drivers/scsi/scsi.c                 |    2 +-
 drivers/scsi/scsi_error.c           |    6 +--
 drivers/scsi/scsi_lib.c             |   71 +++++++++++++++++++++--------------
 drivers/scsi/scsi_sysfs.c           |    9 ++++-
 include/scsi/scsi_host.h            |   10 ++---
 9 files changed, 65 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index d814588..0a6ecbd 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2512,7 +2512,7 @@ static void asc_prt_scsi_host(struct Scsi_Host *s)
 
 	printk("Scsi_Host at addr 0x%p, device %s\n", s, dev_name(boardp->dev));
 	printk(" host_busy %u, host_no %d,\n",
-	       s->host_busy, s->host_no);
+	       atomic_read(&s->host_busy), s->host_no);
 
 	printk(" base 0x%lx, io_port 0x%lx, irq %d,\n",
 	       (ulong)s->base, (ulong)s->io_port, boardp->irq);
@@ -3346,7 +3346,7 @@ static void asc_prt_driver_conf(struct seq_file *m, struct Scsi_Host *shost)
 
 	seq_printf(m,
 		   " host_busy %u, max_id %u, max_lun %u, max_channel %u\n",
-		   shost->host_busy, shost->max_id,
+		   atomic_read(&shost->host_busy), shost->max_id,
 		   shost->max_lun, shost->max_channel);
 
 	seq_printf(m,
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 4046241..718ca88 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2922,7 +2922,7 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
 	 */
 	for (;;) {
 		spin_lock_irqsave(session->host->host_lock, flags);
-		if (!session->host->host_busy) { /* OK for ERL == 0 */
+		if (!atomic_read(&session->host->host_busy)) { /* OK for ERL == 0 */
 			spin_unlock_irqrestore(session->host->host_lock, flags);
 			break;
 		}
@@ -2930,7 +2930,7 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
 		msleep_interruptible(500);
 		iscsi_conn_printk(KERN_INFO, conn, "iscsi conn_destroy(): "
 				  "host_busy %d host_failed %d\n",
-				  session->host->host_busy,
+				  atomic_read(&session->host->host_busy),
 				  session->host->host_failed);
 		/*
 		 * force eh_abort() to unblock
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index da3aee1..9423009 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -812,7 +812,7 @@ retry:
 	spin_unlock_irq(shost->host_lock);
 
 	SAS_DPRINTK("Enter %s busy: %d failed: %d\n",
-		    __func__, shost->host_busy, shost->host_failed);
+		    __func__, atomic_read(&shost->host_busy), shost->host_failed);
 	/*
 	 * Deal with commands that still have SAS tasks (i.e. they didn't
 	 * complete via the normal sas_task completion mechanism),
@@ -857,7 +857,8 @@ out:
 		goto retry;
 
 	SAS_DPRINTK("--- Exit %s: busy: %d failed: %d tries: %d\n",
-		    __func__, shost->host_busy, shost->host_failed, tries);
+		    __func__, atomic_read(&shost->host_busy),
+		    shost->host_failed, tries);
 }
 
 enum blk_eh_timer_return sas_scsi_timed_out(struct scsi_cmnd *cmd)
diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index 6d48d30..740ae49 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -959,7 +959,7 @@ static inline void update_can_queue(struct Scsi_Host *host, u_int in_ptr, u_int
 	/* Temporary workaround until bug is found and fixed (one bug has been found
 	   already, but fixing it makes things even worse) -jj */
 	int num_free = QLOGICPTI_REQ_QUEUE_LEN - REQ_QUEUE_DEPTH(in_ptr, out_ptr) - 64;
-	host->can_queue = host->host_busy + num_free;
+	host->can_queue = atomic_read(&host->host_busy) + num_free;
 	host->sg_tablesize = QLOGICPTI_MAX_SG(num_free);
 }
 
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index eff189f..a0b5e6f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -596,7 +596,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
 			if (level > 3)
 				scmd_printk(KERN_INFO, cmd,
 					    "scsi host busy %d failed %d\n",
-					    cmd->device->host->host_busy,
+					    atomic_read(&cmd->device->host->host_busy),
 					    cmd->device->host->host_failed);
 		}
 	}
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 771c16b..759c168 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -59,7 +59,7 @@ static int scsi_try_to_abort_cmd(struct scsi_host_template *,
 /* called with shost->host_lock held */
 void scsi_eh_wakeup(struct Scsi_Host *shost)
 {
-	if (shost->host_busy == shost->host_failed) {
+	if (atomic_read(&shost->host_busy) == shost->host_failed) {
 		trace_scsi_eh_wakeup(shost);
 		wake_up_process(shost->ehandler);
 		SCSI_LOG_ERROR_RECOVERY(5,
@@ -2141,7 +2141,7 @@ int scsi_error_handler(void *data)
 	while (!kthread_should_stop()) {
 		set_current_state(TASK_INTERRUPTIBLE);
 		if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) ||
-		    shost->host_failed != shost->host_busy) {
+		    shost->host_failed != atomic_read(&shost->host_busy)) {
 			SCSI_LOG_ERROR_RECOVERY(1,
 				printk("scsi_eh_%d: sleeping\n",
 					shost->host_no));
@@ -2153,7 +2153,7 @@ int scsi_error_handler(void *data)
 		SCSI_LOG_ERROR_RECOVERY(1,
 			printk("scsi_eh_%d: waking up %d/%d/%d\n",
 			       shost->host_no, shost->host_eh_scheduled,
-			       shost->host_failed, shost->host_busy));
+			       shost->host_failed, atomic_read(&shost->host_busy)));
 
 		/*
 		 * We have a host that is failing for some reason.  Figure out
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 01a631e..f1ce764 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -281,14 +281,17 @@ void scsi_device_unbusy(struct scsi_device *sdev)
 	struct scsi_target *starget = scsi_target(sdev);
 	unsigned long flags;
 
-	spin_lock_irqsave(shost->host_lock, flags);
-	shost->host_busy--;
+	atomic_dec(&shost->host_busy);
 	atomic_dec(&starget->target_busy);
+
 	if (unlikely(scsi_host_in_recovery(shost) &&
-		     (shost->host_failed || shost->host_eh_scheduled)))
+		     (shost->host_failed || shost->host_eh_scheduled))) {
+		spin_lock_irqsave(shost->host_lock, flags);
 		scsi_eh_wakeup(shost);
-	spin_unlock(shost->host_lock);
-	spin_lock(sdev->request_queue->queue_lock);
+		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);
 }
@@ -356,7 +359,8 @@ static inline int scsi_target_is_busy(struct scsi_target *starget)
 
 static inline int scsi_host_is_busy(struct Scsi_Host *shost)
 {
-	if ((shost->can_queue > 0 && shost->host_busy >= shost->can_queue) ||
+	if ((shost->can_queue > 0 &&
+	     atomic_read(&shost->host_busy) >= shost->can_queue) ||
 	    shost->host_blocked || shost->host_self_blocked)
 		return 1;
 
@@ -1332,38 +1336,51 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
 				   struct Scsi_Host *shost,
 				   struct scsi_device *sdev)
 {
-	int ret = 0;
-
-	spin_lock_irq(shost->host_lock);
+	unsigned int busy;
 
 	if (scsi_host_in_recovery(shost))
-		goto out;
-	if (shost->host_busy == 0 && shost->host_blocked) {
+		return 0;
+
+	busy = atomic_inc_return(&shost->host_busy) - 1;
+	if (busy == 0 && shost->host_blocked) {
 		/*
 		 * unblock after host_blocked iterates to zero
 		 */
-		if (--shost->host_blocked != 0)
-			goto out;
+		spin_lock_irq(shost->host_lock);
+		if (--shost->host_blocked != 0) {
+			spin_unlock_irq(shost->host_lock);
+			goto out_dec;
+		}
+		spin_unlock_irq(shost->host_lock);
 
 		SCSI_LOG_MLQUEUE(3,
 			printk("scsi%d unblocking host at zero depth\n",
 				shost->host_no));
 	}
-	if (scsi_host_is_busy(shost)) {
-		if (list_empty(&sdev->starved_entry))
-			list_add_tail(&sdev->starved_entry, &shost->starved_list);
-		goto out;
-	}
+
+	if (shost->can_queue > 0 && busy >= shost->can_queue)
+		goto starved;
+	if (shost->host_blocked || shost->host_self_blocked)
+		goto starved;
 
 	/* We're OK to process the command, so we can't be starved */
-	if (!list_empty(&sdev->starved_entry))
-		list_del_init(&sdev->starved_entry);
+	if (!list_empty(&sdev->starved_entry)) {
+		spin_lock_irq(shost->host_lock);
+		if (!list_empty(&sdev->starved_entry))
+			list_del_init(&sdev->starved_entry);
+		spin_unlock_irq(shost->host_lock);
+	}
+
+	return 1;
 
-	shost->host_busy++;
-	ret = 1;
-out:
+starved:
+	spin_lock_irq(shost->host_lock);
+	if (list_empty(&sdev->starved_entry))
+		list_add_tail(&sdev->starved_entry, &shost->starved_list);
 	spin_unlock_irq(shost->host_lock);
-	return ret;
+out_dec:
+	atomic_dec(&shost->host_busy);
+	return 0;
 }
 
 /*
@@ -1427,12 +1444,8 @@ static void scsi_kill_request(struct request *req, struct request_queue *q)
 	 * with the locks as normal issue path does.
 	 */
 	sdev->device_busy++;
-	spin_unlock(sdev->request_queue->queue_lock);
-	spin_lock(shost->host_lock);
-	shost->host_busy++;
+	atomic_inc(&shost->host_busy);
 	atomic_inc(&starget->target_busy);
-	spin_unlock(shost->host_lock);
-	spin_lock(sdev->request_queue->queue_lock);
 
 	blk_complete_request(req);
 }
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 9117d0b..658ee82 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -332,7 +332,6 @@ store_shost_eh_deadline(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR(eh_deadline, S_IRUGO | S_IWUSR, show_shost_eh_deadline, store_shost_eh_deadline);
 
 shost_rd_attr(unique_id, "%u\n");
-shost_rd_attr(host_busy, "%hu\n");
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(can_queue, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
@@ -342,6 +341,14 @@ shost_rd_attr(prot_capabilities, "%u\n");
 shost_rd_attr(prot_guard_type, "%hd\n");
 shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
 
+static ssize_t
+show_host_busy(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+	return snprintf(buf, 20, "%hu\n", atomic_read(&shost->host_busy));
+}
+static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL);
+
 static struct attribute *scsi_sysfs_shost_attrs[] = {
 	&dev_attr_unique_id.attr,
 	&dev_attr_host_busy.attr,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 94844fc..590e1af 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -603,13 +603,9 @@ struct Scsi_Host {
 	 */
 	struct blk_queue_tag	*bqt;
 
-	/*
-	 * The following two fields are protected with host_lock;
-	 * however, eh routines can safely access during eh processing
-	 * without acquiring the lock.
-	 */
-	unsigned int host_busy;		   /* commands actually active on low-level */
-	unsigned int host_failed;	   /* commands that failed. */
+	atomic_t host_busy;		   /* commands actually active on low-level */
+	unsigned int host_failed;	   /* commands that failed.
+					      protected by host_lock */
 	unsigned int host_eh_scheduled;    /* EH scheduled without command */
     
 	unsigned int host_no;  /* Used for IOCTL_GET_IDLUN, /proc/scsi et al. */
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 22/39] scsi: convert device_busy to atomic_t
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (20 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 21/39] scsi: convert host_busy to atomic_t Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 23/39] scsi: fix the {host,target,device}_blocked counter mess Christoph Hellwig
                   ` (17 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0022-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 f1ce764..6281595 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;
 }
 
@@ -1223,7 +1222,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:
@@ -1254,26 +1253,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.
@@ -1443,7 +1448,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);
 
@@ -1522,7 +1527,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))) {
@@ -1532,13 +1537,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;
@@ -1604,9 +1610,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);
 }
 
@@ -2345,7 +2351,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



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 23/39] scsi: fix the {host,target,device}_blocked counter mess
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (21 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 22/39] scsi: convert device_busy " Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 24/39] blk-mq: add blk_mq_requeue_request Christoph Hellwig
                   ` (16 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- 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



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 24/39] blk-mq: add blk_mq_requeue_request
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (22 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 23/39] scsi: fix the {host,target,device}_blocked counter mess Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 25/39] blk-mq: add async paramter to blk_mq_start_stopped_hw_queues Christoph Hellwig
                   ` (15 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0024-blk-mq-add-blk_mq_requeue_request.patch --]
[-- Type: text/plain, Size: 2112 bytes --]

This allows to requeue a request that has been accepted by ->queue_rq
earlier.  This is needed by the SCSI layer in various error conditions.

The existing internal blk_mq_requeue_request is renamed to
__blk_mq_requeue_request as it is a lower level building block for this
funtionality.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c         |   18 ++++++++++++++++--
 include/linux/blk-mq.h |    2 ++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c7e723e..9e13761 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -380,7 +380,7 @@ static void blk_mq_start_request(struct request *rq, bool last)
 		rq->cmd_flags |= REQ_END;
 }
 
-static void blk_mq_requeue_request(struct request *rq)
+static void __blk_mq_requeue_request(struct request *rq)
 {
 	struct request_queue *q = rq->q;
 
@@ -393,6 +393,20 @@ static void blk_mq_requeue_request(struct request *rq)
 		rq->nr_phys_segments--;
 }
 
+/* XXX: misnamed, only unpreps but leaves the actual requeue to the caller. */
+void blk_mq_requeue_request(struct request *rq)
+{
+	struct request_queue *q = rq->q;
+
+	__blk_mq_requeue_request(rq);
+	blk_clear_rq_complete(rq);
+
+	trace_block_rq_requeue(q, rq);
+
+	BUG_ON(blk_queued_rq(rq));
+}
+EXPORT_SYMBOL(blk_mq_requeue_request);
+
 struct blk_mq_timeout_data {
 	struct blk_mq_hw_ctx *hctx;
 	unsigned long *next;
@@ -573,7 +587,7 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 			 * time
 			 */
 			list_add(&rq->queuelist, &rq_list);
-			blk_mq_requeue_request(rq);
+			__blk_mq_requeue_request(rq);
 			break;
 		default:
 			pr_err("blk-mq: bad return on queue: %d\n", ret);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index cac10e1..6325a24 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -148,6 +148,8 @@ static inline void blk_mq_end_io(struct request *rq, int error)
 	BUG_ON(!done);
 }
 
+void blk_mq_requeue_request(struct request *rq);
+
 void blk_mq_complete_request(struct request *rq);
 
 void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 25/39] blk-mq: add async paramter to blk_mq_start_stopped_hw_queues
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (23 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 24/39] blk-mq: add blk_mq_requeue_request Christoph Hellwig
@ 2014-03-17 13:27 ` Christoph Hellwig
  2014-03-17 13:27 ` [PATCH 26/39] HACK: support blk_delay_queue for blk-mq Christoph Hellwig
                   ` (14 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0025-blk-mq-add-async-paramter-to-blk_mq_start_stopped_hw.patch --]
[-- Type: text/plain, Size: 2311 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c             |    4 ++--
 drivers/block/virtio_blk.c |    4 ++--
 include/linux/blk-mq.h     |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9e13761..6f04dda 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -671,7 +671,7 @@ void blk_mq_start_hw_queue(struct blk_mq_hw_ctx *hctx)
 }
 EXPORT_SYMBOL(blk_mq_start_hw_queue);
 
-void blk_mq_start_stopped_hw_queues(struct request_queue *q)
+void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async)
 {
 	struct blk_mq_hw_ctx *hctx;
 	int i;
@@ -681,7 +681,7 @@ void blk_mq_start_stopped_hw_queues(struct request_queue *q)
 			continue;
 
 		clear_bit(BLK_MQ_S_STOPPED, &hctx->state);
-		blk_mq_run_hw_queue(hctx, true);
+		blk_mq_run_hw_queue(hctx, async);
 	}
 }
 EXPORT_SYMBOL(blk_mq_start_stopped_hw_queues);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index a2e925b..3232b83 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -148,7 +148,7 @@ static void virtblk_done(struct virtqueue *vq)
 
 	/* In case queue is stopped waiting for more buffers. */
 	if (req_done)
-		blk_mq_start_stopped_hw_queues(vblk->disk->queue);
+		blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
 }
 
 static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
@@ -734,7 +734,7 @@ static int virtblk_restore(struct virtio_device *vdev)
 	vblk->config_enable = true;
 	ret = init_vq(vdev->priv);
 	if (!ret)
-		blk_mq_start_stopped_hw_queues(vblk->disk->queue);
+		blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
 
 	return ret;
 }
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 6325a24..4d34957 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -155,7 +155,7 @@ void blk_mq_complete_request(struct request *rq);
 void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx);
 void blk_mq_start_hw_queue(struct blk_mq_hw_ctx *hctx);
 void blk_mq_stop_hw_queues(struct request_queue *q);
-void blk_mq_start_stopped_hw_queues(struct request_queue *q);
+void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);
 
 /*
  * Driver command data is immediately after the request. So subtract request
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 26/39] HACK: support blk_delay_queue for blk-mq
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (24 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 27/39] blk-mq: export blk_mq_insert_request Christoph Hellwig
                   ` (13 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:27 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0026-HACK-support-blk_delay_queue-for-blk-mq.patch --]
[-- Type: text/plain, Size: 715 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 4cd5ffc..47af781 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -163,9 +163,14 @@ static void blk_delay_work(struct work_struct *work)
 	struct request_queue *q;
 
 	q = container_of(work, struct request_queue, delay_work.work);
-	spin_lock_irq(q->queue_lock);
-	__blk_run_queue(q);
-	spin_unlock_irq(q->queue_lock);
+
+	if (q->mq_ops) {
+		blk_mq_start_stopped_hw_queues(q, false);
+	} else {
+		spin_lock_irq(q->queue_lock);
+		__blk_run_queue(q);
+		spin_unlock_irq(q->queue_lock);
+	}
 }
 
 /**
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 27/39] blk-mq: export blk_mq_insert_request
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (25 preceding siblings ...)
  2014-03-17 13:27 ` [PATCH 26/39] HACK: support blk_delay_queue for blk-mq Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 28/39] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
                   ` (12 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0027-blk-mq-export-blk_mq_insert_request.patch --]
[-- Type: text/plain, Size: 658 bytes --]

We'll need this for now when scsi is built as a module.  Eventually
I hope to be able to move it to a higher level API intead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6f04dda..4408c70 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -740,6 +740,7 @@ void blk_mq_insert_request(struct request *rq, bool at_head, bool run_queue,
 	if (run_queue)
 		blk_mq_run_hw_queue(hctx, async);
 }
+EXPORT_SYMBOL(blk_mq_insert_request);
 
 static void blk_mq_insert_requests(struct request_queue *q,
 				     struct blk_mq_ctx *ctx,
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 28/39] scsi: reintroduce scsi_driver.init_command
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (26 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 27/39] blk-mq: export blk_mq_insert_request Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-26  9:46   ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 29/39] block: remove unprep_rq_fn Christoph Hellwig
                   ` (11 subsequent siblings)
  39 siblings, 1 reply; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0028-scsi-reintroduce-scsi_driver.init_command.patch --]
[-- Type: text/plain, Size: 11749 bytes --]

Instead of letting the ULD play games with the prep_fn move back to
the model of a central prep_fn with a callback to the ULD.  This
already cleans up and shortens the code by itself, and will be required
to properly support blk-mq in the SCSI midlayer.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++------------------
 drivers/scsi/sd.c          |   46 +++++++++++-------------------
 drivers/scsi/sr.c          |   19 ++++---------
 include/scsi/scsi_driver.h |    8 ++----
 4 files changed, 63 insertions(+), 76 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a23e8c3..9889c75 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -470,6 +470,16 @@ void scsi_requeue_run_queue(struct work_struct *work)
 	scsi_run_queue(q);
 }
 
+static void scsi_uninit_command(struct scsi_cmnd *cmd)
+{
+	if (cmd->request->cmd_type == REQ_TYPE_FS) {
+		struct scsi_driver *drv = scsi_cmd_to_driver(cmd);
+
+		if (drv->uninit_command)
+			drv->uninit_command(cmd);
+	}
+}
+
 /*
  * Function:	scsi_requeue_command()
  *
@@ -494,6 +504,8 @@ static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
 	struct request *req = cmd->request;
 	unsigned long flags;
 
+	scsi_uninit_command(cmd);
+
 	spin_lock_irqsave(q->queue_lock, flags);
 	blk_unprep_request(req);
 	req->special = NULL;
@@ -1078,15 +1090,7 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
 
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
 {
-	struct scsi_cmnd *cmd;
-	int ret = scsi_prep_state_check(sdev, req);
-
-	if (ret != BLKPREP_OK)
-		return ret;
-
-	cmd = scsi_get_cmd_from_req(sdev, req);
-	if (unlikely(!cmd))
-		return BLKPREP_DEFER;
+	struct scsi_cmnd *cmd = req->special;
 
 	/*
 	 * BLOCK_PC requests may transfer data, in which case they must
@@ -1130,15 +1134,11 @@ EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
  */
 int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 {
-	struct scsi_cmnd *cmd;
-	int ret = scsi_prep_state_check(sdev, req);
-
-	if (ret != BLKPREP_OK)
-		return ret;
+	struct scsi_cmnd *cmd = req->special;
 
 	if (unlikely(sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh
 			 && sdev->scsi_dh_data->scsi_dh->prep_fn)) {
-		ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
+		int ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
 		if (ret != BLKPREP_OK)
 			return ret;
 	}
@@ -1148,16 +1148,13 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 	 */
 	BUG_ON(!req->nr_phys_segments);
 
-	cmd = scsi_get_cmd_from_req(sdev, req);
-	if (unlikely(!cmd))
-		return BLKPREP_DEFER;
-
 	memset(cmd->cmnd, 0, BLK_MAX_CDB);
 	return scsi_init_io(cmd, GFP_ATOMIC);
 }
 EXPORT_SYMBOL(scsi_setup_fs_cmnd);
 
-int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
+static int
+scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 {
 	int ret = BLKPREP_OK;
 
@@ -1209,9 +1206,9 @@ int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 	}
 	return ret;
 }
-EXPORT_SYMBOL(scsi_prep_state_check);
 
-int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
+static int
+scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 {
 	struct scsi_device *sdev = q->queuedata;
 
@@ -1242,18 +1239,33 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 
 	return ret;
 }
-EXPORT_SYMBOL(scsi_prep_return);
 
-int scsi_prep_fn(struct request_queue *q, struct request *req)
+static int scsi_prep_fn(struct request_queue *q, struct request *req)
 {
 	struct scsi_device *sdev = q->queuedata;
-	int ret = BLKPREP_KILL;
+	struct scsi_cmnd *cmd;
+	int ret;
 
-	if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+	ret = scsi_prep_state_check(sdev, req);
+	if (ret != BLKPREP_OK)
+		goto out;
+
+	cmd = scsi_get_cmd_from_req(sdev, req);
+	if (unlikely(!cmd)) {
+		ret = BLKPREP_DEFER;
+		goto out;
+	}
+
+	if (req->cmd_type == REQ_TYPE_FS)
+		ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
+	else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
 		ret = scsi_setup_blk_pc_cmnd(sdev, req);
+	else
+		ret = BLKPREP_KILL;
+
+out:
 	return scsi_prep_return(q, req, ret);
 }
-EXPORT_SYMBOL(scsi_prep_fn);
 
 /*
  * scsi_dev_queue_ready: if we can send requests to sdev, return 1 else
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 470954a..33e349b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -109,6 +109,8 @@ static int sd_suspend_system(struct device *);
 static int sd_suspend_runtime(struct device *);
 static int sd_resume(struct device *);
 static void sd_rescan(struct device *);
+static int sd_init_command(struct scsi_cmnd *SCpnt);
+static void sd_uninit_command(struct scsi_cmnd *SCpnt);
 static int sd_done(struct scsi_cmnd *);
 static int sd_eh_action(struct scsi_cmnd *, int);
 static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
@@ -503,6 +505,8 @@ static struct scsi_driver sd_template = {
 		.pm		= &sd_pm_ops,
 	},
 	.rescan			= sd_rescan,
+	.init_command		= sd_init_command,
+	.uninit_command		= sd_uninit_command,
 	.done			= sd_done,
 	.eh_action		= sd_eh_action,
 };
@@ -838,9 +842,9 @@ static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
 	return scsi_setup_blk_pc_cmnd(sdp, rq);
 }
 
-static void sd_unprep_fn(struct request_queue *q, struct request *rq)
+static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 {
-	struct scsi_cmnd *SCpnt = rq->special;
+	struct request *rq = SCpnt->request;
 
 	if (rq->cmd_flags & REQ_DISCARD) {
 		free_page((unsigned long)rq->buffer);
@@ -853,18 +857,10 @@ static void sd_unprep_fn(struct request_queue *q, struct request *rq)
 	}
 }
 
-/**
- *	sd_prep_fn - build a scsi (read or write) command from
- *	information in the request structure.
- *	@SCpnt: pointer to mid-level's per scsi command structure that
- *	contains request and into which the scsi command is written
- *
- *	Returns 1 if successful and 0 if error (or cannot be done now).
- **/
-static int sd_prep_fn(struct request_queue *q, struct request *rq)
+static int sd_init_command(struct scsi_cmnd *SCpnt)
 {
-	struct scsi_cmnd *SCpnt;
-	struct scsi_device *sdp = q->queuedata;
+	struct request *rq = SCpnt->request;
+	struct scsi_device *sdp = SCpnt->device;
 	struct gendisk *disk = rq->rq_disk;
 	struct scsi_disk *sdkp;
 	sector_t block = blk_rq_pos(rq);
@@ -886,12 +882,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	} else if (rq->cmd_flags & REQ_FLUSH) {
 		ret = scsi_setup_flush_cmnd(sdp, rq);
 		goto out;
-	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
-		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
-		goto out;
-	} else if (rq->cmd_type != REQ_TYPE_FS) {
-		ret = BLKPREP_KILL;
-		goto out;
 	}
 	ret = scsi_setup_fs_cmnd(sdp, rq);
 	if (ret != BLKPREP_OK)
@@ -903,11 +893,10 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	 * is used for a killable error condition */
 	ret = BLKPREP_KILL;
 
-	SCSI_LOG_HLQUEUE(1, scmd_printk(KERN_INFO, SCpnt,
-					"sd_prep_fn: block=%llu, "
-					"count=%d\n",
-					(unsigned long long)block,
-					this_count));
+	SCSI_LOG_HLQUEUE(1,
+		scmd_printk(KERN_INFO, SCpnt,
+			"%s: block=%llu, count=%d\n",
+			__func__, (unsigned long long)block, this_count));
 
 	if (!sdp || !scsi_device_online(sdp) ||
 	    block + blk_rq_sectors(rq) > get_capacity(disk)) {
@@ -1127,7 +1116,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	 */
 	ret = BLKPREP_OK;
  out:
-	return scsi_prep_return(q, rq, ret);
+ 	return ret;
 }
 
 /**
@@ -1663,6 +1652,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
 	unsigned char op = SCpnt->cmnd[0];
 	unsigned char unmap = SCpnt->cmnd[1] & 8;
 
+	sd_uninit_command(SCpnt);
+
 	if (req->cmd_flags & REQ_DISCARD || req->cmd_flags & REQ_WRITE_SAME) {
 		if (!result) {
 			good_bytes = blk_rq_bytes(req);
@@ -2872,9 +2863,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_revalidate_disk(gd);
 
-	blk_queue_prep_rq(sdp->request_queue, sd_prep_fn);
-	blk_queue_unprep_rq(sdp->request_queue, sd_unprep_fn);
-
 	gd->driverfs_dev = &sdp->sdev_gendev;
 	gd->flags = GENHD_FL_EXT_DEVT;
 	if (sdp->removable) {
@@ -3021,8 +3009,6 @@ static int sd_remove(struct device *dev)
 	scsi_autopm_get_device(sdkp->device);
 
 	async_synchronize_full_domain(&scsi_sd_probe_domain);
-	blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
-	blk_queue_unprep_rq(sdkp->device->request_queue, NULL);
 	device_del(&sdkp->dev);
 	del_gendisk(sdkp->disk);
 	sd_shutdown(dev);
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 40d8592..93cbd36 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -79,6 +79,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
 static DEFINE_MUTEX(sr_mutex);
 static int sr_probe(struct device *);
 static int sr_remove(struct device *);
+static int sr_init_command(struct scsi_cmnd *SCpnt);
 static int sr_done(struct scsi_cmnd *);
 static int sr_runtime_suspend(struct device *dev);
 
@@ -94,6 +95,7 @@ static struct scsi_driver sr_template = {
 		.remove		= sr_remove,
 		.pm		= &sr_pm_ops,
 	},
+	.init_command		= sr_init_command,
 	.done			= sr_done,
 };
 
@@ -378,21 +380,14 @@ static int sr_done(struct scsi_cmnd *SCpnt)
 	return good_bytes;
 }
 
-static int sr_prep_fn(struct request_queue *q, struct request *rq)
+static int sr_init_command(struct scsi_cmnd *SCpnt)
 {
 	int block = 0, this_count, s_size;
 	struct scsi_cd *cd;
-	struct scsi_cmnd *SCpnt;
-	struct scsi_device *sdp = q->queuedata;
+	struct request *rq = SCpnt->request;
+	struct scsi_device *sdp = SCpnt->device;
 	int ret;
 
-	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
-		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
-		goto out;
-	} else if (rq->cmd_type != REQ_TYPE_FS) {
-		ret = BLKPREP_KILL;
-		goto out;
-	}
 	ret = scsi_setup_fs_cmnd(sdp, rq);
 	if (ret != BLKPREP_OK)
 		goto out;
@@ -517,7 +512,7 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)
 	 */
 	ret = BLKPREP_OK;
  out:
-	return scsi_prep_return(q, rq, ret);
+	return ret;
 }
 
 static int sr_block_open(struct block_device *bdev, fmode_t mode)
@@ -718,7 +713,6 @@ static int sr_probe(struct device *dev)
 
 	/* FIXME: need to handle a get_capabilities failure properly ?? */
 	get_capabilities(cd);
-	blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
 	sr_vendor_init(cd);
 
 	disk->driverfs_dev = &sdev->sdev_gendev;
@@ -993,7 +987,6 @@ static int sr_remove(struct device *dev)
 
 	scsi_autopm_get_device(cd->device);
 
-	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
 	del_gendisk(cd->disk);
 
 	mutex_lock(&sr_ref_mutex);
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 20fdfc2..b507729 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -6,15 +6,14 @@
 struct module;
 struct scsi_cmnd;
 struct scsi_device;
-struct request;
-struct request_queue;
-
 
 struct scsi_driver {
 	struct module		*owner;
 	struct device_driver	gendrv;
 
 	void (*rescan)(struct device *);
+	int (*init_command)(struct scsi_cmnd *);
+	void (*uninit_command)(struct scsi_cmnd *);
 	int (*done)(struct scsi_cmnd *);
 	int (*eh_action)(struct scsi_cmnd *, int);
 };
@@ -31,8 +30,5 @@ extern int scsi_register_interface(struct class_interface *);
 
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
 int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req);
-int scsi_prep_state_check(struct scsi_device *sdev, struct request *req);
-int scsi_prep_return(struct request_queue *q, struct request *req, int ret);
-int scsi_prep_fn(struct request_queue *, struct request *);
 
 #endif /* _SCSI_SCSI_DRIVER_H */
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 29/39] block: remove unprep_rq_fn
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (27 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 28/39] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 30/39] scsi: centralize command re-queueing in scsi_dispatch_fn Christoph Hellwig
                   ` (10 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0029-block-remove-unprep_rq_fn.patch --]
[-- Type: text/plain, Size: 3764 bytes --]

Now that scsi doesn't use it anymore there's no user left.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c       |   11 ++---------
 block/blk-settings.c   |   17 -----------------
 include/linux/blkdev.h |    3 ---
 3 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 47af781..d58f984 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -722,7 +722,6 @@ blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
 
 	q->request_fn		= rfn;
 	q->prep_rq_fn		= NULL;
-	q->unprep_rq_fn		= NULL;
 	q->queue_flags		|= QUEUE_FLAG_DEFAULT;
 
 	/* Override internal queue lock with supplied lock pointer */
@@ -2486,18 +2485,12 @@ static bool blk_update_bidi_request(struct request *rq, int error,
  * @req:	the request
  *
  * This function makes a request ready for complete resubmission (or
- * completion).  It happens only after all error handling is complete,
- * so represents the appropriate moment to deallocate any resources
- * that were allocated to the request in the prep_rq_fn.  The queue
- * lock is held when calling this.
+ * completion).  It happens only after all error handling is complete.
+ * The queue lock is held when calling this.
  */
 void blk_unprep_request(struct request *req)
 {
-	struct request_queue *q = req->q;
-
 	req->cmd_flags &= ~REQ_DONTPREP;
-	if (q->unprep_rq_fn)
-		q->unprep_rq_fn(q, req);
 }
 EXPORT_SYMBOL_GPL(blk_unprep_request);
 
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 5d21239..47a266c 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -37,23 +37,6 @@ void blk_queue_prep_rq(struct request_queue *q, prep_rq_fn *pfn)
 EXPORT_SYMBOL(blk_queue_prep_rq);
 
 /**
- * blk_queue_unprep_rq - set an unprepare_request function for queue
- * @q:		queue
- * @ufn:	unprepare_request function
- *
- * It's possible for a queue to register an unprepare_request callback
- * which is invoked before the request is finally completed. The goal
- * of the function is to deallocate any data that was allocated in the
- * prepare_request callback.
- *
- */
-void blk_queue_unprep_rq(struct request_queue *q, unprep_rq_fn *ufn)
-{
-	q->unprep_rq_fn = ufn;
-}
-EXPORT_SYMBOL(blk_queue_unprep_rq);
-
-/**
  * blk_queue_merge_bvec - set a merge_bvec function for queue
  * @q:		queue
  * @mbfn:	merge_bvec_fn
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4afa4f8..fc010ff 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -223,7 +223,6 @@ struct blk_queue_ctx;
 typedef void (request_fn_proc) (struct request_queue *q);
 typedef void (make_request_fn) (struct request_queue *q, struct bio *bio);
 typedef int (prep_rq_fn) (struct request_queue *, struct request *);
-typedef void (unprep_rq_fn) (struct request_queue *, struct request *);
 
 struct bio_vec;
 struct bvec_merge_data {
@@ -312,7 +311,6 @@ struct request_queue {
 	request_fn_proc		*request_fn;
 	make_request_fn		*make_request_fn;
 	prep_rq_fn		*prep_rq_fn;
-	unprep_rq_fn		*unprep_rq_fn;
 	merge_bvec_fn		*merge_bvec_fn;
 	softirq_done_fn		*softirq_done_fn;
 	rq_timed_out_fn		*rq_timed_out_fn;
@@ -985,7 +983,6 @@ extern int blk_queue_dma_drain(struct request_queue *q,
 extern void blk_queue_lld_busy(struct request_queue *q, lld_busy_fn *fn);
 extern void blk_queue_segment_boundary(struct request_queue *, unsigned long);
 extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
-extern void blk_queue_unprep_rq(struct request_queue *, unprep_rq_fn *ufn);
 extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
 extern void blk_queue_dma_alignment(struct request_queue *, int);
 extern void blk_queue_update_dma_alignment(struct request_queue *, int);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 30/39] scsi: centralize command re-queueing in scsi_dispatch_fn
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (28 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 29/39] block: remove unprep_rq_fn Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 31/39] scsi: split __scsi_queue_insert Christoph Hellwig
                   ` (9 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0030-scsi-centralize-command-re-queueing-in-scsi_dispatch.patch --]
[-- Type: text/plain, Size: 3248 bytes --]

Make sure we only have the logic for requeing commands in one place.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi.c     |   36 +++++++++++++-----------------------
 drivers/scsi/scsi_lib.c |    9 ++++++---
 2 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 0e8a077..0df3913 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -639,9 +639,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 		 * returns an immediate error upwards, and signals
 		 * that the device is no longer present */
 		cmd->result = DID_NO_CONNECT << 16;
-		scsi_done(cmd);
-		/* return 0 (because the command has been processed) */
-		goto out;
+		goto done;
 	}
 
 	/* Check to see if the scsi lld made this device blocked. */
@@ -653,16 +651,8 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 		 * occur until the device transitions out of the
 		 * suspend state.
 		 */
-
-		scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY);
-
 		SCSI_LOG_MLQUEUE(3, printk("queuecommand : device blocked \n"));
-
-		/*
-		 * NOTE: rtn is still zero here because we don't need the
-		 * queue to be plugged on return (it's already stopped)
-		 */
-		goto out;
+		return SCSI_MLQUEUE_DEVICE_BUSY;
 	}
 
 	/* 
@@ -686,35 +676,35 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 			       "cdb_size=%d host->max_cmd_len=%d\n",
 			       cmd->cmd_len, cmd->device->host->max_cmd_len));
 		cmd->result = (DID_ABORT << 16);
-
-		scsi_done(cmd);
-		goto out;
+		goto done;
 	}
 
 	if (unlikely(host->shost_state == SHOST_DEL)) {
 		cmd->result = (DID_NO_CONNECT << 16);
-		scsi_done(cmd);
-	} else {
-		trace_scsi_dispatch_cmd_start(cmd);
-		cmd->scsi_done = scsi_done;
-		rtn = host->hostt->queuecommand(host, cmd);
+		goto done;
+
 	}
 
+	trace_scsi_dispatch_cmd_start(cmd);
+
+	cmd->scsi_done = scsi_done;
+	rtn = host->hostt->queuecommand(host, cmd);
 	if (rtn) {
 		trace_scsi_dispatch_cmd_error(cmd, rtn);
 		if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
 		    rtn != SCSI_MLQUEUE_TARGET_BUSY)
 			rtn = SCSI_MLQUEUE_HOST_BUSY;
 
-		scsi_queue_insert(cmd, rtn);
-
 		SCSI_LOG_MLQUEUE(3,
 		    printk("queuecommand : request rejected\n"));
 	}
 
- out:
 	SCSI_LOG_MLQUEUE(3, printk("leaving scsi_dispatch_cmnd()\n"));
 	return rtn;
+ done:
+	SCSI_LOG_MLQUEUE(3, printk("scsi_dispatch_cmnd() failed\n"));
+	scsi_done(cmd);
+	return 0;
 }
 
 /**
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9889c75..53d06e8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1598,9 +1598,12 @@ static void scsi_request_fn(struct request_queue *q)
 		 * Dispatch the command to the low-level driver.
 		 */
 		rtn = scsi_dispatch_cmd(cmd);
-		spin_lock_irq(q->queue_lock);
-		if (rtn)
+		if (rtn) {
+			scsi_queue_insert(cmd, rtn);
+			spin_lock_irq(q->queue_lock);
 			goto out_delay;
+		}
+		spin_lock_irq(q->queue_lock);
 	}
 
 	return;
@@ -1620,7 +1623,7 @@ static void scsi_request_fn(struct request_queue *q)
 	blk_requeue_request(q, req);
 	atomic_dec(&sdev->device_busy);
 out_delay:
-	if (atomic_read(&sdev->device_busy) == 0)
+	if (atomic_read(&sdev->device_busy) == 0 && !scsi_device_blocked(sdev))
 		blk_delay_queue(q, SCSI_QUEUE_DELAY);
 }
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 31/39] scsi: split __scsi_queue_insert
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (29 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 30/39] scsi: centralize command re-queueing in scsi_dispatch_fn Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 32/39] scsi: unwind blk_end_request_all and blk_end_request_err calls Christoph Hellwig
                   ` (8 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0031-scsi-split-__scsi_queue_insert.patch --]
[-- Type: text/plain, Size: 2535 bytes --]

Factor out a helper to set the _blocked values, which we'll reuse for the
blk-mq code path.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |   44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 53d06e8..cd82e4c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -75,28 +75,12 @@ struct kmem_cache *scsi_sdb_cache;
  */
 #define SCSI_QUEUE_DELAY	3
 
-/**
- * __scsi_queue_insert - private queue insertion
- * @cmd: The SCSI command being requeued
- * @reason:  The reason for the requeue
- * @unbusy: Whether the queue should be unbusied
- *
- * This is a private queue insertion.  The public interface
- * scsi_queue_insert() always assumes the queue should be unbusied
- * because it's always called before the completion.  This function is
- * for a requeue after completion, which should only occur in this
- * file.
- */
-static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
+static void
+scsi_set_blocked(struct scsi_cmnd *cmd, int reason)
 {
 	struct Scsi_Host *host = cmd->device->host;
 	struct scsi_device *device = cmd->device;
 	struct scsi_target *starget = scsi_target(device);
-	struct request_queue *q = device->request_queue;
-	unsigned long flags;
-
-	SCSI_LOG_MLQUEUE(1,
-		 printk("Inserting command %p into mlqueue\n", cmd));
 
 	/*
 	 * Set the appropriate busy bit for the device/host.
@@ -125,6 +109,30 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
 			   starget->max_target_blocked);
 		break;
 	}
+}
+
+/**
+ * __scsi_queue_insert - private queue insertion
+ * @cmd: The SCSI command being requeued
+ * @reason:  The reason for the requeue
+ * @unbusy: Whether the queue should be unbusied
+ *
+ * This is a private queue insertion.  The public interface
+ * scsi_queue_insert() always assumes the queue should be unbusied
+ * because it's always called before the completion.  This function is
+ * for a requeue after completion, which should only occur in this
+ * file.
+ */
+static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
+{
+	struct scsi_device *device = cmd->device;
+	struct request_queue *q = device->request_queue;
+	unsigned long flags;
+
+	SCSI_LOG_MLQUEUE(1,
+		 printk("Inserting command %p into mlqueue\n", cmd));
+
+	scsi_set_blocked(cmd, reason);
 
 	/*
 	 * Decrement the counters, since these commands are no longer
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 32/39] scsi: unwind blk_end_request_all and blk_end_request_err calls
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (30 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 31/39] scsi: split __scsi_queue_insert Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 33/39] scsi: initial blk-mq support Christoph Hellwig
                   ` (7 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0032-scsi-unwind-blk_end_request_all-and-blk_end_request_.patch --]
[-- Type: text/plain, Size: 1197 bytes --]

Calling blk_end_request directly with the helpers to get the right amount
will make the conversion to blk-mq easier as we only have one place to
switch between the different completion functions.  It also makes the
intention a little more clear to me, but that's a minor side effect.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cd82e4c..94d5893 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -808,7 +808,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	 * Kill remainder if no retrys.
 	 */
 	if (error && scsi_noretry_cmd(cmd)) {
-		blk_end_request_all(req, error);
+		blk_end_request(req, error, blk_rq_bytes(req));
 		goto next_command;
 	}
 
@@ -948,7 +948,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 				scsi_print_sense("", cmd);
 			scsi_print_command(cmd);
 		}
-		if (!blk_end_request_err(req, error))
+		if (!blk_end_request(req, error, blk_rq_err_bytes(req)))
 			goto next_command;
 		/*FALLTHRU*/
 	case ACTION_REPREP:
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 33/39] scsi: initial blk-mq support
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (31 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 34/39] scsi: partially stub out scsi_adjust_queue_depth when using blk-mq Christoph Hellwig
                   ` (6 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0033-scsi-initial-blk-mq-support.patch --]
[-- Type: text/plain, Size: 18879 bytes --]

Add support for using the blk-mq code to submit requests to SCSI
drivers.  There is very little blk-mq specific code, because we
try to push most things out to the block layer.

Based on the earlier scsi-mq prototype by Nicholas Bellinger, although
not a whole lot of actual code is left.

Not-quite-signed-off-yet-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi.c      |   13 +-
 drivers/scsi/scsi_lib.c  |  355 +++++++++++++++++++++++++++++++++++++++++-----
 drivers/scsi/scsi_priv.h |    1 +
 drivers/scsi/scsi_scan.c |    5 +-
 include/scsi/scsi_host.h |    3 +
 5 files changed, 338 insertions(+), 39 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 0df3913..7672371 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -44,6 +44,7 @@
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/blkdev.h>
+#include <linux/blk-mq.h>
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/completion.h>
@@ -462,6 +463,10 @@ int scsi_setup_command_freelist(struct Scsi_Host *shost)
 	spin_lock_init(&shost->free_list_lock);
 	INIT_LIST_HEAD(&shost->free_list);
 
+	/* blk-mq uses a block-level allocator */
+	if (shost->hostt->use_blk_mq)
+		return 0;
+
 	shost->cmd_pool = scsi_get_host_cmd_pool(shost);
 	if (!shost->cmd_pool)
 		return -ENOMEM;
@@ -720,8 +725,14 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
  */
 static void scsi_done(struct scsi_cmnd *cmd)
 {
+	struct request *req = cmd->request;
+
 	trace_scsi_dispatch_cmd_done(cmd);
-	blk_complete_request(cmd->request);
+
+	if (req->mq_ctx)
+		blk_mq_complete_request(req);
+	else
+		blk_complete_request(req);
 }
 
 /**
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 94d5893..836f197 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -20,6 +20,7 @@
 #include <linux/delay.h>
 #include <linux/hardirq.h>
 #include <linux/scatterlist.h>
+#include <linux/blk-mq.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -111,6 +112,21 @@ scsi_set_blocked(struct scsi_cmnd *cmd, int reason)
 	}
 }
 
+static void scsi_mq_requeue_work(struct work_struct *work)
+{
+	struct request *rq = container_of(work, struct request, mq_flush_work);
+	struct scsi_cmnd *cmd = rq->special;
+
+	blk_mq_insert_request(rq, true, true, false);
+	put_device(&cmd->device->sdev_gendev);
+}
+
+static void scsi_mq_requeue_request(struct request *rq)
+{
+	INIT_WORK(&rq->mq_flush_work, scsi_mq_requeue_work);
+	kblockd_schedule_work(rq->q, &rq->mq_flush_work);
+}
+
 /**
  * __scsi_queue_insert - private queue insertion
  * @cmd: The SCSI command being requeued
@@ -147,6 +163,11 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
 	 * lock such that the kblockd_schedule_work() call happens
 	 * before blk_cleanup_queue() finishes.
 	 */
+	if (q->mq_ops) {
+		cmd->request->cmd_flags |= REQ_DONTPREP;
+		scsi_mq_requeue_request(cmd->request);
+		return;
+	}
 	spin_lock_irqsave(q->queue_lock, flags);
 	blk_requeue_request(q, cmd->request);
 	kblockd_schedule_work(q, &device->requeue_work);
@@ -304,6 +325,14 @@ void scsi_device_unbusy(struct scsi_device *sdev)
 	atomic_dec(&sdev->device_busy);
 }
 
+static void __scsi_kick_queue(struct request_queue *q)
+{
+	if (q->mq_ops)
+		blk_mq_start_stopped_hw_queues(q, false);
+	else
+		blk_run_queue(q);
+}
+
 /*
  * Called for single_lun devices on IO completion. Clear starget_sdev_user,
  * and call blk_run_queue for all the scsi_devices on the target -
@@ -328,7 +357,7 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
 	 * but in most cases, we will be first. Ideally, each LU on the
 	 * target would get some limited time or requests on the target.
 	 */
-	blk_run_queue(current_sdev->request_queue);
+	__scsi_kick_queue(current_sdev->request_queue);
 
 	spin_lock_irqsave(shost->host_lock, flags);
 	if (starget->starget_sdev_user)
@@ -341,7 +370,7 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
 			continue;
 
 		spin_unlock_irqrestore(shost->host_lock, flags);
-		blk_run_queue(sdev->request_queue);
+		__scsi_kick_queue(sdev->request_queue);
 		spin_lock_irqsave(shost->host_lock, flags);
 	
 		scsi_device_put(sdev);
@@ -434,7 +463,7 @@ static void scsi_starved_list_run(struct Scsi_Host *shost)
 			continue;
 		spin_unlock_irqrestore(shost->host_lock, flags);
 
-		blk_run_queue(slq);
+		__scsi_kick_queue(slq);
 		blk_put_queue(slq);
 
 		spin_lock_irqsave(shost->host_lock, flags);
@@ -465,7 +494,7 @@ static void scsi_run_queue(struct request_queue *q)
 	if (!list_empty(&sdev->host->starved_list))
 		scsi_starved_list_run(sdev->host);
 
-	blk_run_queue(q);
+	__scsi_kick_queue(q);
 }
 
 void scsi_requeue_run_queue(struct work_struct *work)
@@ -476,6 +505,9 @@ void scsi_requeue_run_queue(struct work_struct *work)
 	sdev = container_of(work, struct scsi_device, requeue_work);
 	q = sdev->request_queue;
 	scsi_run_queue(q);
+
+	if (q->mq_ops)
+		put_device(&sdev->sdev_gendev);
 }
 
 static void scsi_uninit_command(struct scsi_cmnd *cmd)
@@ -545,6 +577,13 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }
 
+static bool scsi_end_request(struct request *req, int error, unsigned int bytes)
+{
+	if (req->mq_ctx)
+		return blk_mq_end_io_partial(req, error, bytes);
+	return blk_end_request(req, error, bytes);
+}
+
 static inline unsigned int scsi_sgtable_index(unsigned short nents)
 {
 	unsigned int index;
@@ -705,14 +744,16 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
  *		   be put back on the queue and retried using the same
  *		   command as before, possibly after a delay.
  *
- *		c) We can call blk_end_request() with -EIO to fail
+ *		c) We can call scsi_end_request() with -EIO to fail
  *		   the remainder of the request.
  */
 void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 {
 	int result = cmd->result;
-	struct request_queue *q = cmd->device->request_queue;
+	struct scsi_device *sdev = cmd->device;
+	struct request_queue *q = sdev->request_queue;
 	struct request *req = cmd->request;
+	bool is_mq = !!q->mq_ops;
 	int error = 0;
 	struct scsi_sense_hdr sshdr;
 	int sense_valid = 0;
@@ -801,14 +842,14 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	/*
 	 * If we finished all bytes in the request we are done now.
 	 */
-	if (!blk_end_request(req, error, good_bytes))
+	if (!scsi_end_request(req, error, good_bytes))
 		goto next_command;
 
 	/*
 	 * Kill remainder if no retrys.
 	 */
 	if (error && scsi_noretry_cmd(cmd)) {
-		blk_end_request(req, error, blk_rq_bytes(req));
+		scsi_end_request(req, error, blk_rq_bytes(req));
 		goto next_command;
 	}
 
@@ -830,11 +871,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	} else if (sense_valid && !sense_deferred) {
 		switch (sshdr.sense_key) {
 		case UNIT_ATTENTION:
-			if (cmd->device->removable) {
+			if (sdev->removable) {
 				/* Detected disc change.  Set a bit
 				 * and quietly refuse further access.
 				 */
-				cmd->device->changed = 1;
+				sdev->changed = 1;
 				description = "Media Changed";
 				action = ACTION_FAIL;
 			} else {
@@ -948,7 +989,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 				scsi_print_sense("", cmd);
 			scsi_print_command(cmd);
 		}
-		if (!blk_end_request(req, error, blk_rq_err_bytes(req)))
+		if (!scsi_end_request(req, error, blk_rq_err_bytes(req)))
 			goto next_command;
 		/*FALLTHRU*/
 	case ACTION_REPREP:
@@ -956,8 +997,14 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 		/* Unprep the request and put it back at the head of the queue.
 		 * A new command will be prepared and issued.
 		 */
-		scsi_release_buffers(cmd);
-		scsi_requeue_command(q, cmd);
+		if (is_mq) {
+			cancel_delayed_work(&cmd->abort_work);
+			scsi_uninit_command(cmd);
+			scsi_mq_requeue_request(req);
+		} else {
+			scsi_release_buffers(cmd);
+			scsi_requeue_command(q, cmd);
+		}
 		break;
 	case ACTION_RETRY:
 		/* Retry the same command immediately */
@@ -971,8 +1018,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	return;
 
 next_command:
-	scsi_release_buffers(cmd);
-	scsi_next_command(cmd);
+	if (is_mq) {
+		kblockd_schedule_work(q, &sdev->requeue_work);
+	} else {
+		scsi_release_buffers(cmd);
+		scsi_next_command(cmd);
+	}
 }
 
 static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb,
@@ -980,12 +1031,17 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb,
 {
 	int count;
 
-	/*
-	 * If sg table allocation fails, requeue request later.
-	 */
-	if (unlikely(scsi_alloc_sgtable(sdb, req->nr_phys_segments,
-					gfp_mask))) {
-		return BLKPREP_DEFER;
+	if (!req->mq_ctx) {
+		/*
+		 * If sg table allocation fails, requeue request later.
+		 */
+		if (unlikely(scsi_alloc_sgtable(sdb, req->nr_phys_segments,
+						gfp_mask)))
+			return BLKPREP_DEFER;
+	} else {
+		BUG_ON(req->nr_phys_segments > SCSI_MAX_SG_SEGMENTS);
+		sdb->table.nents = req->nr_phys_segments;
+		sg_init_table(sdb->table.sgl, sdb->table.nents);
 	}
 
 	req->buffer = NULL;
@@ -1041,9 +1097,11 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
 		BUG_ON(prot_sdb == NULL);
 		ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-		if (scsi_alloc_sgtable(prot_sdb, ivecs, gfp_mask)) {
-			error = BLKPREP_DEFER;
-			goto err_exit;
+		if (!rq->mq_ctx) {
+			if (scsi_alloc_sgtable(prot_sdb, ivecs, gfp_mask)) {
+				error = BLKPREP_DEFER;
+				goto err_exit;
+			}
 		}
 
 		count = blk_rq_map_integrity_sg(rq->q, rq->bio,
@@ -1635,6 +1693,123 @@ out_delay:
 		blk_delay_queue(q, SCSI_QUEUE_DELAY);
 }
 
+static int scsi_mq_prep_fn(struct request *req)
+{
+	struct scsi_cmnd *cmd = req->special;
+	struct scsi_device *sdev = req->q->queuedata;
+	struct Scsi_Host *shost = sdev->host;
+	unsigned char *sense_buf = cmd->sense_buffer;
+	struct scatterlist *sg;
+	int ret;
+
+	memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
+	memset(cmd, 0, sizeof(struct scsi_cmnd));
+
+	cmd->request = req;
+	cmd->device = sdev;
+	cmd->sense_buffer = sense_buf;
+
+	cmd->tag = req->tag;
+	cmd->cmnd = req->cmd;
+	cmd->prot_op = SCSI_PROT_NORMAL;
+
+	INIT_LIST_HEAD(&cmd->list);
+	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
+	cmd->jiffies_at_alloc = jiffies;
+	/* XXX: add to sdev->cmd_list here */
+
+	sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
+	cmd->sdb.table.sgl = sg;
+
+	if (scsi_host_get_prot(shost)) {
+		cmd->prot_sdb = (void *)sg +
+			shost->sg_tablesize * sizeof(struct scatterlist);
+		memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
+
+		cmd->prot_sdb->table.sgl =
+			(struct scatterlist *)(cmd->prot_sdb + 1);
+	}
+
+	ret = scsi_prep_state_check(cmd->device, req);
+	if (ret != BLKPREP_OK)
+		goto out;
+
+	if (req->cmd_type == REQ_TYPE_FS)
+		ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
+	else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+		ret = scsi_setup_blk_pc_cmnd(cmd->device, req);
+	else
+		ret = BLKPREP_KILL;
+
+out:
+	switch (ret) {
+	case BLKPREP_OK:
+		return 0;
+	case BLKPREP_DEFER:
+		return BLK_MQ_RQ_QUEUE_BUSY;
+	default:
+		return BLK_MQ_RQ_QUEUE_ERROR;
+	}
+}
+
+static int scsi_mq_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
+{
+	struct request_queue *q = req->q;
+	struct scsi_device *sdev = q->queuedata;
+	struct Scsi_Host *shost = sdev->host;
+	struct scsi_cmnd *cmd = req->special;
+	int ret = BLK_MQ_RQ_QUEUE_BUSY;
+	int reason;
+
+	if (!get_device(&sdev->sdev_gendev))
+		goto out;
+
+	if (!scsi_dev_queue_ready(q, sdev))
+		goto out_put_device;
+	if (!scsi_target_queue_ready(shost, sdev))
+		goto out_dec_device_busy;
+	if (!scsi_host_queue_ready(q, shost, sdev))
+		goto out_dec_target_busy;
+
+	if (!(req->cmd_flags & REQ_DONTPREP)) {
+		ret = scsi_mq_prep_fn(req);
+		if (ret)
+			goto out_dec_host_busy;
+	}
+	req->cmd_flags &= ~REQ_DONTPREP;
+
+	scsi_init_cmd_errh(cmd);
+
+	reason = scsi_dispatch_cmd(cmd);
+	if (reason) {
+		scsi_set_blocked(cmd, reason);
+		ret = BLK_MQ_RQ_QUEUE_BUSY;
+		goto out_uninit;
+	}
+
+	return BLK_MQ_RQ_QUEUE_OK;
+
+out_uninit:
+	scsi_uninit_command(cmd);
+out_dec_host_busy:
+	cancel_delayed_work(&cmd->abort_work);
+	atomic_dec(&shost->host_busy);
+out_dec_target_busy:
+	atomic_dec(&scsi_target(sdev)->target_busy);
+out_dec_device_busy:
+	atomic_dec(&sdev->device_busy);
+out_put_device:
+	put_device(&sdev->sdev_gendev);
+out:
+	if (ret == BLK_MQ_RQ_QUEUE_BUSY) {
+		blk_mq_stop_hw_queue(hctx);
+		if (atomic_read(&sdev->device_busy) == 0 &&
+		    !scsi_device_blocked(sdev))
+		    	blk_delay_queue(q, SCSI_QUEUE_DELAY);
+	}
+	return ret;
+}
+
 u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 {
 	struct device *host_dev;
@@ -1657,16 +1832,10 @@ u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 }
 EXPORT_SYMBOL(scsi_calculate_bounce_limit);
 
-struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost,
-					 request_fn_proc *request_fn)
+static void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 {
-	struct request_queue *q;
 	struct device *dev = shost->dma_dev;
 
-	q = blk_init_queue(request_fn, NULL);
-	if (!q)
-		return NULL;
-
 	/*
 	 * this limit is imposed by hardware restrictions
 	 */
@@ -1697,7 +1866,17 @@ struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost,
 	 * blk_queue_update_dma_alignment() later.
 	 */
 	blk_queue_dma_alignment(q, 0x03);
+}
 
+struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost,
+					 request_fn_proc *request_fn)
+{
+	struct request_queue *q;
+
+	q = blk_init_queue(request_fn, NULL);
+	if (!q)
+		return NULL;
+	__scsi_init_queue(shost, q);
 	return q;
 }
 EXPORT_SYMBOL(__scsi_alloc_queue);
@@ -1717,6 +1896,100 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
 	return q;
 }
 
+static struct blk_mq_ops scsi_mq_ops = {
+	.queue_rq	= scsi_mq_queue_rq,
+	.map_queue	= blk_mq_map_queue,
+	.alloc_hctx	= blk_mq_alloc_single_hw_queue,
+	.free_hctx	= blk_mq_free_single_hw_queue,
+	.complete	= scsi_softirq_done,
+	.timeout	= scsi_times_out,
+};
+
+struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
+{
+	struct Scsi_Host *shost = sdev->host;
+	struct blk_mq_hw_ctx *hctx;
+	struct request_queue *q;
+	struct request *rq;
+	struct scsi_cmnd *cmd;
+	struct blk_mq_reg reg;
+	int i, j, sgl_size;
+
+	memset(&reg, 0, sizeof(reg));
+	reg.ops = &scsi_mq_ops;
+	reg.queue_depth = shost->cmd_per_lun;
+	if (!reg.queue_depth)
+		reg.queue_depth = 1;
+
+	/* XXX: what to do about chained S/G lists? */
+	if (shost->hostt->sg_tablesize > SCSI_MAX_SG_SEGMENTS)
+		shost->sg_tablesize = SCSI_MAX_SG_SEGMENTS;
+	sgl_size = shost->sg_tablesize * sizeof(struct scatterlist);
+
+	reg.cmd_size = sizeof(struct scsi_cmnd) +
+			sgl_size +
+			shost->hostt->cmd_size;
+	if (scsi_host_get_prot(shost))
+		reg.cmd_size += sizeof(struct scsi_data_buffer) + sgl_size;
+	reg.numa_node = NUMA_NO_NODE;
+	reg.nr_hw_queues = 1;
+	reg.flags = BLK_MQ_F_SHOULD_MERGE;
+
+	q = blk_mq_init_queue(&reg, sdev);
+	if (IS_ERR(q)) {
+		printk("blk_mq_init_queue failed\n");
+		return NULL;
+	}
+
+	sdev->request_queue = q;
+	q->queuedata = sdev;
+
+	__scsi_init_queue(shost, q);
+
+	/*
+	 * XXX: figure out if we can get alignment right to allocate the sense
+	 * buffer with the other chunks of memory.
+	 *
+	 * If not we'll need to find a way to have the blk-mq core call us to
+	 * allocate/free commands so that we can properly clean up the
+	 * allocation instead of leaking it.
+	 */
+	queue_for_each_hw_ctx(q, hctx, i) {
+		for (j = 0; j < hctx->queue_depth; j++) {
+			rq = hctx->rqs[j];
+			cmd = rq->special;
+
+			cmd->sense_buffer = kzalloc_node(SCSI_SENSE_BUFFERSIZE,
+					   GFP_KERNEL, reg.numa_node);
+			if (!cmd->sense_buffer)
+				goto out_free_sense_buffers;
+		}
+	}
+
+	rq = q->flush_rq;
+	cmd = blk_mq_rq_to_pdu(rq);
+
+	cmd->sense_buffer = kzalloc_node(SCSI_SENSE_BUFFERSIZE,
+					   GFP_KERNEL, reg.numa_node);
+	if (!cmd->sense_buffer)
+		goto out_free_sense_buffers;
+
+	return q;
+
+out_free_sense_buffers:
+	queue_for_each_hw_ctx(q, hctx, i) {
+		for (j = 0; j < hctx->queue_depth; j++) {
+			rq = hctx->rqs[j];
+			cmd = rq->special;
+
+			kfree(cmd->sense_buffer);
+		}
+	}
+
+	blk_cleanup_queue(q);
+	return NULL;
+}
+
 /*
  * Function:    scsi_block_requests()
  *
@@ -2462,9 +2735,13 @@ scsi_internal_device_block(struct scsi_device *sdev)
 	 * block layer from calling the midlayer with this device's
 	 * request queue. 
 	 */
-	spin_lock_irqsave(q->queue_lock, flags);
-	blk_stop_queue(q);
-	spin_unlock_irqrestore(q->queue_lock, flags);
+	if (q->mq_ops) {
+		blk_mq_stop_hw_queues(q);
+	} else {
+		spin_lock_irqsave(q->queue_lock, flags);
+		blk_stop_queue(q);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+	}
 
 	return 0;
 }
@@ -2510,9 +2787,13 @@ scsi_internal_device_unblock(struct scsi_device *sdev,
 		 sdev->sdev_state != SDEV_OFFLINE)
 		return -EINVAL;
 
-	spin_lock_irqsave(q->queue_lock, flags);
-	blk_start_queue(q);
-	spin_unlock_irqrestore(q->queue_lock, flags);
+	if (q->mq_ops) {
+		blk_mq_start_stopped_hw_queues(q, false);
+	} else {
+		spin_lock_irqsave(q->queue_lock, flags);
+		blk_start_queue(q);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+	}
 
 	return 0;
 }
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index f079a59..d63c87e 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -88,6 +88,7 @@ extern void scsi_next_command(struct scsi_cmnd *cmd);
 extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_run_host_queues(struct Scsi_Host *shost);
 extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev);
+extern struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev);
 extern int scsi_init_queue(void);
 extern void scsi_exit_queue(void);
 struct request_queue;
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 307a811..c807bc2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -277,7 +277,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	 */
 	sdev->borken = 1;
 
-	sdev->request_queue = scsi_alloc_queue(sdev);
+	if (shost->hostt->use_blk_mq)
+		sdev->request_queue = scsi_mq_alloc_queue(sdev);
+	else
+		sdev->request_queue = scsi_alloc_queue(sdev);
 	if (!sdev->request_queue) {
 		/* release fn is set up in scsi_sysfs_device_initialise, so
 		 * have to free and put manually here */
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index c4e4875..d2661cb 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -531,6 +531,9 @@ struct scsi_host_template {
 	 */
 	unsigned int cmd_size;
 	struct scsi_host_cmd_pool *cmd_pool;
+
+	/* temporary flag to use blk-mq I/O path */
+	bool use_blk_mq;
 };
 
 /*
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 34/39] scsi: partially stub out scsi_adjust_queue_depth when using blk-mq
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (32 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 33/39] scsi: initial blk-mq support Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 35/39] virtio_scsi: use blk_mq Christoph Hellwig
                   ` (5 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0034-scsi-partially-stub-out-scsi_adjust_queue_depth-when.patch --]
[-- Type: text/plain, Size: 796 bytes --]

This will have to be funnelled to blk-mq directly, but skip it for now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 7672371..22af3d3 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -833,7 +833,7 @@ void scsi_adjust_queue_depth(struct scsi_device *sdev, int tagged, int tags)
 	 * is more IO than the LLD's can_queue (so there are not enuogh
 	 * tags) request_fn's host queue ready check will handle it.
 	 */
-	if (!sdev->host->bqt) {
+	if (!sdev->host->hostt->use_blk_mq && !sdev->host->bqt) {
 		if (blk_queue_tagged(sdev->request_queue) &&
 		    blk_queue_resize_tags(sdev->request_queue, tags) != 0)
 			goto out;
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 35/39] virtio_scsi: use blk_mq
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (33 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 36/39] iscsi_tcp: " Christoph Hellwig
                   ` (4 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0035-virtio_scsi-use-blk_mq.patch --]
[-- Type: text/plain, Size: 890 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/virtio_scsi.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 5ec4a73..98469cd 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -680,6 +680,7 @@ static struct scsi_host_template virtscsi_host_template_single = {
 	.use_clustering = ENABLE_CLUSTERING,
 	.target_alloc = virtscsi_target_alloc,
 	.target_destroy = virtscsi_target_destroy,
+	.use_blk_mq = true,
 };
 
 static struct scsi_host_template virtscsi_host_template_multi = {
@@ -697,6 +698,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
 	.use_clustering = ENABLE_CLUSTERING,
 	.target_alloc = virtscsi_target_alloc,
 	.target_destroy = virtscsi_target_destroy,
+	.use_blk_mq = true,
 };
 
 #define virtscsi_config_get(vdev, fld) \
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 36/39] iscsi_tcp: use blk_mq
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (34 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 35/39] virtio_scsi: use blk_mq Christoph Hellwig
@ 2014-03-17 13:28 ` Christoph Hellwig
  2014-03-17 13:28 ` [PATCH 37/39] ata_piix: " Christoph Hellwig
                   ` (3 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0036-iscsi_tcp-use-blk_mq.patch --]
[-- Type: text/plain, Size: 537 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/iscsi_tcp.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index add6d15..44aae3d 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -957,6 +957,7 @@ static struct scsi_host_template iscsi_sw_tcp_sht = {
 	.target_alloc		= iscsi_target_alloc,
 	.proc_name		= "iscsi_tcp",
 	.this_id		= -1,
+	.use_blk_mq		= true,
 };
 
 static struct iscsi_transport iscsi_sw_tcp_transport = {
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 37/39] ata_piix: use blk_mq
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (35 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 36/39] iscsi_tcp: " Christoph Hellwig
@ 2014-03-17 13:28 ` 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
                   ` (2 subsequent siblings)
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0037-ata_piix-use-blk_mq.patch --]
[-- Type: text/plain, Size: 513 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/ata_piix.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 6334c8d..df00cef 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1068,6 +1068,7 @@ static u8 piix_vmw_bmdma_status(struct ata_port *ap)
 
 static struct scsi_host_template piix_sht = {
 	ATA_BMDMA_SHT(DRV_NAME),
+	.use_blk_mq		= true,
 };
 
 static struct ata_port_operations piix_sata_ops = {
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 38/39] blk-mq: make blk_mq_start_stopped_hw_queues run a queue even if not stopped
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (36 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 37/39] ata_piix: " Christoph Hellwig
@ 2014-03-17 13:28 ` 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
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0038-blk-mq-make-blk_mq_start_stopped_hw_queues-run-a-que.patch --]
[-- Type: text/plain, Size: 706 bytes --]

We need this as a workaround for the scsi midlayer, should be fixed in
a nicer way eventually.
---
 block/blk-mq.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4408c70..4950f8e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -677,10 +677,8 @@ void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async)
 	int i;
 
 	queue_for_each_hw_ctx(q, hctx, i) {
-		if (!test_bit(BLK_MQ_S_STOPPED, &hctx->state))
-			continue;
-
-		clear_bit(BLK_MQ_S_STOPPED, &hctx->state);
+		if (test_bit(BLK_MQ_S_STOPPED, &hctx->state))
+			clear_bit(BLK_MQ_S_STOPPED, &hctx->state);
 		blk_mq_run_hw_queue(hctx, async);
 	}
 }
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 39/39] scsi: implement ->init_request and ->exit_request
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (37 preceding siblings ...)
  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 ` Christoph Hellwig
  2014-03-17 13:33 ` [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:28 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

[-- Attachment #1: 0039-scsi-implement-init_request-and-exit_request.patch --]
[-- Type: text/plain, Size: 3705 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c |   67 ++++++++++++++++-------------------------------
 include/linux/blk-mq.h  |    3 +++
 2 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 836f197..e6d87e3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1896,11 +1896,32 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
 	return q;
 }
 
+static int scsi_init_request(void *data, struct blk_mq_hw_ctx *hctx,
+		struct request *rq, unsigned int nr)
+{
+	struct scsi_cmnd *cmd = rq->special;
+
+	cmd->sense_buffer = kzalloc_node(SCSI_SENSE_BUFFERSIZE, GFP_KERNEL,
+					hctx ? hctx->numa_node : -1);
+	if (!cmd->sense_buffer)
+		return -ENOMEM;
+	return 0;
+}
+
+static void scsi_exit_request(struct blk_mq_hw_ctx *hctx, struct request *rq)
+{
+	struct scsi_cmnd *cmd = rq->special;
+
+	kfree(cmd->sense_buffer);
+}
+
 static struct blk_mq_ops scsi_mq_ops = {
 	.queue_rq	= scsi_mq_queue_rq,
 	.map_queue	= blk_mq_map_queue,
 	.alloc_hctx	= blk_mq_alloc_single_hw_queue,
 	.free_hctx	= blk_mq_free_single_hw_queue,
+	.init_request	= scsi_init_request,
+	.exit_request	= scsi_exit_request,
 	.complete	= scsi_softirq_done,
 	.timeout	= scsi_times_out,
 };
@@ -1908,12 +1929,9 @@ static struct blk_mq_ops scsi_mq_ops = {
 struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
 {
 	struct Scsi_Host *shost = sdev->host;
-	struct blk_mq_hw_ctx *hctx;
 	struct request_queue *q;
-	struct request *rq;
-	struct scsi_cmnd *cmd;
 	struct blk_mq_reg reg;
-	int i, j, sgl_size;
+	int sgl_size;
 
 	memset(&reg, 0, sizeof(reg));
 	reg.ops = &scsi_mq_ops;
@@ -1946,48 +1964,7 @@ struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
 
 	__scsi_init_queue(shost, q);
 
-	/*
-	 * XXX: figure out if we can get alignment right to allocate the sense
-	 * buffer with the other chunks of memory.
-	 *
-	 * If not we'll need to find a way to have the blk-mq core call us to
-	 * allocate/free commands so that we can properly clean up the
-	 * allocation instead of leaking it.
-	 */
-	queue_for_each_hw_ctx(q, hctx, i) {
-		for (j = 0; j < hctx->queue_depth; j++) {
-			rq = hctx->rqs[j];
-			cmd = rq->special;
-
-			cmd->sense_buffer = kzalloc_node(SCSI_SENSE_BUFFERSIZE,
-					   GFP_KERNEL, reg.numa_node);
-			if (!cmd->sense_buffer)
-				goto out_free_sense_buffers;
-		}
-	}
-
-	rq = q->flush_rq;
-	cmd = blk_mq_rq_to_pdu(rq);
-
-	cmd->sense_buffer = kzalloc_node(SCSI_SENSE_BUFFERSIZE,
-					   GFP_KERNEL, reg.numa_node);
-	if (!cmd->sense_buffer)
-		goto out_free_sense_buffers;
-
 	return q;
-
-out_free_sense_buffers:
-	queue_for_each_hw_ctx(q, hctx, i) {
-		for (j = 0; j < hctx->queue_depth; j++) {
-			rq = hctx->rqs[j];
-			cmd = rq->special;
-
-			kfree(cmd->sense_buffer);
-		}
-	}
-
-	blk_cleanup_queue(q);
-	return NULL;
 }
 
 /*
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 4d34957..ff194f8 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -68,6 +68,7 @@ typedef int (init_hctx_fn)(struct blk_mq_hw_ctx *, void *, unsigned int);
 typedef void (exit_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
 typedef int (init_request_fn)(void *, struct blk_mq_hw_ctx *,
 		struct request *, unsigned int);
+typedef void (exit_request_fn)(struct blk_mq_hw_ctx *, struct request *);
 
 struct blk_mq_ops {
 	/*
@@ -104,8 +105,10 @@ struct blk_mq_ops {
 	/*
 	 * Called for every command allocated by the block layer to allow
 	 * the driver to set up driver specific data.
+	 * Ditto for exit/teardown.
 	 */
 	init_request_fn		*init_request;
+	exit_request_fn		*exit_request;
 };
 
 enum {
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH 00/39] [WIP] scsi multiqueue
  2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
                   ` (38 preceding siblings ...)
  2014-03-17 13:28 ` [PATCH 39/39] scsi: implement ->init_request and ->exit_request Christoph Hellwig
@ 2014-03-17 13:33 ` Christoph Hellwig
  39 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-17 13:33 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

Btw, this is also available from

    git://git.infradead.org/users/hch/scsi.git#scsi-mq-wip.3

for those who prefer a git tree.

This work was sponsored by the ION division of Fusion IO.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 13/39] megaraid: simplify internal command handling
  2014-03-17 13:27 ` [PATCH 13/39] megaraid: simplify internal command handling Christoph Hellwig
@ 2014-03-21  1:12   ` adam radford
  0 siblings, 0 replies; 46+ messages in thread
From: adam radford @ 2014-03-21  1:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-scsi, linux-kernel

On Mon, Mar 17, 2014 at 6:27 AM, Christoph Hellwig <hch@infradead.org> wrote:
> We don't use the passed in scsi command for anything, so just add a adapter-
> wide internal status to go along with the internal scb that is used unter
> int_mtx to pass back the return value and get rid of all the complexities
> and abuse of the scsi_cmnd structure.
>
> This gets rid of the only user of scsi_allocate_command/scsi_free_command,
> which can now be removed.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Cc: Sumit.Saxena@lsi.com
> Cc: kashyap.desai@lsi.com
> Cc: megaraidlinux@lsi.com
> ---
>  drivers/scsi/megaraid.c  |  120 ++++++++++++----------------------------------
>  drivers/scsi/megaraid.h  |    3 +-
>  drivers/scsi/scsi.c      |   56 ----------------------
>  include/scsi/scsi_cmnd.h |    3 --
>  4 files changed, 32 insertions(+), 150 deletions(-)
>
> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
> index 816db12..8bca30f 100644
> --- a/drivers/scsi/megaraid.c
> +++ b/drivers/scsi/megaraid.c
> @@ -531,13 +531,6 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
>         int     target = 0;
>         int     ldrv_num = 0;   /* logical drive number */
>
> -
> -       /*
> -        * filter the internal and ioctl commands
> -        */
> -       if((cmd->cmnd[0] == MEGA_INTERNAL_CMD))
> -               return (scb_t *)cmd->host_scribble;
> -
>         /*
>          * We know what channels our logical drives are on - mega_find_card()
>          */
> @@ -1439,19 +1432,22 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status)
>
>                 cmdid = completed[i];
>
> -               if( cmdid == CMDID_INT_CMDS ) { /* internal command */
> +               /*
> +                * Only free SCBs for the commands coming down from the
> +                * mid-layer, not for which were issued internally
> +                *
> +                * For internal command, restore the status returned by the
> +                * firmware so that user can interpret it.
> +                */
> +               if (cmdid == CMDID_INT_CMDS) {
>                         scb = &adapter->int_scb;
> -                       cmd = scb->cmd;
> -                       mbox = (mbox_t *)scb->raw_mbox;
>
> -                       /*
> -                        * Internal command interface do not fire the extended
> -                        * passthru or 64-bit passthru
> -                        */
> -                       pthru = scb->pthru;
> +                       list_del_init(&scb->list);
> +                       scb->state = SCB_FREE;
>
> -               }
> -               else {
> +                       adapter->int_status = status;
> +                       complete(&adapter->int_waitq);
> +               } else {
>                         scb = &adapter->scb_list[cmdid];
>
>                         /*
> @@ -1640,25 +1636,7 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status)
>                                 cmd->result |= (DID_BAD_TARGET << 16)|status;
>                 }
>
> -               /*
> -                * Only free SCBs for the commands coming down from the
> -                * mid-layer, not for which were issued internally
> -                *
> -                * For internal command, restore the status returned by the
> -                * firmware so that user can interpret it.
> -                */
> -               if( cmdid == CMDID_INT_CMDS ) { /* internal command */
> -                       cmd->result = status;
> -
> -                       /*
> -                        * Remove the internal command from the pending list
> -                        */
> -                       list_del_init(&scb->list);
> -                       scb->state = SCB_FREE;
> -               }
> -               else {
> -                       mega_free_scb(adapter, scb);
> -               }
> +               mega_free_scb(adapter, scb);
>
>                 /* Add Scsi_Command to end of completed queue */
>                 list_add_tail(SCSI_LIST(cmd), &adapter->completed_list);
> @@ -4133,23 +4111,15 @@ mega_internal_dev_inquiry(adapter_t *adapter, u8 ch, u8 tgt,
>   * The last argument is the address of the passthru structure if the command
>   * to be fired is a passthru command
>   *
> - * lockscope specifies whether the caller has already acquired the lock. Of
> - * course, the caller must know which lock we are talking about.
> - *
>   * Note: parameter 'pthru' is null for non-passthru commands.
>   */
>  static int
>  mega_internal_command(adapter_t *adapter, megacmd_t *mc, mega_passthru *pthru)
>  {
> -       Scsi_Cmnd       *scmd;
> -       struct  scsi_device *sdev;
> +       unsigned long flags;
>         scb_t   *scb;
>         int     rval;
>
> -       scmd = scsi_allocate_command(GFP_KERNEL);
> -       if (!scmd)
> -               return -ENOMEM;
> -
>         /*
>          * The internal commands share one command id and hence are
>          * serialized. This is so because we want to reserve maximum number of
> @@ -4160,73 +4130,45 @@ mega_internal_command(adapter_t *adapter, megacmd_t *mc, mega_passthru *pthru)
>         scb = &adapter->int_scb;
>         memset(scb, 0, sizeof(scb_t));
>
> -       sdev = kzalloc(sizeof(struct scsi_device), GFP_KERNEL);
> -       scmd->device = sdev;
> -
> -       memset(adapter->int_cdb, 0, sizeof(adapter->int_cdb));
> -       scmd->cmnd = adapter->int_cdb;
> -       scmd->device->host = adapter->host;
> -       scmd->host_scribble = (void *)scb;
> -       scmd->cmnd[0] = MEGA_INTERNAL_CMD;
> -
> -       scb->state |= SCB_ACTIVE;
> -       scb->cmd = scmd;
> +       scb->idx = CMDID_INT_CMDS;
> +       scb->state |= SCB_ACTIVE | SCB_PENDQ;
>
>         memcpy(scb->raw_mbox, mc, sizeof(megacmd_t));
>
>         /*
>          * Is it a passthru command
>          */
> -       if( mc->cmd == MEGA_MBOXCMD_PASSTHRU ) {
> -
> +       if (mc->cmd == MEGA_MBOXCMD_PASSTHRU)
>                 scb->pthru = pthru;
> -       }
> -
> -       scb->idx = CMDID_INT_CMDS;
>
> -       megaraid_queue_lck(scmd, mega_internal_done);
> +       spin_lock_irqsave(&adapter->lock, flags);
> +       list_add_tail(&scb->list, &adapter->pending_list);
> +       /*
> +        * Check if the HBA is in quiescent state, e.g., during a
> +        * delete logical drive opertion. If it is, don't run
> +        * the pending_list.
> +        */
> +       if (atomic_read(&adapter->quiescent) == 0)
> +               mega_runpendq(adapter);
> +       spin_unlock_irqrestore(&adapter->lock, flags);
>
>         wait_for_completion(&adapter->int_waitq);
>
> -       rval = scmd->result;
> -       mc->status = scmd->result;
> -       kfree(sdev);
> +       mc->status = rval = adapter->int_status;
>
>         /*
>          * Print a debug message for all failed commands. Applications can use
>          * this information.
>          */
> -       if( scmd->result && trace_level ) {
> +       if( rval && trace_level ) {
>                 printk("megaraid: cmd [%x, %x, %x] status:[%x]\n",
> -                       mc->cmd, mc->opcode, mc->subopcode, scmd->result);
> +                       mc->cmd, mc->opcode, mc->subopcode, rval);
>         }
>
>         mutex_unlock(&adapter->int_mtx);
> -
> -       scsi_free_command(GFP_KERNEL, scmd);
> -
>         return rval;
>  }
>
> -
> -/**
> - * mega_internal_done()
> - * @scmd - internal scsi command
> - *
> - * Callback routine for internal commands.
> - */
> -static void
> -mega_internal_done(Scsi_Cmnd *scmd)
> -{
> -       adapter_t       *adapter;
> -
> -       adapter = (adapter_t *)scmd->device->host->hostdata;
> -
> -       complete(&adapter->int_waitq);
> -
> -}
> -
> -
>  static struct scsi_host_template megaraid_template = {
>         .module                         = THIS_MODULE,
>         .name                           = "MegaRAID",
> diff --git a/drivers/scsi/megaraid.h b/drivers/scsi/megaraid.h
> index 4d0ce4e..8f2e026 100644
> --- a/drivers/scsi/megaraid.h
> +++ b/drivers/scsi/megaraid.h
> @@ -853,10 +853,10 @@ typedef struct {
>
>         u8      sglen;  /* f/w supported scatter-gather list length */
>
> -       unsigned char int_cdb[MAX_COMMAND_SIZE];
>         scb_t                   int_scb;
>         struct mutex            int_mtx;        /* To synchronize the internal
>                                                 commands */
> +       int                     int_status;     /* status of internal cmd */
>         struct completion       int_waitq;      /* wait queue for internal
>                                                  cmds */
>
> @@ -1004,7 +1004,6 @@ static int mega_del_logdrv(adapter_t *, int);
>  static int mega_do_del_logdrv(adapter_t *, int);
>  static void mega_get_max_sgl(adapter_t *);
>  static int mega_internal_command(adapter_t *, megacmd_t *, mega_passthru *);
> -static void mega_internal_done(Scsi_Cmnd *);
>  static int mega_support_cluster(adapter_t *);
>  #endif
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 2b12983..8b2bc06 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -403,62 +403,6 @@ static void scsi_put_host_cmd_pool(gfp_t gfp_mask)
>  }
>
>  /**
> - * scsi_allocate_command - get a fully allocated SCSI command
> - * @gfp_mask:  allocation mask
> - *
> - * This function is for use outside of the normal host based pools.
> - * It allocates the relevant command and takes an additional reference
> - * on the pool it used.  This function *must* be paired with
> - * scsi_free_command which also has the identical mask, otherwise the
> - * free pool counts will eventually go wrong and you'll trigger a bug.
> - *
> - * This function should *only* be used by drivers that need a static
> - * command allocation at start of day for internal functions.
> - */
> -struct scsi_cmnd *scsi_allocate_command(gfp_t gfp_mask)
> -{
> -       struct scsi_host_cmd_pool *pool = scsi_get_host_cmd_pool(gfp_mask);
> -
> -       if (!pool)
> -               return NULL;
> -
> -       return scsi_pool_alloc_command(pool, gfp_mask);
> -}
> -EXPORT_SYMBOL(scsi_allocate_command);
> -
> -/**
> - * scsi_free_command - free a command allocated by scsi_allocate_command
> - * @gfp_mask:  mask used in the original allocation
> - * @cmd:       command to free
> - *
> - * Note: using the original allocation mask is vital because that's
> - * what determines which command pool we use to free the command.  Any
> - * mismatch will cause the system to BUG eventually.
> - */
> -void scsi_free_command(gfp_t gfp_mask, struct scsi_cmnd *cmd)
> -{
> -       struct scsi_host_cmd_pool *pool = scsi_get_host_cmd_pool(gfp_mask);
> -
> -       /*
> -        * this could trigger if the mask to scsi_allocate_command
> -        * doesn't match this mask.  Otherwise we're guaranteed that this
> -        * succeeds because scsi_allocate_command must have taken a reference
> -        * on the pool
> -        */
> -       BUG_ON(!pool);
> -
> -       scsi_pool_free_command(pool, cmd);
> -       /*
> -        * scsi_put_host_cmd_pool is called twice; once to release the
> -        * reference we took above, and once to release the reference
> -        * originally taken by scsi_allocate_command
> -        */
> -       scsi_put_host_cmd_pool(gfp_mask);
> -       scsi_put_host_cmd_pool(gfp_mask);
> -}
> -EXPORT_SYMBOL(scsi_free_command);
> -
> -/**
>   * scsi_setup_command_freelist - Setup the command freelist for a scsi host.
>   * @shost: host to allocate the freelist for.
>   *
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 414edf9..dd7c998 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -155,9 +155,6 @@ extern void scsi_release_buffers(struct scsi_cmnd *cmd);
>  extern int scsi_dma_map(struct scsi_cmnd *cmd);
>  extern void scsi_dma_unmap(struct scsi_cmnd *cmd);
>
> -struct scsi_cmnd *scsi_allocate_command(gfp_t gfp_mask);
> -void scsi_free_command(gfp_t gfp_mask, struct scsi_cmnd *cmd);
> -
>  static inline unsigned scsi_sg_count(struct scsi_cmnd *cmd)
>  {
>         return cmd->sdb.table.nents;
> --
> 1.7.10.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Christoph/James,

I have reviewed this patch, and it looks good to me.
Please consider this ACK'd by the Megaraid driver team.

Acked-by: Adam Radford <aradford@gmail.com>

-Adam

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 16/39] virtio_scsi: use cmd_size
  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
  0 siblings, 1 reply; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-25 15:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-scsi, linux-kernel

Paolo,

do you have a virtio_scsi tree to picks this up in?  We now have all
the infrastructure for it in James' tree.

Btw, it would also be interesting to see if we could get rid of
virtscsi_cmd_cache/pool by reusing fields in EH, but I didn't feel
familar enough with the driver to look into that.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 16/39] virtio_scsi: use cmd_size
  2014-03-25 15:31   ` Christoph Hellwig
@ 2014-03-25 15:36     ` Paolo Bonzini
  2014-03-27 15:28       ` James Bottomley
  0 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2014-03-25 15:36 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-scsi, linux-kernel

Il 25/03/2014 16:31, Christoph Hellwig ha scritto:
> Paolo,
>
> do you have a virtio_scsi tree to picks this up in?  We now have all
> the infrastructure for it in James' tree.

I don't, I usually just give my Acked-by and James picks it up.

Paolo


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 28/39] scsi: reintroduce scsi_driver.init_command
  2014-03-17 13:28 ` [PATCH 28/39] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
@ 2014-03-26  9:46   ` Christoph Hellwig
  0 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2014-03-26  9:46 UTC (permalink / raw)
  To: linux-scsi, linux-kernel

This is another one I'd like to send off to James ASAP, can I get some
reviews?

On Mon, Mar 17, 2014 at 06:28:01AM -0700, Christoph Hellwig wrote:
> Instead of letting the ULD play games with the prep_fn move back to
> the model of a central prep_fn with a callback to the ULD.  This
> already cleans up and shortens the code by itself, and will be required
> to properly support blk-mq in the SCSI midlayer.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++------------------
>  drivers/scsi/sd.c          |   46 +++++++++++-------------------
>  drivers/scsi/sr.c          |   19 ++++---------
>  include/scsi/scsi_driver.h |    8 ++----
>  4 files changed, 63 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index a23e8c3..9889c75 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -470,6 +470,16 @@ void scsi_requeue_run_queue(struct work_struct *work)
>  	scsi_run_queue(q);
>  }
>  
> +static void scsi_uninit_command(struct scsi_cmnd *cmd)
> +{
> +	if (cmd->request->cmd_type == REQ_TYPE_FS) {
> +		struct scsi_driver *drv = scsi_cmd_to_driver(cmd);
> +
> +		if (drv->uninit_command)
> +			drv->uninit_command(cmd);
> +	}
> +}
> +
>  /*
>   * Function:	scsi_requeue_command()
>   *
> @@ -494,6 +504,8 @@ static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
>  	struct request *req = cmd->request;
>  	unsigned long flags;
>  
> +	scsi_uninit_command(cmd);
> +
>  	spin_lock_irqsave(q->queue_lock, flags);
>  	blk_unprep_request(req);
>  	req->special = NULL;
> @@ -1078,15 +1090,7 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
>  
>  int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
>  {
> -	struct scsi_cmnd *cmd;
> -	int ret = scsi_prep_state_check(sdev, req);
> -
> -	if (ret != BLKPREP_OK)
> -		return ret;
> -
> -	cmd = scsi_get_cmd_from_req(sdev, req);
> -	if (unlikely(!cmd))
> -		return BLKPREP_DEFER;
> +	struct scsi_cmnd *cmd = req->special;
>  
>  	/*
>  	 * BLOCK_PC requests may transfer data, in which case they must
> @@ -1130,15 +1134,11 @@ EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
>   */
>  int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
>  {
> -	struct scsi_cmnd *cmd;
> -	int ret = scsi_prep_state_check(sdev, req);
> -
> -	if (ret != BLKPREP_OK)
> -		return ret;
> +	struct scsi_cmnd *cmd = req->special;
>  
>  	if (unlikely(sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh
>  			 && sdev->scsi_dh_data->scsi_dh->prep_fn)) {
> -		ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
> +		int ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
>  		if (ret != BLKPREP_OK)
>  			return ret;
>  	}
> @@ -1148,16 +1148,13 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
>  	 */
>  	BUG_ON(!req->nr_phys_segments);
>  
> -	cmd = scsi_get_cmd_from_req(sdev, req);
> -	if (unlikely(!cmd))
> -		return BLKPREP_DEFER;
> -
>  	memset(cmd->cmnd, 0, BLK_MAX_CDB);
>  	return scsi_init_io(cmd, GFP_ATOMIC);
>  }
>  EXPORT_SYMBOL(scsi_setup_fs_cmnd);
>  
> -int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
> +static int
> +scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
>  {
>  	int ret = BLKPREP_OK;
>  
> @@ -1209,9 +1206,9 @@ int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
>  	}
>  	return ret;
>  }
> -EXPORT_SYMBOL(scsi_prep_state_check);
>  
> -int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
> +static int
> +scsi_prep_return(struct request_queue *q, struct request *req, int ret)
>  {
>  	struct scsi_device *sdev = q->queuedata;
>  
> @@ -1242,18 +1239,33 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(scsi_prep_return);
>  
> -int scsi_prep_fn(struct request_queue *q, struct request *req)
> +static int scsi_prep_fn(struct request_queue *q, struct request *req)
>  {
>  	struct scsi_device *sdev = q->queuedata;
> -	int ret = BLKPREP_KILL;
> +	struct scsi_cmnd *cmd;
> +	int ret;
>  
> -	if (req->cmd_type == REQ_TYPE_BLOCK_PC)
> +	ret = scsi_prep_state_check(sdev, req);
> +	if (ret != BLKPREP_OK)
> +		goto out;
> +
> +	cmd = scsi_get_cmd_from_req(sdev, req);
> +	if (unlikely(!cmd)) {
> +		ret = BLKPREP_DEFER;
> +		goto out;
> +	}
> +
> +	if (req->cmd_type == REQ_TYPE_FS)
> +		ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
> +	else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
>  		ret = scsi_setup_blk_pc_cmnd(sdev, req);
> +	else
> +		ret = BLKPREP_KILL;
> +
> +out:
>  	return scsi_prep_return(q, req, ret);
>  }
> -EXPORT_SYMBOL(scsi_prep_fn);
>  
>  /*
>   * scsi_dev_queue_ready: if we can send requests to sdev, return 1 else
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 470954a..33e349b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -109,6 +109,8 @@ static int sd_suspend_system(struct device *);
>  static int sd_suspend_runtime(struct device *);
>  static int sd_resume(struct device *);
>  static void sd_rescan(struct device *);
> +static int sd_init_command(struct scsi_cmnd *SCpnt);
> +static void sd_uninit_command(struct scsi_cmnd *SCpnt);
>  static int sd_done(struct scsi_cmnd *);
>  static int sd_eh_action(struct scsi_cmnd *, int);
>  static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
> @@ -503,6 +505,8 @@ static struct scsi_driver sd_template = {
>  		.pm		= &sd_pm_ops,
>  	},
>  	.rescan			= sd_rescan,
> +	.init_command		= sd_init_command,
> +	.uninit_command		= sd_uninit_command,
>  	.done			= sd_done,
>  	.eh_action		= sd_eh_action,
>  };
> @@ -838,9 +842,9 @@ static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
>  	return scsi_setup_blk_pc_cmnd(sdp, rq);
>  }
>  
> -static void sd_unprep_fn(struct request_queue *q, struct request *rq)
> +static void sd_uninit_command(struct scsi_cmnd *SCpnt)
>  {
> -	struct scsi_cmnd *SCpnt = rq->special;
> +	struct request *rq = SCpnt->request;
>  
>  	if (rq->cmd_flags & REQ_DISCARD) {
>  		free_page((unsigned long)rq->buffer);
> @@ -853,18 +857,10 @@ static void sd_unprep_fn(struct request_queue *q, struct request *rq)
>  	}
>  }
>  
> -/**
> - *	sd_prep_fn - build a scsi (read or write) command from
> - *	information in the request structure.
> - *	@SCpnt: pointer to mid-level's per scsi command structure that
> - *	contains request and into which the scsi command is written
> - *
> - *	Returns 1 if successful and 0 if error (or cannot be done now).
> - **/
> -static int sd_prep_fn(struct request_queue *q, struct request *rq)
> +static int sd_init_command(struct scsi_cmnd *SCpnt)
>  {
> -	struct scsi_cmnd *SCpnt;
> -	struct scsi_device *sdp = q->queuedata;
> +	struct request *rq = SCpnt->request;
> +	struct scsi_device *sdp = SCpnt->device;
>  	struct gendisk *disk = rq->rq_disk;
>  	struct scsi_disk *sdkp;
>  	sector_t block = blk_rq_pos(rq);
> @@ -886,12 +882,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
>  	} else if (rq->cmd_flags & REQ_FLUSH) {
>  		ret = scsi_setup_flush_cmnd(sdp, rq);
>  		goto out;
> -	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> -		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> -		goto out;
> -	} else if (rq->cmd_type != REQ_TYPE_FS) {
> -		ret = BLKPREP_KILL;
> -		goto out;
>  	}
>  	ret = scsi_setup_fs_cmnd(sdp, rq);
>  	if (ret != BLKPREP_OK)
> @@ -903,11 +893,10 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
>  	 * is used for a killable error condition */
>  	ret = BLKPREP_KILL;
>  
> -	SCSI_LOG_HLQUEUE(1, scmd_printk(KERN_INFO, SCpnt,
> -					"sd_prep_fn: block=%llu, "
> -					"count=%d\n",
> -					(unsigned long long)block,
> -					this_count));
> +	SCSI_LOG_HLQUEUE(1,
> +		scmd_printk(KERN_INFO, SCpnt,
> +			"%s: block=%llu, count=%d\n",
> +			__func__, (unsigned long long)block, this_count));
>  
>  	if (!sdp || !scsi_device_online(sdp) ||
>  	    block + blk_rq_sectors(rq) > get_capacity(disk)) {
> @@ -1127,7 +1116,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
>  	 */
>  	ret = BLKPREP_OK;
>   out:
> -	return scsi_prep_return(q, rq, ret);
> + 	return ret;
>  }
>  
>  /**
> @@ -1663,6 +1652,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>  	unsigned char op = SCpnt->cmnd[0];
>  	unsigned char unmap = SCpnt->cmnd[1] & 8;
>  
> +	sd_uninit_command(SCpnt);
> +
>  	if (req->cmd_flags & REQ_DISCARD || req->cmd_flags & REQ_WRITE_SAME) {
>  		if (!result) {
>  			good_bytes = blk_rq_bytes(req);
> @@ -2872,9 +2863,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
>  
>  	sd_revalidate_disk(gd);
>  
> -	blk_queue_prep_rq(sdp->request_queue, sd_prep_fn);
> -	blk_queue_unprep_rq(sdp->request_queue, sd_unprep_fn);
> -
>  	gd->driverfs_dev = &sdp->sdev_gendev;
>  	gd->flags = GENHD_FL_EXT_DEVT;
>  	if (sdp->removable) {
> @@ -3021,8 +3009,6 @@ static int sd_remove(struct device *dev)
>  	scsi_autopm_get_device(sdkp->device);
>  
>  	async_synchronize_full_domain(&scsi_sd_probe_domain);
> -	blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
> -	blk_queue_unprep_rq(sdkp->device->request_queue, NULL);
>  	device_del(&sdkp->dev);
>  	del_gendisk(sdkp->disk);
>  	sd_shutdown(dev);
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 40d8592..93cbd36 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -79,6 +79,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
>  static DEFINE_MUTEX(sr_mutex);
>  static int sr_probe(struct device *);
>  static int sr_remove(struct device *);
> +static int sr_init_command(struct scsi_cmnd *SCpnt);
>  static int sr_done(struct scsi_cmnd *);
>  static int sr_runtime_suspend(struct device *dev);
>  
> @@ -94,6 +95,7 @@ static struct scsi_driver sr_template = {
>  		.remove		= sr_remove,
>  		.pm		= &sr_pm_ops,
>  	},
> +	.init_command		= sr_init_command,
>  	.done			= sr_done,
>  };
>  
> @@ -378,21 +380,14 @@ static int sr_done(struct scsi_cmnd *SCpnt)
>  	return good_bytes;
>  }
>  
> -static int sr_prep_fn(struct request_queue *q, struct request *rq)
> +static int sr_init_command(struct scsi_cmnd *SCpnt)
>  {
>  	int block = 0, this_count, s_size;
>  	struct scsi_cd *cd;
> -	struct scsi_cmnd *SCpnt;
> -	struct scsi_device *sdp = q->queuedata;
> +	struct request *rq = SCpnt->request;
> +	struct scsi_device *sdp = SCpnt->device;
>  	int ret;
>  
> -	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> -		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> -		goto out;
> -	} else if (rq->cmd_type != REQ_TYPE_FS) {
> -		ret = BLKPREP_KILL;
> -		goto out;
> -	}
>  	ret = scsi_setup_fs_cmnd(sdp, rq);
>  	if (ret != BLKPREP_OK)
>  		goto out;
> @@ -517,7 +512,7 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)
>  	 */
>  	ret = BLKPREP_OK;
>   out:
> -	return scsi_prep_return(q, rq, ret);
> +	return ret;
>  }
>  
>  static int sr_block_open(struct block_device *bdev, fmode_t mode)
> @@ -718,7 +713,6 @@ static int sr_probe(struct device *dev)
>  
>  	/* FIXME: need to handle a get_capabilities failure properly ?? */
>  	get_capabilities(cd);
> -	blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
>  	sr_vendor_init(cd);
>  
>  	disk->driverfs_dev = &sdev->sdev_gendev;
> @@ -993,7 +987,6 @@ static int sr_remove(struct device *dev)
>  
>  	scsi_autopm_get_device(cd->device);
>  
> -	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
>  	del_gendisk(cd->disk);
>  
>  	mutex_lock(&sr_ref_mutex);
> diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
> index 20fdfc2..b507729 100644
> --- a/include/scsi/scsi_driver.h
> +++ b/include/scsi/scsi_driver.h
> @@ -6,15 +6,14 @@
>  struct module;
>  struct scsi_cmnd;
>  struct scsi_device;
> -struct request;
> -struct request_queue;
> -
>  
>  struct scsi_driver {
>  	struct module		*owner;
>  	struct device_driver	gendrv;
>  
>  	void (*rescan)(struct device *);
> +	int (*init_command)(struct scsi_cmnd *);
> +	void (*uninit_command)(struct scsi_cmnd *);
>  	int (*done)(struct scsi_cmnd *);
>  	int (*eh_action)(struct scsi_cmnd *, int);
>  };
> @@ -31,8 +30,5 @@ extern int scsi_register_interface(struct class_interface *);
>  
>  int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
>  int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req);
> -int scsi_prep_state_check(struct scsi_device *sdev, struct request *req);
> -int scsi_prep_return(struct request_queue *q, struct request *req, int ret);
> -int scsi_prep_fn(struct request_queue *, struct request *);
>  
>  #endif /* _SCSI_SCSI_DRIVER_H */
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 16/39] virtio_scsi: use cmd_size
  2014-03-25 15:36     ` Paolo Bonzini
@ 2014-03-27 15:28       ` James Bottomley
  0 siblings, 0 replies; 46+ messages in thread
From: James Bottomley @ 2014-03-27 15:28 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Christoph Hellwig, linux-scsi, linux-kernel

On Tue, 2014-03-25 at 16:36 +0100, Paolo Bonzini wrote:
> Il 25/03/2014 16:31, Christoph Hellwig ha scritto:
> > Paolo,
> >
> > do you have a virtio_scsi tree to picks this up in?  We now have all
> > the infrastructure for it in James' tree.
> 
> I don't, I usually just give my Acked-by and James picks it up.

OK, since we have the acked by, this can go in ... there are some other
dependent patches as well, I think, could you resend those as a separate
series too, please Christoph.

James



^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2014-03-27 15:28 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH 23/39] scsi: fix the {host,target,device}_blocked counter mess Christoph Hellwig
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox