On Thu, Apr 24, 2025 at 4:38 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> > +static void blk_mq_extract_queue_requests(struct rq_list *rqs,
> > + struct rq_list *queue_rqs,
> > + unsigned *queue_depth)
> > +{
> > + struct rq_list matched_rqs = {}, unmatched_rqs = {};
> > + struct request *rq = rq_list_pop(rqs);
> > + struct request_queue *this_q = rq->q;
> > + unsigned depth = 1;
> > +
> > + rq_list_add_tail(&matched_rqs, rq);
> > + while ((rq = rq_list_pop(rqs))) {
> > + if (rq->q == this_q) {
> > + rq_list_add_tail(&matched_rqs, rq);
> > + depth++;
> > + } else {
> > + rq_list_add_tail(&unmatched_rqs, rq);
> > + }
>
> This might be moe efficient if you keep an extra iterator and never
> mode the request for another queue.
Sure, will do.
>
> > + }
> > +
> > + *queue_rqs = matched_rqs;
> > + *rqs = unmatched_rqs;
> > + *queue_depth = depth;
>
> .. and I'd return the queue depth here instead of making it another
> output argument.
Okay.
>
> > +static void blk_mq_dispatch_multiple_queue_requests(struct rq_list *rqs)
> > +{
> > + do {
> > + struct rq_list queue_rqs;
> > + unsigned depth;
> > +
> > + blk_mq_extract_queue_requests(rqs, &queue_rqs, &depth);
> > + blk_mq_dispatch_queue_requests(&queue_rqs, depth);
> > + while (!rq_list_empty(&queue_rqs)) {
> > + blk_mq_dispatch_list(&queue_rqs, false);
> > + }
>
> No need for the braces in the inner while loop here.
Old habits die hard :)
>
> The other caller of blk_mq_dispatch_list loops until the list is empty,
> why don't we need that here?
This loop does continue calling blk_mq_dispatch_list() until queue_rqs
is empty. And the outer loop keeps repopulating queue_rqs until the
entire rqs list is empty. Am I misunderstanding you?
Thanks for the review,
Caleb
