Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 1/3] blk-mq: optimise rq sort function
To:     Nikolay Borisov <nborisov@suse.com>, Jens Axboe <axboe@kernel.dk>,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
References: <cover.1574974577.git.asml.silence@gmail.com>
 <a21e91fff1abe201ebbc010d596b034f7b5123de.1574974577.git.asml.silence@gmail.com>
 <0124910c-4dcf-2143-0ec6-32dad123067c@suse.com>
From:   Pavel Begunkov <asml.silence@gmail.com>
Message-ID: <7b1a16db-287f-bba0-fca0-cc7f85f69ec6@gmail.com>
Date:   Fri, 29 Nov 2019 12:45:03 +0300
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.1
MIME-Version: 1.0
In-Reply-To: <0124910c-4dcf-2143-0ec6-32dad123067c@suse.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@gmail.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.

> 
> So :
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.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);
>>  }
>>

-- 
Pavel Begunkov