All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Xu <haoxu@linux.alibaba.com>
To: Pavel Begunkov <asml.silence@gmail.com>, Jens Axboe <axboe@kernel.dk>
Cc: io-uring@vger.kernel.org, Joseph Qi <joseph.qi@linux.alibaba.com>
Subject: Re: [PATCH 1/8] io-wq: code clean for io_wq_add_work_after()
Date: Wed, 29 Sep 2021 15:36:00 +0800	[thread overview]
Message-ID: <140f1e02-d400-b6c7-5c78-5eab6ac23f24@linux.alibaba.com> (raw)
In-Reply-To: <ec45dd61-194b-3611-dcd6-2a5440099575@gmail.com>

在 2021/9/28 下午7:08, Pavel Begunkov 写道:
> On 9/27/21 7:17 AM, Hao Xu wrote:
>> Remove a local variable.
> 
> It's there to help alias analysis, which usually can't do anything
> with pointer heavy logic. Compare ASMs below, before and after
> respectively:
> 	testq	%rax, %rax	# next
> 
> replaced with
> 	cmpq	$0, (%rdi)	#, node_2(D)->next
> 
> One extra memory dereference and a bigger binary
> 
> 
> =====================================================
> 
> wq_list_add_after:
> # fs/io_uring.c:271: 	struct io_wq_work_node *next = pos->next;
> 	movq	(%rsi), %rax	# pos_3(D)->next, next
> # fs/io_uring.c:273: 	pos->next = node;
> 	movq	%rdi, (%rsi)	# node, pos_3(D)->next
> # fs/io_uring.c:275: 	if (!next)
> 	testq	%rax, %rax	# next
> # fs/io_uring.c:274: 	node->next = next;
> 	movq	%rax, (%rdi)	# next, node_5(D)->next
> # fs/io_uring.c:275: 	if (!next)
> 	je	.L5927	#,
> 	ret	
> .L5927:
> # fs/io_uring.c:276: 		list->last = node;
> 	movq	%rdi, 8(%rdx)	# node, list_8(D)->last
> 	ret	
> 
> =====================================================
> 
> wq_list_add_after:
> # fs/io-wq.h:48: 	node->next = pos->next;
> 	movq	(%rsi), %rax	# pos_3(D)->next, _5
> # fs/io-wq.h:48: 	node->next = pos->next;
> 	movq	%rax, (%rdi)	# _5, node_2(D)->next
> # fs/io-wq.h:49: 	pos->next = node;
> 	movq	%rdi, (%rsi)	# node, pos_3(D)->next
> # fs/io-wq.h:50: 	if (!node->next)
> 	cmpq	$0, (%rdi)	#, node_2(D)->next
hmm, this is definitely not good, not sure why this is not optimised to
cmpq $0, %rax (haven't touched assembly for a long time..)
> 	je	.L5924	#,
> 	ret	
> .L5924:
> # fs/io-wq.h:51: 		list->last = node;
> 	movq	%rdi, 8(%rdx)	# node, list_4(D)->last
> 	ret	
> 
> 
>>
>> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
>> ---
>>   fs/io-wq.h | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/io-wq.h b/fs/io-wq.h
>> index bf5c4c533760..8369a51b65c0 100644
>> --- a/fs/io-wq.h
>> +++ b/fs/io-wq.h
>> @@ -33,11 +33,9 @@ static inline void wq_list_add_after(struct io_wq_work_node *node,
>>   				     struct io_wq_work_node *pos,
>>   				     struct io_wq_work_list *list)
>>   {
>> -	struct io_wq_work_node *next = pos->next;
>> -
>> +	node->next = pos->next;
>>   	pos->next = node;
>> -	node->next = next;
>> -	if (!next)
>> +	if (!node->next)
>>   		list->last = node;
>>   }
>>   
>>
> 


  reply	other threads:[~2021-09-29  7:36 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-27  6:17 [PATCH 0/6] task_work optimization Hao Xu
2021-09-27  6:17 ` [PATCH 1/8] io-wq: code clean for io_wq_add_work_after() Hao Xu
2021-09-28 11:08   ` Pavel Begunkov
2021-09-29  7:36     ` Hao Xu [this message]
2021-09-29 11:23       ` Pavel Begunkov
2021-09-27  6:17 ` [PATCH 2/8] io-wq: add helper to merge two wq_lists Hao Xu
2021-09-27 10:17   ` Hao Xu
2021-09-28 11:10   ` Pavel Begunkov
2021-09-28 16:48     ` Hao Xu
2021-09-29 11:23       ` Pavel Begunkov
2021-09-27  6:17 ` [PATCH 3/8] io_uring: add a limited tw list for irq completion work Hao Xu
2021-09-28 11:29   ` Pavel Begunkov
2021-09-28 16:55     ` Hao Xu
2021-09-29 11:25       ` Pavel Begunkov
2021-09-29 11:38     ` Hao Xu
2021-09-30  9:02       ` Pavel Begunkov
2021-09-30  3:21     ` Hao Xu
2021-09-27  6:17 ` [PATCH 4/8] io_uring: add helper for task work execution code Hao Xu
2021-09-27  6:17 ` [PATCH 5/8] io_uring: split io_req_complete_post() and add a helper Hao Xu
2021-09-27  6:17 ` [PATCH 6/8] io_uring: move up io_put_kbuf() and io_put_rw_kbuf() Hao Xu
2021-09-27  6:17 ` [PATCH 7/8] io_uring: add tw_ctx for io_uring_task Hao Xu
2021-09-27  6:17 ` [PATCH 8/8] io_uring: batch completion in prior_task_list Hao Xu
2021-09-27  6:21 ` [PATCH 0/6] task_work optimization Hao Xu
2021-09-27 10:51 [PATCH v2 0/8] " Hao Xu
2021-09-27 10:51 ` [PATCH 1/8] io-wq: code clean for io_wq_add_work_after() Hao Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=140f1e02-d400-b6c7-5c78-5eab6ac23f24@linux.alibaba.com \
    --to=haoxu@linux.alibaba.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=joseph.qi@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.