[lkml]   [2018]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH V7 1/4] blk-mq: refactor the code of issue request directly

On 11/14/18 11:22 PM, Jens Axboe wrote:
> On 11/14/18 2:43 AM, Ming Lei wrote:
>> On Wed, Nov 14, 2018 at 05:23:48PM +0800, wrote:
>>> Hi Ming
>>> On 11/14/18 5:11 PM, Ming Lei wrote:
>>>>> - if (!blk_mq_get_dispatch_budget(hctx))
>>>>> - goto insert;
>>>>> + if (unlikely(!blk_mq_get_dispatch_budget(hctx)))
>>>>> + goto out_unlock;
>>>> The unlikely annotation is a bit misleading, since out-of-budget can
>>>> happen frequently in case of low queue depth, and there are lots of
>>>> such examples.
>>> This could be good for the case for no .get_budget and getting budget success.
>>> In case of out-of-budget, we insert the request which is slow path.
>> In case of low queue depth, it is hard to say that 'insert request' is
>> done in slow path, cause it happens quite frequently.
>> I suggest to remove these two unlikely() since modern CPU's branch prediction
>> should work well enough.
>> Especially the annotation of unlikely() often means that this branch is
>> missed in most of times for all settings, and it is obviously not true
>> in this case.
> Agree, unlikely() should only be used for the error handling case or
> similar that does indeed almost never trigger. It should not be used
> for cases that don't trigger a lot in "most" circumstances.

That's really appreciated for all of your kindly response.
Fair enough with 'unlikely'.
I will remove these two wrong 'unlikely' in next version.


 \ /
  Last update: 2018-11-15 02:36    [W:0.056 / U:3.652 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site