Re: [PATCH] blk-mq: only run the hardware queue if IO is pending

[Jens Axboe <axboe@kernel.dk>]

On 11/10/2017 10:34 AM, Christoph Hellwig wrote:
> On Fri, Nov 10, 2017 at 10:31:29AM -0700, Jens Axboe wrote:
>> On 11/10/2017 10:29 AM, Christoph Hellwig wrote:
>>> On Fri, Nov 10, 2017 at 09:12:18AM -0700, Jens Axboe wrote:
>>>> Currently we are inconsistent in when we decide to run the queue. Using
>>>> blk_mq_run_hw_queues() we check if the hctx has pending IO before
>>>> running it, but we don't do that from the individual queue run function,
>>>> blk_mq_run_hw_queue(). This results in a lot of extra and pointless
>>>> queue runs, potentially, on flush requests and (much worse) on tag
>>>> starvation situations. This is observable just looking at the top
>>>> output, with lots of kworkers active. For the !async runs, it just adds
>>>> to the CPU overhead of blk-mq.
>>>>
>>>> Move the has-pending check into the run function instead of having
>>>> callers do it.
>>>>
>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>
>>> Do we even still need the blk_mq_hctx_has_pending helper at all?
>>
>> We do, otherwise we end up running the queue for cases where we don't
>> need to. The caller doesn't always know if it's necessary. And even
>> if the caller can figure it out, it'd add complicated logic for some
>> of those cases (like flush). It's easier/cleaner and more efficient
>> to keep it.
> 
> I didn't mean the logic - just wondering if we need the separate
> helper given it has a single caller left.
> 
> But it's really just a cosmetic point.

Ah, that makes more sense. Since the function isn't necessarily that
straight forward if open coded (to read, I mean), I'd prefer to keep
it separate. But yeah, cosmetic, can always be revisited.

-- 
Jens Axboe