netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: use unlikely for dql_avail case
@ 2019-08-22  6:58 xiaolinkui
  2019-08-24 21:23 ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: xiaolinkui @ 2019-08-22  6:58 UTC (permalink / raw)
  To: davem, ast, daniel, kafai, songliubraving, yhs; +Cc: netdev, bpf, xiaolinkui

This is an unlikely case, use unlikely() on it seems logical.

Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>
---
 include/linux/netdevice.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..005f3da1b13d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3270,7 +3270,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
 	 */
 	smp_mb();
 
-	if (dql_avail(&dev_queue->dql) < 0)
+	if (unlikely(dql_avail(&dev_queue->dql) < 0))
 		return;
 
 	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: use unlikely for dql_avail case
  2019-08-22  6:58 [PATCH] net: use unlikely for dql_avail case xiaolinkui
@ 2019-08-24 21:23 ` David Miller
  0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2019-08-24 21:23 UTC (permalink / raw)
  To: xiaolinkui; +Cc: ast, daniel, kafai, songliubraving, yhs, netdev, bpf

From: xiaolinkui <xiaolinkui@kylinos.cn>
Date: Thu, 22 Aug 2019 14:58:16 +0800

> This is an unlikely case, use unlikely() on it seems logical.
> 
> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>

Applied to net-next.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: use unlikely for dql_avail case
  2019-09-25 12:25 ` Daniel Borkmann
@ 2019-09-26 22:52   ` Eric Dumazet
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2019-09-26 22:52 UTC (permalink / raw)
  To: Daniel Borkmann, xiaolinkui; +Cc: davem, netdev



On 9/25/19 5:25 AM, Daniel Borkmann wrote:
> On Wed, Sep 25, 2019 at 10:40:43AM +0800, xiaolinkui wrote:
>> This is an unlikely case, use unlikely() on it seems logical.
>>
>> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>
> 
> It's already here [0], but should probably rather get reverted instead
> due to lack of a more elaborate reasoning on why it needs to be done
> this way instead of letting compiler do it's job in this case. "Seems
> logical" is never a good technical explanation. Do you have any better
> analysis you performed prior to submitting the patch (twice by now)?
>

Yes, we need more details here.

We could probably save more cpu cycles checking if we can move
the smb_mb() after the dql_avail() check :)


 
> Thanks,
> Daniel
> 
>   [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3acd33d840d3ea3e1233d234605c85cbbf26054
> 
>> ---
>>  include/linux/netdevice.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 88292953aa6f..005f3da1b13d 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -3270,7 +3270,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
>>  	 */
>>  	smp_mb();
>>  
>> -	if (dql_avail(&dev_queue->dql) < 0)
>> +	if (unlikely(dql_avail(&dev_queue->dql) < 0))
>>  		return;
>>  
>>  	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
>> -- 
>> 2.17.1
>>
>>
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: use unlikely for dql_avail case
  2019-09-25  2:40 xiaolinkui
@ 2019-09-25 12:25 ` Daniel Borkmann
  2019-09-26 22:52   ` Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2019-09-25 12:25 UTC (permalink / raw)
  To: xiaolinkui; +Cc: davem, netdev

On Wed, Sep 25, 2019 at 10:40:43AM +0800, xiaolinkui wrote:
> This is an unlikely case, use unlikely() on it seems logical.
> 
> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>

It's already here [0], but should probably rather get reverted instead
due to lack of a more elaborate reasoning on why it needs to be done
this way instead of letting compiler do it's job in this case. "Seems
logical" is never a good technical explanation. Do you have any better
analysis you performed prior to submitting the patch (twice by now)?

Thanks,
Daniel

  [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3acd33d840d3ea3e1233d234605c85cbbf26054

> ---
>  include/linux/netdevice.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 88292953aa6f..005f3da1b13d 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3270,7 +3270,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
>  	 */
>  	smp_mb();
>  
> -	if (dql_avail(&dev_queue->dql) < 0)
> +	if (unlikely(dql_avail(&dev_queue->dql) < 0))
>  		return;
>  
>  	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
> -- 
> 2.17.1
> 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] net: use unlikely for dql_avail case
@ 2019-09-25  2:40 xiaolinkui
  2019-09-25 12:25 ` Daniel Borkmann
  0 siblings, 1 reply; 5+ messages in thread
From: xiaolinkui @ 2019-09-25  2:40 UTC (permalink / raw)
  To: davem; +Cc: netdev, xiaolinkui

This is an unlikely case, use unlikely() on it seems logical.

Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>
---
 include/linux/netdevice.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..005f3da1b13d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3270,7 +3270,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
 	 */
 	smp_mb();
 
-	if (dql_avail(&dev_queue->dql) < 0)
+	if (unlikely(dql_avail(&dev_queue->dql) < 0))
 		return;
 
 	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-09-26 22:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22  6:58 [PATCH] net: use unlikely for dql_avail case xiaolinkui
2019-08-24 21:23 ` David Miller
2019-09-25  2:40 xiaolinkui
2019-09-25 12:25 ` Daniel Borkmann
2019-09-26 22:52   ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).