public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue()
@ 2026-02-03 21:47 Eric Dumazet
  2026-02-04 18:24 ` Jamal Hadi Salim
  2026-02-05  4:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-02-03 21:47 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Jamal Hadi Salim, Jiri Pirko, netdev, eric.dumazet,
	Eric Dumazet

After 076433bd78d7 ("net_sched: sch_fq: add fast path
for mostly idle qdisc") we need to remove one unlikely()
because q->internal holds all the fast path packets.

       skb = fq_peek(&q->internal);
       if (unlikely(skb)) {
                q->internal.qlen--;

Calling INET_ECN_set_ce() is very unlikely.

These changes allow fq_dequeue_skb() to be (auto)inlined,
thus making fq_dequeue() faster.

$ scripts/bloat-o-meter -t vmlinux.0 vmlinux
add/remove: 2/2 grow/shrink: 0/1 up/down: 283/-269 (14)
Function                                     old     new   delta
INET_ECN_set_ce                                -     267    +267
__pfx_INET_ECN_set_ce                          -      16     +16
__pfx_fq_dequeue_skb                          16       -     -16
fq_dequeue_skb                               103       -    -103
fq_dequeue                                  1685    1535    -150
Total: Before=24886569, After=24886583, chg +0.00%

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_fq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 6e5f2f4f241546605f8ba37f96275446c8836eee..d0200ec8ada62e86f10d823556bedcaefb470e6c 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -665,7 +665,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 		return NULL;
 
 	skb = fq_peek(&q->internal);
-	if (unlikely(skb)) {
+	if (skb) {
 		q->internal.qlen--;
 		fq_dequeue_skb(sch, &q->internal, skb);
 		goto out;
@@ -716,7 +716,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 		}
 		prefetch(&skb->end);
 		fq_dequeue_skb(sch, f, skb);
-		if ((s64)(now - time_next_packet - q->ce_threshold) > 0) {
+		if (unlikely((s64)(now - time_next_packet - q->ce_threshold) > 0)) {
 			INET_ECN_set_ce(skb);
 			q->stat_ce_mark++;
 		}
-- 
2.53.0.rc2.204.g2597b5adb4-goog


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

* Re: [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue()
  2026-02-03 21:47 [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue() Eric Dumazet
@ 2026-02-04 18:24 ` Jamal Hadi Salim
  2026-02-04 18:43   ` Eric Dumazet
  2026-02-05  4:50 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: Jamal Hadi Salim @ 2026-02-04 18:24 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Jiri Pirko, netdev, eric.dumazet

On Tue, Feb 3, 2026 at 4:47 PM Eric Dumazet <edumazet@google.com> wrote:
>
> After 076433bd78d7 ("net_sched: sch_fq: add fast path
> for mostly idle qdisc") we need to remove one unlikely()
> because q->internal holds all the fast path packets.
>
>        skb = fq_peek(&q->internal);
>        if (unlikely(skb)) {
>                 q->internal.qlen--;
>
> Calling INET_ECN_set_ce() is very unlikely.
>
> These changes allow fq_dequeue_skb() to be (auto)inlined,
> thus making fq_dequeue() faster.
>
> $ scripts/bloat-o-meter -t vmlinux.0 vmlinux
> add/remove: 2/2 grow/shrink: 0/1 up/down: 283/-269 (14)
> Function                                     old     new   delta
> INET_ECN_set_ce                                -     267    +267
> __pfx_INET_ECN_set_ce                          -      16     +16
> __pfx_fq_dequeue_skb                          16       -     -16
> fq_dequeue_skb                               103       -    -103
> fq_dequeue                                  1685    1535    -150
> Total: Before=24886569, After=24886583, chg +0.00%
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/sched/sch_fq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> index 6e5f2f4f241546605f8ba37f96275446c8836eee..d0200ec8ada62e86f10d823556bedcaefb470e6c 100644
> --- a/net/sched/sch_fq.c
> +++ b/net/sched/sch_fq.c
> @@ -665,7 +665,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
>                 return NULL;
>
>         skb = fq_peek(&q->internal);
> -       if (unlikely(skb)) {
> +       if (skb) {
>                 q->internal.qlen--;
>                 fq_dequeue_skb(sch, &q->internal, skb);
>                 goto out;
> @@ -716,7 +716,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
>                 }
>                 prefetch(&skb->end);
>                 fq_dequeue_skb(sch, f, skb);
> -               if ((s64)(now - time_next_packet - q->ce_threshold) > 0) {
> +               if (unlikely((s64)(now - time_next_packet - q->ce_threshold) > 0)) {
>                         INET_ECN_set_ce(skb);
>                         q->stat_ce_mark++;
>                 }

While it looks rational you didnt mention any numbers.
Iam curious, is it always _guaranteed_ that inlining improves performance?

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

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

* Re: [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue()
  2026-02-04 18:24 ` Jamal Hadi Salim
@ 2026-02-04 18:43   ` Eric Dumazet
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-02-04 18:43 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Jiri Pirko, netdev, eric.dumazet

On Wed, Feb 4, 2026 at 7:25 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
>
> While it looks rational you didnt mention any numbers.

It is a very long process, one spot at a time.
Some cpus have a limited amount of return addresses in their RAS
branch predictor.
I am seeing high costs for functions needing a stack canary, for which
clang can not use tail calls.
When cpu returns back to these functions, we see a stall when
1) checking the stack canary
2) return from the function

I am not repeating the rationale on each patch, because we will have
maybe one hundred of them :/

This is also one of the reasons I am working on IPv6 stack to remove
stack canaries by all means.

> Iam curious, is it always _guaranteed_ that inlining improves performance?

Not always, but here the clang compiler was not inlining it because of
the wrong unlikely().

>
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

Note : A stack canary is enforced in fq_enqueue() (because of the
tofree[] array in fq_gc()),
I will send a patch to remove it soon.

Thanks !

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

* Re: [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue()
  2026-02-03 21:47 [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue() Eric Dumazet
  2026-02-04 18:24 ` Jamal Hadi Salim
@ 2026-02-05  4:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-02-05  4:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kuba, pabeni, horms, jhs, jiri, netdev, eric.dumazet

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  3 Feb 2026 21:47:16 +0000 you wrote:
> After 076433bd78d7 ("net_sched: sch_fq: add fast path
> for mostly idle qdisc") we need to remove one unlikely()
> because q->internal holds all the fast path packets.
> 
>        skb = fq_peek(&q->internal);
>        if (unlikely(skb)) {
>                 q->internal.qlen--;
> 
> [...]

Here is the summary with links:
  - [net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue()
    https://git.kernel.org/netdev/net-next/c/acd21dd2da19

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-02-05  4:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-03 21:47 [PATCH net-next] net_sched: sch_fq: tweak unlikely() hints in fq_dequeue() Eric Dumazet
2026-02-04 18:24 ` Jamal Hadi Salim
2026-02-04 18:43   ` Eric Dumazet
2026-02-05  4:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox