| Message ID | 542D8143.3050305@acm.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 10/02/2014 10:45 AM, Bart Van Assche wrote: > On 10/01/14 18:54, Jens Axboe wrote: >> 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; >> } > > Would it be acceptable to let blk_mq_rq_tag() always return the > hardware context number and the per-hctx tag ? Block and SCSI LLD Sure, that's fine as well, but the function needs a more descriptive name. I try to think of it like I have never looked at the code and need to write a driver, it's a lot easier if the functions are named appropriately. Seeing blk_mq_rq_tag() and even with reading the function comment, I'm really none the wiser and would assume I need to use this function to get the tag. So we can do the single function, but lets call it blk_mq_unique_rq_tag(). That's special enough that people will know this is something that doesn't just return the request tag. Then add an extra sentence to the comment you already have on when this is needed. And lets roll those bitshift values and masks into a define or enum so it's collected in one place.
On Thu, Oct 02, 2014 at 06:45:55PM +0200, Bart Van Assche wrote: > Would it be acceptable to let blk_mq_rq_tag() always return the > hardware context number and the per-hctx tag ? Block and SCSI LLD > drivers that do not need the hardware context number can still use > rq->tag. Drivers that need both can use blk_mq_rq_tag(). That way we do > not have to introduce a new queue flag. How about the patch below > (which is still missing a BLK_MQ_MAX_DEPTH check): I'd add the unique_ part to the name that Jens added, and fix up the comment to be valid kerneldoc, but otherwise this looks fine to me. Also if we want to merge scsi LLDDs that can take advantage of multiqueue support it would probably be best if I take this via the SCSI tree. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/14 19:30, Christoph Hellwig wrote: > Also if we want to merge scsi LLDDs that can take advantage of > multiqueue support it would probably be best if I take this via the SCSI > tree. Sending these patches to you is fine with me, at least if Roland agrees. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 6, 2014 at 4:16 AM, Bart Van Assche <bvanassche@acm.org> wrote: > On 10/02/14 19:30, Christoph Hellwig wrote: >> Also if we want to merge scsi LLDDs that can take advantage of >> multiqueue support it would probably be best if I take this via the SCSI >> tree. > Sending these patches to you is fine with me, at least if Roland agrees. That's fine with me. Christoph/Bart should I just let you guys handle all the pending SRP patches, or is there anything I should pick up via the InfiniBand tree? Everything that's in flight looks reasonable to me, so I'm fine with however you guys want to merge it. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index c1b9242..8cfbc7b 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_mq_rq_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. + * + * Note: When called for a non-multiqueue request queue, the hardware context + * index is set to zero. + */ +u32 blk_mq_rq_tag(struct request *rq) +{ + struct request_queue *q = rq->q; + struct blk_mq_hw_ctx *hctx; + int hwq = 0; + + if (q->mq_ops) { + hctx = q->mq_ops->map_queue(q, rq->mq_ctx->cpu); + hwq = hctx->queue_num; + } + + return blk_mq_build_rq_tag(hwq, rq->tag); +} +EXPORT_SYMBOL(blk_mq_rq_tag); + ssize_t blk_mq_tag_sysfs_show(struct blk_mq_tags *tags, char *page) { char *orig_page = page; diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index eac4f31..c5be535 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -166,6 +166,19 @@ bool blk_mq_can_queue(struct blk_mq_hw_ctx *); struct request *blk_mq_alloc_request(struct request_queue *q, int rw, gfp_t gfp, bool reserved); struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag); +u32 blk_mq_rq_tag(struct request *rq); +static inline u32 blk_mq_build_rq_tag(int hwq, int tag) +{ + return (hwq << 16) | (tag & ((1 << 16) - 1)); +} +static inline u16 blk_mq_rq_tag_to_hwq(u32 rq_tag) +{ + return rq_tag >> 16; +} +static inline u16 blk_mq_rq_tag_to_tag(u32 rq_tag) +{ + return rq_tag & ((1 << 16) - 1); +} struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *, const int ctx_index); struct blk_mq_hw_ctx *blk_mq_alloc_single_hw_queue(struct blk_mq_tag_set *, unsigned int, int);