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, jianchao.wang 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. Thanks Jianchao
