Re: [PATCH 8/8] IB/srp: Add multichannel support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2014-10-01 10:08, Bart Van Assche wrote:
On 09/19/14 17:38, Jens Axboe wrote:
ctx was meant to be private, unfortunately it's leaked a bit into other
parts of block/. But it's still private within that, at least.

Lets not add more stuff to struct request, it's already way too large.
We could add an exported

struct blk_mq_hw_ctx *blk_mq_request_to_hctx(struct request *rq)
{
	struct request_queue *q = rq->q;

	return q->mq_ops->map_queue(q, rq->mq_ctx->cpu);
}

for this.

How about the patch below ? That patch makes it easy for SCSI LLDs to obtain
the hardware context index.

[PATCH] blk-mq: Add blk_get_mq_tag()

The queuecommand() callback functions in SCSI low-level drivers
must know which hardware context has been selected by the block
layer. Since passing the hctx pointer directly to the queuecommand
callback function would require modification of all SCSI LLDs,
add a function to the block layer that allows to query the hardware
context index.

Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
  block/blk-mq-tag.c     | 24 ++++++++++++++++++++++++
  include/linux/blk-mq.h | 16 ++++++++++++++++
  2 files changed, 40 insertions(+)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index c1b9242..5618759 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -595,6 +595,30 @@ int blk_mq_tag_update_depth(struct blk_mq_tags *tags, unsigned int tdepth)
  	return 0;
  }

+/**
+ * blk_get_mq_tag() - return a tag that is unique queue-wide
+ *
+ * The tag field in struct request is unique per hardware queue but not
+ * queue-wide. Hence this function. It is not only safe to use this function
+ * for multiqueue request queues but also for single-queue request queues.
+ * Note: rq->tag == -1 if tagging is not enabled for single-queue request
+ * queues.
+ */
+struct blk_mq_tag blk_get_mq_tag(struct request *rq)
+{
+	struct request_queue *q = rq->q;
+	struct blk_mq_tag tag = { rq->tag & ((1 << 16) - 1) };
+	struct blk_mq_hw_ctx *hctx;
+
+	if (q->mq_ops) {
+		hctx = q->mq_ops->map_queue(q, rq->mq_ctx->cpu);
+		tag.tag |= hctx->queue_num << 16;
+	}
+
+	return tag;
+}
+EXPORT_SYMBOL(blk_get_mq_tag);

Lets get rid of the blk_mq_tag struct and just have it be an u32 or something. We could potentially typedef it, but I'd prefer to just have it be an unsigned 32-bit int.

Probably also need some init time safety checks that 16-bits is enough to hold BLK_MQ_MAX_DEPTH. Just in case that is ever bumped, or the queue prefixing changes.

And I think we need to name this better. Your comment correctly describes that this generates a unique tag queue wide, but the name of the function implies that we just return the request tag. Most drivers wont use this. Perhaps add a queue flag that tells us that we should generate these tags and have it setup ala:

u32 blk_mq_unique_rq_tag(struct request *rq)
{
	struct request_queue *q = rq->q;
	u32 tag = rq->tag & ((1 << 16) - 1);
	struct blk_mq_hw_ctx *hctx;

	hctx = q->mq_ops->map_queue(q, rq->mq_ctx->cpu);	
	return tag | (hctx->queue_num << 16);
}

u32 blk_mq_rq_tag(struct request *rq)
{
	struct request_queue *q = rq->q;

	if (q->mq_ops &&
	    test_bit(QUEUE_FLAG_UNIQUE_TAGS, &q->queue_flags))
		return blk_mq_unique_rq_tag(rq);

	return rq->tag;
}

Totally untested, just typed in email.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux