Re: [PATCH RESEND] io_uring: a small optimization for REQ_F_DRAIN_LINK

[JackieLiu <jackieliu@byteisland.com>]

> 2019年11月22日 18:05，Pavel Begunkov <asml.silence@gmail.com> 写道：
> 
> On 11/22/2019 9:01 AM, Jackie Liu wrote:
>> From: Jackie Liu <liuyun01@kylinos.cn>
>> 
>> We don't need to set drain_next every time, make a small judgment
>> and add unlikely, it looks like there will be a little optimization.
>> 
> Not sure about that. It's 1 CMP + 1 SETcc/STORE, which works pretty fast
> as @drain_next is hot (especially after read) and there is no write-read
> dependency close. For yours, there is likely always 3 CMPs in the way.
> 
> Did you benchmarked it somehow or compared assembly?

It is only theoretically possible. In most cases, our drain_link 
and drain_next are both false, so only two CMPs are needed, and modern CPUs
have branch predictions. Perhaps these judgments can be optimized.

Your code is very nice, when I reading and understanding your code,
I want to try if there is any other way to optimize it. 

Sometimes you don't need to reset drain_next, such as drain_link == true && 
drain_next == true, you don't need to set below one more time.

--
Jackie Liu

> 
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>> resend that patch, because reject by mail-list.
>> 
>> fs/io_uring.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 013e5ed..f4ec44a 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2938,12 +2938,14 @@ static void __io_queue_sqe(struct io_kiocb *req)
>> static void io_queue_sqe(struct io_kiocb *req)
>> {
>> 	int ret;
>> +	bool drain_link = req->flags & REQ_F_DRAIN_LINK;
>> 
>> -	if (unlikely(req->ctx->drain_next)) {
>> +	if (unlikely(req->ctx->drain_next && !drain_link)) {
>> 		req->flags |= REQ_F_IO_DRAIN;
>> 		req->ctx->drain_next = false;
>> +	} else if (unlikely(drain_link)) {
>> +		req->ctx->drain_next = true;
>> 	}
>> -	req->ctx->drain_next = (req->flags & REQ_F_DRAIN_LINK);
>> 
>> 	ret = io_req_defer(req);
>> 	if (ret) {
>> 
> 
> -- 
> Pavel Begunkov