Re: [PATCH] megaraid_sas: Enable shared host tag map

[Hannes Reinecke <hare@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

On 11/25/2014 03:31 PM, Christoph Hellwig wrote:
> On Mon, Nov 24, 2014 at 04:51:14PM +0100, Hannes Reinecke wrote:
>> It is useful as is, as we'll be getting prefixed logging output :-)
> 
> Use the blk-mq code path if you care :)
> 
>> Which I didn't do yet as the driver is using a larger tag map than
>> that one announced to the block layer.
>> This is to facilitate internal command submission, which should
>> always work independent on any tag starvation issues from the
>> upper layers.
> 
> This is an "issue" for a lot of drivers.  blk-mq provides a reserved_tags
> pool for that, which reserves a number of tags for internal use, those
> must be allocated using blk_mq_alloc_request with the reserved argument
> set to true.
> 
> The lockless hpsa patches expose this to SCSI, which I'm generally
> fine with, but we need to find a way to transparently make this work
> for the old code path, too.  This might be as simple as embedding a
> second blk_queue_tag structure into the Scsi_Host, adding a constant
> prefix to the tag and providing some wrappes in scsi that allow
> allocating a struct request (or rather scsi_cmnd) for internal use.
> 
I'd rather have a single map to get request/tags from; otherwise
we'd be arbitrarily starving internal requests even though the
'main' tag map is empty.
My plan was more to mark a certain range of tags as 'reserved',
and add another helper/argument to allow to dip into the reserved
pool, too.

A tentative patch is attached.
Idea is to call blk_queue_init_tags() with the actual tag size and
then blk_resize_tags() to limit the number of tags for the request
queue.
The driver can then use 'blk_allocate_tag' with the appropriate max
depth to get tags from the range [max_depth:real_max_depth].

Cheers,

Hannes

-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-blk-tag-separate-out-blk_-allocate-release-_tag.patch --]
[-- Type: text/x-patch; name="0001-blk-tag-separate-out-blk_-allocate-release-_tag.patch", Size: 3252 bytes --]

From e872bb0c4c2b1f72982f4d31925d3b2f65317ffd Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@suse.de>
Date: Tue, 25 Nov 2014 15:43:07 +0100
Subject: [PATCH] blk-tag: separate out blk_(allocate|release)_tag

Separate out helper functions blk_allocate_tag() and
blk_release_tag().

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-tag.c | 74 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/block/blk-tag.c b/block/blk-tag.c
index a185b86..6526fff 100644
--- a/block/blk-tag.c
+++ b/block/blk-tag.c
@@ -244,6 +244,24 @@ int blk_queue_resize_tags(struct request_queue *q, int new_depth)
 }
 EXPORT_SYMBOL(blk_queue_resize_tags);
 
+void blk_release_tag(struct blk_queue_tag *bqt, int tag)
+{
+	if (unlikely(!bqt))
+		return;
+
+	if (unlikely(!test_bit(tag, bqt->tag_map))) {
+		printk(KERN_ERR "%s: attempt to clear non-busy tag (%d)\n",
+		       __func__, tag);
+		return;
+	}
+	/*
+	 * The tag_map bit acts as a lock for tag_index[bit], so we need
+	 * unlock memory barrier semantics.
+	 */
+	clear_bit_unlock(tag, bqt->tag_map);
+}
+EXPORT_SYMBOL(blk_release_tag);
+
 /**
  * blk_queue_end_tag - end tag operations for a request
  * @q:  the request queue for the device
@@ -275,18 +293,43 @@ void blk_queue_end_tag(struct request_queue *q, struct request *rq)
 
 	bqt->tag_index[tag] = NULL;
 
-	if (unlikely(!test_bit(tag, bqt->tag_map))) {
-		printk(KERN_ERR "%s: attempt to clear non-busy tag (%d)\n",
-		       __func__, tag);
-		return;
-	}
+	blk_release_tag(bqt, tag);
+}
+EXPORT_SYMBOL(blk_queue_end_tag);
+
+/**
+ * blk_reserve_tag - lock and return the next free tag
+ * @bqt:  tag map
+ * @max_depth: max tag depth to use
+ *
+ * Description:
+ *   Lock and return the next free tag.
+ *   The tag needs to be freed up after usage with
+ *   blk_release_tag()
+ *
+ **/
+int blk_reserve_tag(struct blk_queue_tag *bqt, int max_depth)
+{
+	int tag = -1;
+
+	if (!bqt)
+		return tag;
+	if (max_depth >= bqt->real_max_depth)
+		return -1;
+
+	do {
+		tag = find_first_zero_bit(bqt->tag_map, max_depth);
+		if (tag >= max_depth)
+			return tag;
+
+	} while (test_and_set_bit_lock(tag, bqt->tag_map));
 	/*
-	 * The tag_map bit acts as a lock for tag_index[bit], so we need
-	 * unlock memory barrier semantics.
+	 * We need lock ordering semantics given by test_and_set_bit_lock.
+	 * See blk_queue_end_tag for details.
 	 */
-	clear_bit_unlock(tag, bqt->tag_map);
+	return tag;
 }
-EXPORT_SYMBOL(blk_queue_end_tag);
+EXPORT_SYMBOL(blk_reserve_tag);
 
 /**
  * blk_queue_start_tag - find a free tag and assign it
@@ -343,16 +386,9 @@ int blk_queue_start_tag(struct request_queue *q, struct request *rq)
 			return 1;
 	}
 
-	do {
-		tag = find_first_zero_bit(bqt->tag_map, max_depth);
-		if (tag >= max_depth)
-			return 1;
-
-	} while (test_and_set_bit_lock(tag, bqt->tag_map));
-	/*
-	 * We need lock ordering semantics given by test_and_set_bit_lock.
-	 * See blk_queue_end_tag for details.
-	 */
+	tag = blk_reserve_tag(bqt, max_depth);
+	if (tag == -1)
+		return 1;
 
 	rq->cmd_flags |= REQ_QUEUED;
 	rq->tag = tag;
-- 
1.8.5.2