linux-kernel - Re: [PATCH 1/3] blk-mq: optimise rq sort function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 29 Nov 2019 12:07:07 +0200
From:   Nikolay Borisov <nborisov@...e.com>
To:     Pavel Begunkov <asml.silence@...il.com>,
        Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] blk-mq: optimise rq sort function



On 29.11.19 г. 11:45 ч., Pavel Begunkov wrote:
> On 11/29/2019 11:28 AM, Nikolay Borisov wrote:
>> On 28.11.19 г. 23:11 ч., Pavel Begunkov wrote:
>>> Check "!=" in multi-layer comparisons. The same memory usage, fewer
>>> instructions, and 2 from 4 jumps are replaced with SETcc.
>>>
>>> Note, that list_sort() doesn't differ 0 and <0.
>>>
>>> Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
>>
>> My first reaction was this is wrong since you no longer return negative
>> values. But then I looked into list_sort/merge and this branch
>> 'if (cmp(priv, a, b) <= 0) {' clearly shows this is correct.
> 
> Yes, that's why there is a note in the patch description. The same is
> told by list_sort() description.

And of course I have missed your remark in the changelog :)

> 
>>
>> So :
>>
>> Reviewed-by: Nikolay Borisov <nborisov@...e.com>
> 
> Thanks for taking a look
> 
>>
>>> ---
>>>  block/blk-mq.c | 12 ++++--------
>>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>> index 323c9cb28066..f32a3cfdd34e 100644
>>> --- a/block/blk-mq.c
>>> +++ b/block/blk-mq.c
>>> @@ -1668,14 +1668,10 @@ static int plug_rq_cmp(void *priv, struct list_head *a, struct list_head *b)
>>>  	struct request *rqa = container_of(a, struct request, queuelist);
>>>  	struct request *rqb = container_of(b, struct request, queuelist);
>>>  
>>> -	if (rqa->mq_ctx < rqb->mq_ctx)
>>> -		return -1;
>>> -	else if (rqa->mq_ctx > rqb->mq_ctx)
>>> -		return 1;
>>> -	else if (rqa->mq_hctx < rqb->mq_hctx)
>>> -		return -1;
>>> -	else if (rqa->mq_hctx > rqb->mq_hctx)
>>> -		return 1;
>>> +	if (rqa->mq_ctx != rqb->mq_ctx)
>>> +		return rqa->mq_ctx > rqb->mq_ctx;
>>> +	if (rqa->mq_hctx != rqb->mq_hctx)
>>> +		return rqa->mq_hctx > rqb->mq_hctx;
>>>  
>>>  	return blk_rq_pos(rqa) > blk_rq_pos(rqb);
>>>  }
>>>
>