Messages in this thread | | | Subject | Re: [PATCH RFC 3/5] blk-mq: Facilitate a shared tags per tagset | From | Bart Van Assche <> | Date | Fri, 15 Nov 2019 09:57:55 -0800 |
| |
On 11/15/19 2:24 AM, John Garry wrote: > Bart Van Assche wrote: > > How about sharing tag sets across hardware > > queues, e.g. like in the (totally untested) patch below? > > So this is similar in principle what Ming Lei came up with here: > https://lore.kernel.org/linux-block/20190531022801.10003-1-ming.lei@redhat.com/ > > However your implementation looks neater, which is good. > > My concern with this approach is that we can't differentiate which tags > are allocated for which hctx, and sometimes we need to know that. > > An example here was blk_mq_queue_tag_busy_iter(), which iterates the > bits for each hctx. This would just be broken by that change, unless we > record which bits are associated with each hctx.
I disagree. In bt_iter() I added " && rq->mq_hctx == hctx" such that blk_mq_queue_tag_busy_iter() only calls the callback function for matching (hctx, rq) pairs.
> Another example was __blk_mq_tag_idle(), which looks problematic.
Please elaborate.
> For debugfs, when we examine > /sys/kernel/debug/block/.../hctxX/tags_bitmap, wouldn't that be the tags > for all hctx (hctx0)? > > For debugging reasons, I would say we want to know which tags are > allocated for a specific hctx, as this is tightly related to the > requests for that hctx.
That is an open issue in the patch I posted and something that needs to be addressed. One way to address this is to change the sbitmap_bitmap_show() calls into calls to a function that only shows those bits for which rq->mq_hctx == hctx.
>> @@ -341,8 +341,11 @@ void blk_mq_tagset_busy_iter(struct >> blk_mq_tag_set *tagset, >> int i; >> >> for (i = 0; i < tagset->nr_hw_queues; i++) { >> - if (tagset->tags && tagset->tags[i]) >> + if (tagset->tags && tagset->tags[i]) { >> blk_mq_all_tag_busy_iter(tagset->tags[i], fn, priv); > > As I mentioned earlier, wouldn't this iterate over all tags for all > hctx's, when we just want the tags for hctx[i]? > > Thanks, > John > > [Not trimming reply for future reference] > >> + if (tagset->share_tags) >> + break; >> + } >> } >> } >> EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
Since blk_mq_tagset_busy_iter() loops over all hardware queues all what is changed is the order in which requests are examined. I am not aware of any block driver that calls blk_mq_tagset_busy_iter() and that depends on the order of the requests passed to the callback function.
Thanks,
Bart.
| |