netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int
@ 2023-07-11 11:07 menglong8.dong
  2023-07-12  3:09 ` Jakub Kicinski
  2023-07-12  3:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: menglong8.dong @ 2023-07-11 11:07 UTC (permalink / raw)
  To: michael.chan, leon
  Cc: davem, edumazet, kuba, pabeni, netdev, linux-kernel,
	Menglong Dong, Leon Romanovsky

From: Menglong Dong <imagedong@tencent.com>

In bnxt_tx_int(), the skb in the tx ring buffer will be freed after the
transmission completes with dev_kfree_skb_any(), which will produce
the noise on the tracepoint "skb:kfree_skb":

$ perf script record -e skb:kfree_skb -a
$ perf script
  swapper     0 [014] 12814.337522: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [003] 12814.338318: skb:kfree_skb: skbaddr=0xffff888108380600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12814.375258: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12814.451960: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [008] 12814.562166: skb:kfree_skb: skbaddr=0xffff888112664600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12814.732517: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12814.800608: skb:kfree_skb: skbaddr=0xffff88810025d100 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12814.861501: skb:kfree_skb: skbaddr=0xffff888108295a00 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12815.377038: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
  swapper     0 [014] 12815.395530: skb:kfree_skb: skbaddr=0xffff88818f145ee0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED

And the call stack is like this:

$ perf script record -e skb:kfree_skb -a --call-graph fp
$ perf script
swapper     0 [015] 12915.386236: skb:kfree_skb: skbaddr=0xffff88b0473cd000 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
        ffffffff81e5b0a7 kfree_skb_reason+0x117 (vmlinux)
        ffffffff81e5b0a7 kfree_skb_reason+0x117 (vmlinux)
        ffffffff81e6dd1e dev_kfree_skb_any_reason+0x2e (vmlinux)
        ffffffffa04510d5 bnxt_tx_int+0x95 ([bnxt_en])
        ffffffffa0454ee5 __bnxt_poll_work_done+0x25 ([bnxt_en])
        ffffffffa045b9d2 bnxt_poll+0x72 ([bnxt_en])
        ffffffff81e7b47e __napi_poll+0x2e (vmlinux)
        ffffffff81e7bb34 net_rx_action+0x294 (vmlinux)
        ffffffff8218ff5e __do_softirq+0xfe (vmlinux)
        ffffffff810ad51e irq_exit_rcu+0x7e (vmlinux)
        ffffffff82175bb5 common_interrupt+0xc5 (vmlinux)
        ffffffff82200d27 asm_common_interrupt+0x27 (vmlinux)
        ffffffff8217ae54 intel_idle_irq+0x64 (vmlinux)
        ffffffff81dea7ae cpuidle_enter+0x2e (vmlinux)
        ffffffff811057e3 call_cpuidle+0x23 (vmlinux)
        ffffffff8110b16a do_idle+0x1ea (vmlinux)
        ffffffff8110b3cd cpu_startup_entry+0x1d (vmlinux)
        ffffffff8106dc08 start_secondary+0x118 (vmlinux)
        ffffffff81000263 secondary_startup_64_no_verify+0x17e (vmlinux)

Replace dev_kfree_skb_any() with dev_consume_skb_any() in bnxt_tx_int()
to reduce the noise.

Signed-off-by: Menglong Dong <imagedong@tencent.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
---
v2:
- be more specific in the commit message
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index e5b54e6025be..d84ded8db93d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -685,7 +685,7 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
 next_tx_int:
 		cons = NEXT_TX(cons);
 
-		dev_kfree_skb_any(skb);
+		dev_consume_skb_any(skb);
 	}
 
 	WRITE_ONCE(txr->tx_cons, cons);
-- 
2.40.1


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

* Re: [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int
  2023-07-11 11:07 [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int menglong8.dong
@ 2023-07-12  3:09 ` Jakub Kicinski
  2023-07-12  5:48   ` Leon Romanovsky
  2023-07-12  3:30 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2023-07-12  3:09 UTC (permalink / raw)
  To: menglong8.dong
  Cc: michael.chan, leon, davem, edumazet, pabeni, netdev, linux-kernel,
	Menglong Dong, Leon Romanovsky

On Tue, 11 Jul 2023 19:07:43 +0800 menglong8.dong@gmail.com wrote:
> In bnxt_tx_int(), the skb in the tx ring buffer will be freed after the
> transmission completes with dev_kfree_skb_any(), which will produce
> the noise on the tracepoint "skb:kfree_skb":
> 
> $ perf script record -e skb:kfree_skb -a
> $ perf script
>   swapper     0 [014] 12814.337522: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [003] 12814.338318: skb:kfree_skb: skbaddr=0xffff888108380600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.375258: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.451960: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [008] 12814.562166: skb:kfree_skb: skbaddr=0xffff888112664600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.732517: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.800608: skb:kfree_skb: skbaddr=0xffff88810025d100 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.861501: skb:kfree_skb: skbaddr=0xffff888108295a00 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12815.377038: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12815.395530: skb:kfree_skb: skbaddr=0xffff88818f145ee0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED

I think this is way too verbose, people looking at networking code 
are expected to understand kfree_skb vs consume_skb. Your v1 was fine,
I'm going to apply it.
-- 
pw-bot: na

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

* Re: [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int
  2023-07-11 11:07 [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int menglong8.dong
  2023-07-12  3:09 ` Jakub Kicinski
@ 2023-07-12  3:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-07-12  3:30 UTC (permalink / raw)
  To: Menglong Dong
  Cc: michael.chan, leon, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, imagedong, leonro

Hello:

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

On Tue, 11 Jul 2023 19:07:43 +0800 you wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> In bnxt_tx_int(), the skb in the tx ring buffer will be freed after the
> transmission completes with dev_kfree_skb_any(), which will produce
> the noise on the tracepoint "skb:kfree_skb":
> 
> $ perf script record -e skb:kfree_skb -a
> $ perf script
>   swapper     0 [014] 12814.337522: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [003] 12814.338318: skb:kfree_skb: skbaddr=0xffff888108380600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.375258: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.451960: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [008] 12814.562166: skb:kfree_skb: skbaddr=0xffff888112664600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.732517: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.800608: skb:kfree_skb: skbaddr=0xffff88810025d100 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12814.861501: skb:kfree_skb: skbaddr=0xffff888108295a00 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12815.377038: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
>   swapper     0 [014] 12815.395530: skb:kfree_skb: skbaddr=0xffff88818f145ee0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> 
> [...]

Here is the summary with links:
  - [net-next,v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int
    https://git.kernel.org/netdev/net-next/c/47b7acfb016b

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

* Re: [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int
  2023-07-12  3:09 ` Jakub Kicinski
@ 2023-07-12  5:48   ` Leon Romanovsky
  0 siblings, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2023-07-12  5:48 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: menglong8.dong, michael.chan, davem, edumazet, pabeni, netdev,
	linux-kernel, Menglong Dong

On Tue, Jul 11, 2023 at 08:09:55PM -0700, Jakub Kicinski wrote:
> On Tue, 11 Jul 2023 19:07:43 +0800 menglong8.dong@gmail.com wrote:
> > In bnxt_tx_int(), the skb in the tx ring buffer will be freed after the
> > transmission completes with dev_kfree_skb_any(), which will produce
> > the noise on the tracepoint "skb:kfree_skb":
> > 
> > $ perf script record -e skb:kfree_skb -a
> > $ perf script
> >   swapper     0 [014] 12814.337522: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [003] 12814.338318: skb:kfree_skb: skbaddr=0xffff888108380600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12814.375258: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12814.451960: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [008] 12814.562166: skb:kfree_skb: skbaddr=0xffff888112664600 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12814.732517: skb:kfree_skb: skbaddr=0xffff88818f145ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12814.800608: skb:kfree_skb: skbaddr=0xffff88810025d100 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12814.861501: skb:kfree_skb: skbaddr=0xffff888108295a00 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12815.377038: skb:kfree_skb: skbaddr=0xffff88818f147ce0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> >   swapper     0 [014] 12815.395530: skb:kfree_skb: skbaddr=0xffff88818f145ee0 protocol=2048 location=dev_kfree_skb_any_reason+0x2e reason: NOT_SPECIFIED
> 
> I think this is way too verbose, people looking at networking code 
> are expected to understand kfree_skb vs consume_skb. 

There are many people who look in git log and don't understand networking
code at all. Verbose commit messages are intended for them.

Thanks

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

end of thread, other threads:[~2023-07-12  5:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-11 11:07 [PATCH net-next v2] bnxt_en: use dev_consume_skb_any() in bnxt_tx_int menglong8.dong
2023-07-12  3:09 ` Jakub Kicinski
2023-07-12  5:48   ` Leon Romanovsky
2023-07-12  3:30 ` 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;
as well as URLs for NNTP newsgroup(s).