diff mbox series

[net-next] net: call skb_defer_free_flush() from __napi_busy_loop()

Message ID 20240227210105.3815474-1-edumazet@google.com (mailing list archive)
State Accepted
Commit 1200097fa8f0d8e8ddfe5c554d8fa2bc03b2df92
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: call skb_defer_free_flush() from __napi_busy_loop() | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 947 this patch: 947
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang success Errors and warnings before: 958 this patch: 958
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 964 this patch: 964
netdev/checkpatch warning CHECK: Comparison to NULL could be written "skb"
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-02-28--18-00 (tests: 882)

Commit Message

Eric Dumazet Feb. 27, 2024, 9:01 p.m. UTC
skb_defer_free_flush() is currently called from net_rx_action()
and napi_threaded_poll().

We should also call it from __napi_busy_loop() otherwise
there is the risk the percpu queue can grow until an IPI
is forced from skb_attempt_defer_free() adding a latency spike.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Samiullah Khawaja <skhawaja@google.com>
Cc: Stanislav Fomichev <sdf@google.com>
---
 net/core/dev.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

Comments

Stanislav Fomichev Feb. 27, 2024, 9:07 p.m. UTC | #1
On 02/27, Eric Dumazet wrote:
> skb_defer_free_flush() is currently called from net_rx_action()
> and napi_threaded_poll().
> 
> We should also call it from __napi_busy_loop() otherwise
> there is the risk the percpu queue can grow until an IPI
> is forced from skb_attempt_defer_free() adding a latency spike.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Samiullah Khawaja <skhawaja@google.com>
> Cc: Stanislav Fomichev <sdf@google.com>

Acked-by: Stanislav Fomichev <sdf@google.com>

Thank you!
Jiri Pirko Feb. 28, 2024, 11:08 a.m. UTC | #2
Tue, Feb 27, 2024 at 10:01:04PM CET, edumazet@google.com wrote:
>skb_defer_free_flush() is currently called from net_rx_action()
>and napi_threaded_poll().
>
>We should also call it from __napi_busy_loop() otherwise
>there is the risk the percpu queue can grow until an IPI
>is forced from skb_attempt_defer_free() adding a latency spike.
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Samiullah Khawaja <skhawaja@google.com>
>Cc: Stanislav Fomichev <sdf@google.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
patchwork-bot+netdevbpf@kernel.org Feb. 29, 2024, 4:40 a.m. UTC | #3
Hello:

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

On Tue, 27 Feb 2024 21:01:04 +0000 you wrote:
> skb_defer_free_flush() is currently called from net_rx_action()
> and napi_threaded_poll().
> 
> We should also call it from __napi_busy_loop() otherwise
> there is the risk the percpu queue can grow until an IPI
> is forced from skb_attempt_defer_free() adding a latency spike.
> 
> [...]

Here is the summary with links:
  - [net-next] net: call skb_defer_free_flush() from __napi_busy_loop()
    https://git.kernel.org/netdev/net-next/c/1200097fa8f0

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index 275fd5259a4a92d0bd2e145d66a716248b6c2804..053fac78305c7322b894ceb07a925f7e64ed70aa 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6173,6 +6173,27 @@  struct napi_struct *napi_by_id(unsigned int napi_id)
 	return NULL;
 }
 
+static void skb_defer_free_flush(struct softnet_data *sd)
+{
+	struct sk_buff *skb, *next;
+
+	/* Paired with WRITE_ONCE() in skb_attempt_defer_free() */
+	if (!READ_ONCE(sd->defer_list))
+		return;
+
+	spin_lock(&sd->defer_lock);
+	skb = sd->defer_list;
+	sd->defer_list = NULL;
+	sd->defer_count = 0;
+	spin_unlock(&sd->defer_lock);
+
+	while (skb != NULL) {
+		next = skb->next;
+		napi_consume_skb(skb, 1);
+		skb = next;
+	}
+}
+
 #if defined(CONFIG_NET_RX_BUSY_POLL)
 
 static void __busy_poll_stop(struct napi_struct *napi, bool skip_schedule)
@@ -6297,6 +6318,7 @@  static void __napi_busy_loop(unsigned int napi_id,
 		if (work > 0)
 			__NET_ADD_STATS(dev_net(napi->dev),
 					LINUX_MIB_BUSYPOLLRXPACKETS, work);
+		skb_defer_free_flush(this_cpu_ptr(&softnet_data));
 		local_bh_enable();
 
 		if (!loop_end || loop_end(loop_end_arg, start_time))
@@ -6726,27 +6748,6 @@  static int napi_thread_wait(struct napi_struct *napi)
 	return -1;
 }
 
-static void skb_defer_free_flush(struct softnet_data *sd)
-{
-	struct sk_buff *skb, *next;
-
-	/* Paired with WRITE_ONCE() in skb_attempt_defer_free() */
-	if (!READ_ONCE(sd->defer_list))
-		return;
-
-	spin_lock(&sd->defer_lock);
-	skb = sd->defer_list;
-	sd->defer_list = NULL;
-	sd->defer_count = 0;
-	spin_unlock(&sd->defer_lock);
-
-	while (skb != NULL) {
-		next = skb->next;
-		napi_consume_skb(skb, 1);
-		skb = next;
-	}
-}
-
 static int napi_threaded_poll(void *data)
 {
 	struct napi_struct *napi = data;