* [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
* Re: [PATCH] net: use unlikely for dql_avail case
2019-09-25 2:40 [PATCH] net: use unlikely for dql_avail case 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
* 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
* [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
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-09-25 2:40 [PATCH] net: use unlikely for dql_avail case xiaolinkui
2019-09-25 12:25 ` Daniel Borkmann
2019-09-26 22:52 ` Eric Dumazet
-- strict thread matches above, loose matches on Subject: below --
2019-08-22 6:58 xiaolinkui
2019-08-24 21:23 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox