[PATCH AUTOSEL 5.10 01/15] page_pool: avoid infinite loop to schedule delayed worker

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH AUTOSEL 5.10 01/15] page_pool: avoid infinite loop to schedule delayed worker
@ 2025-04-03 19:09 Sasha Levin
  2025-04-03 19:10 ` [PATCH AUTOSEL 5.10 13/15] net: vlan: don't propagate flags on open Sasha Levin
  0 siblings, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2025-04-03 19:09 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Jason Xing, Mina Almasry, Paolo Abeni, Sasha Levin, hawk,
	ilias.apalodimas, davem, edumazet, kuba, netdev

From: Jason Xing <kerneljasonxing@gmail.com>

[ Upstream commit 43130d02baa137033c25297aaae95fd0edc41654 ]

We noticed the kworker in page_pool_release_retry() was waken
up repeatedly and infinitely in production because of the
buggy driver causing the inflight less than 0 and warning
us in page_pool_inflight()[1].

Since the inflight value goes negative, it means we should
not expect the whole page_pool to get back to work normally.

This patch mitigates the adverse effect by not rescheduling
the kworker when detecting the inflight negative in
page_pool_release_retry().

[1]
[Mon Feb 10 20:36:11 2025] ------------[ cut here ]------------
[Mon Feb 10 20:36:11 2025] Negative(-51446) inflight packet-pages
...
[Mon Feb 10 20:36:11 2025] Call Trace:
[Mon Feb 10 20:36:11 2025]  page_pool_release_retry+0x23/0x70
[Mon Feb 10 20:36:11 2025]  process_one_work+0x1b1/0x370
[Mon Feb 10 20:36:11 2025]  worker_thread+0x37/0x3a0
[Mon Feb 10 20:36:11 2025]  kthread+0x11a/0x140
[Mon Feb 10 20:36:11 2025]  ? process_one_work+0x370/0x370
[Mon Feb 10 20:36:11 2025]  ? __kthread_cancel_work+0x40/0x40
[Mon Feb 10 20:36:11 2025]  ret_from_fork+0x35/0x40
[Mon Feb 10 20:36:11 2025] ---[ end trace ebffe800f33e7e34 ]---
Note: before this patch, the above calltrace would flood the
dmesg due to repeated reschedule of release_dw kworker.

Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Link: https://patch.msgid.link/20250214064250.85987-1-kerneljasonxing@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/core/page_pool.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 08fbf4049c108..a11809b3149b4 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -485,7 +485,13 @@ static void page_pool_release_retry(struct work_struct *wq)
 	int inflight;
 
 	inflight = page_pool_release(pool);
-	if (!inflight)
+	/* In rare cases, a driver bug may cause inflight to go negative.
+	 * Don't reschedule release if inflight is 0 or negative.
+	 * - If 0, the page_pool has been destroyed
+	 * - if negative, we will never recover
+	 * in both cases no reschedule is necessary.
+	 */
+	if (inflight <= 0)
 		return;
 
 	/* Periodic warning */
-- 
2.39.5


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

* [PATCH AUTOSEL 5.10 13/15] net: vlan: don't propagate flags on open
  2025-04-03 19:09 [PATCH AUTOSEL 5.10 01/15] page_pool: avoid infinite loop to schedule delayed worker Sasha Levin
@ 2025-04-03 19:10 ` Sasha Levin
  2025-04-18 17:01   ` Pavel Machek
  0 siblings, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2025-04-03 19:10 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Stanislav Fomichev, syzbot+b0c03d76056ef6cd12a6, Simon Horman,
	Paolo Abeni, Sasha Levin, davem, edumazet, kuba,
	aleksander.lobakin, kory.maincent, netdev

From: Stanislav Fomichev <sdf@fomichev.me>

[ Upstream commit 27b918007d96402aba10ed52a6af8015230f1793 ]

With the device instance lock, there is now a possibility of a deadlock:

[    1.211455] ============================================
[    1.211571] WARNING: possible recursive locking detected
[    1.211687] 6.14.0-rc5-01215-g032756b4ca7a-dirty #5 Not tainted
[    1.211823] --------------------------------------------
[    1.211936] ip/184 is trying to acquire lock:
[    1.212032] ffff8881024a4c30 (&dev->lock){+.+.}-{4:4}, at: dev_set_allmulti+0x4e/0xb0
[    1.212207]
[    1.212207] but task is already holding lock:
[    1.212332] ffff8881024a4c30 (&dev->lock){+.+.}-{4:4}, at: dev_open+0x50/0xb0
[    1.212487]
[    1.212487] other info that might help us debug this:
[    1.212626]  Possible unsafe locking scenario:
[    1.212626]
[    1.212751]        CPU0
[    1.212815]        ----
[    1.212871]   lock(&dev->lock);
[    1.212944]   lock(&dev->lock);
[    1.213016]
[    1.213016]  *** DEADLOCK ***
[    1.213016]
[    1.213143]  May be due to missing lock nesting notation
[    1.213143]
[    1.213294] 3 locks held by ip/184:
[    1.213371]  #0: ffffffff838b53e0 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_nets_lock+0x1b/0xa0
[    1.213543]  #1: ffffffff84e5fc70 (&net->rtnl_mutex){+.+.}-{4:4}, at: rtnl_nets_lock+0x37/0xa0
[    1.213727]  #2: ffff8881024a4c30 (&dev->lock){+.+.}-{4:4}, at: dev_open+0x50/0xb0
[    1.213895]
[    1.213895] stack backtrace:
[    1.213991] CPU: 0 UID: 0 PID: 184 Comm: ip Not tainted 6.14.0-rc5-01215-g032756b4ca7a-dirty #5
[    1.213993] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014
[    1.213994] Call Trace:
[    1.213995]  <TASK>
[    1.213996]  dump_stack_lvl+0x8e/0xd0
[    1.214000]  print_deadlock_bug+0x28b/0x2a0
[    1.214020]  lock_acquire+0xea/0x2a0
[    1.214027]  __mutex_lock+0xbf/0xd40
[    1.214038]  dev_set_allmulti+0x4e/0xb0 # real_dev->flags & IFF_ALLMULTI
[    1.214040]  vlan_dev_open+0xa5/0x170 # ndo_open on vlandev
[    1.214042]  __dev_open+0x145/0x270
[    1.214046]  __dev_change_flags+0xb0/0x1e0
[    1.214051]  netif_change_flags+0x22/0x60 # IFF_UP vlandev
[    1.214053]  dev_change_flags+0x61/0xb0 # for each device in group from dev->vlan_info
[    1.214055]  vlan_device_event+0x766/0x7c0 # on netdevsim0
[    1.214058]  notifier_call_chain+0x78/0x120
[    1.214062]  netif_open+0x6d/0x90
[    1.214064]  dev_open+0x5b/0xb0 # locks netdevsim0
[    1.214066]  bond_enslave+0x64c/0x1230
[    1.214075]  do_set_master+0x175/0x1e0 # on netdevsim0
[    1.214077]  do_setlink+0x516/0x13b0
[    1.214094]  rtnl_newlink+0xaba/0xb80
[    1.214132]  rtnetlink_rcv_msg+0x440/0x490
[    1.214144]  netlink_rcv_skb+0xeb/0x120
[    1.214150]  netlink_unicast+0x1f9/0x320
[    1.214153]  netlink_sendmsg+0x346/0x3f0
[    1.214157]  __sock_sendmsg+0x86/0xb0
[    1.214160]  ____sys_sendmsg+0x1c8/0x220
[    1.214164]  ___sys_sendmsg+0x28f/0x2d0
[    1.214179]  __x64_sys_sendmsg+0xef/0x140
[    1.214184]  do_syscall_64+0xec/0x1d0
[    1.214190]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[    1.214191] RIP: 0033:0x7f2d1b4a7e56

Device setup:

     netdevsim0 (down)
     ^        ^
  bond        netdevsim1.100@netdevsim1 allmulticast=on (down)

When we enslave the lower device (netdevsim0) which has a vlan, we
propagate vlan's allmuti/promisc flags during ndo_open. This causes
(re)locking on of the real_dev.

Propagate allmulti/promisc on flags change, not on the open. There
is a slight semantics change that vlans that are down now propagate
the flags, but this seems unlikely to result in the real issues.

Reproducer:

  echo 0 1 > /sys/bus/netdevsim/new_device

  dev_path=$(ls -d /sys/bus/netdevsim/devices/netdevsim0/net/*)
  dev=$(echo $dev_path | rev | cut -d/ -f1 | rev)

  ip link set dev $dev name netdevsim0
  ip link set dev netdevsim0 up

  ip link add link netdevsim0 name netdevsim0.100 type vlan id 100
  ip link set dev netdevsim0.100 allmulticast on down
  ip link add name bond1 type bond mode 802.3ad
  ip link set dev netdevsim0 down
  ip link set dev netdevsim0 master bond1
  ip link set dev bond1 up
  ip link show

Reported-by: syzbot+b0c03d76056ef6cd12a6@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/Z9CfXjLMKn6VLG5d@mini-arch/T/#m15ba130f53227c883e79fb969687d69d670337a0
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250313100657.2287455-1-sdf@fomichev.me
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/8021q/vlan_dev.c | 31 ++++---------------------------
 1 file changed, 4 insertions(+), 27 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index b7cf430006e55..c134f8210b21b 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -272,17 +272,6 @@ static int vlan_dev_open(struct net_device *dev)
 			goto out;
 	}
 
-	if (dev->flags & IFF_ALLMULTI) {
-		err = dev_set_allmulti(real_dev, 1);
-		if (err < 0)
-			goto del_unicast;
-	}
-	if (dev->flags & IFF_PROMISC) {
-		err = dev_set_promiscuity(real_dev, 1);
-		if (err < 0)
-			goto clear_allmulti;
-	}
-
 	ether_addr_copy(vlan->real_dev_addr, real_dev->dev_addr);
 
 	if (vlan->flags & VLAN_FLAG_GVRP)
@@ -296,12 +285,6 @@ static int vlan_dev_open(struct net_device *dev)
 		netif_carrier_on(dev);
 	return 0;
 
-clear_allmulti:
-	if (dev->flags & IFF_ALLMULTI)
-		dev_set_allmulti(real_dev, -1);
-del_unicast:
-	if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
-		dev_uc_del(real_dev, dev->dev_addr);
 out:
 	netif_carrier_off(dev);
 	return err;
@@ -314,10 +297,6 @@ static int vlan_dev_stop(struct net_device *dev)
 
 	dev_mc_unsync(real_dev, dev);
 	dev_uc_unsync(real_dev, dev);
-	if (dev->flags & IFF_ALLMULTI)
-		dev_set_allmulti(real_dev, -1);
-	if (dev->flags & IFF_PROMISC)
-		dev_set_promiscuity(real_dev, -1);
 
 	if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
 		dev_uc_del(real_dev, dev->dev_addr);
@@ -474,12 +453,10 @@ static void vlan_dev_change_rx_flags(struct net_device *dev, int change)
 {
 	struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
 
-	if (dev->flags & IFF_UP) {
-		if (change & IFF_ALLMULTI)
-			dev_set_allmulti(real_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
-		if (change & IFF_PROMISC)
-			dev_set_promiscuity(real_dev, dev->flags & IFF_PROMISC ? 1 : -1);
-	}
+	if (change & IFF_ALLMULTI)
+		dev_set_allmulti(real_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
+	if (change & IFF_PROMISC)
+		dev_set_promiscuity(real_dev, dev->flags & IFF_PROMISC ? 1 : -1);
 }
 
 static void vlan_dev_set_rx_mode(struct net_device *vlan_dev)
-- 
2.39.5


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

* Re: [PATCH AUTOSEL 5.10 13/15] net: vlan: don't propagate flags on open
  2025-04-03 19:10 ` [PATCH AUTOSEL 5.10 13/15] net: vlan: don't propagate flags on open Sasha Levin
@ 2025-04-18 17:01   ` Pavel Machek
  0 siblings, 0 replies; 3+ messages in thread
From: Pavel Machek @ 2025-04-18 17:01 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-kernel, stable, Stanislav Fomichev,
	syzbot+b0c03d76056ef6cd12a6, Simon Horman, Paolo Abeni, davem,
	edumazet, kuba, aleksander.lobakin, kory.maincent, netdev

[-- Attachment #1: Type: text/plain, Size: 2522 bytes --]

On Thu 2025-04-03 15:10:00, Sasha Levin wrote:
> From: Stanislav Fomichev <sdf@fomichev.me>
> 
> [ Upstream commit 27b918007d96402aba10ed52a6af8015230f1793 ]
> 
> With the device instance lock, there is now a possibility of a deadlock:

"now". Does the same problem exist in 5.10?

Best regards,
								Pavel

> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -272,17 +272,6 @@ static int vlan_dev_open(struct net_device *dev)
>  			goto out;
>  	}
>  
> -	if (dev->flags & IFF_ALLMULTI) {
> -		err = dev_set_allmulti(real_dev, 1);
> -		if (err < 0)
> -			goto del_unicast;
> -	}
> -	if (dev->flags & IFF_PROMISC) {
> -		err = dev_set_promiscuity(real_dev, 1);
> -		if (err < 0)
> -			goto clear_allmulti;
> -	}
> -
>  	ether_addr_copy(vlan->real_dev_addr, real_dev->dev_addr);
>  
>  	if (vlan->flags & VLAN_FLAG_GVRP)
> @@ -296,12 +285,6 @@ static int vlan_dev_open(struct net_device *dev)
>  		netif_carrier_on(dev);
>  	return 0;
>  
> -clear_allmulti:
> -	if (dev->flags & IFF_ALLMULTI)
> -		dev_set_allmulti(real_dev, -1);
> -del_unicast:
> -	if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
> -		dev_uc_del(real_dev, dev->dev_addr);
>  out:
>  	netif_carrier_off(dev);
>  	return err;
> @@ -314,10 +297,6 @@ static int vlan_dev_stop(struct net_device *dev)
>  
>  	dev_mc_unsync(real_dev, dev);
>  	dev_uc_unsync(real_dev, dev);
> -	if (dev->flags & IFF_ALLMULTI)
> -		dev_set_allmulti(real_dev, -1);
> -	if (dev->flags & IFF_PROMISC)
> -		dev_set_promiscuity(real_dev, -1);
>  
>  	if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
>  		dev_uc_del(real_dev, dev->dev_addr);
> @@ -474,12 +453,10 @@ static void vlan_dev_change_rx_flags(struct net_device *dev, int change)
>  {
>  	struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
>  
> -	if (dev->flags & IFF_UP) {
> -		if (change & IFF_ALLMULTI)
> -			dev_set_allmulti(real_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
> -		if (change & IFF_PROMISC)
> -			dev_set_promiscuity(real_dev, dev->flags & IFF_PROMISC ? 1 : -1);
> -	}
> +	if (change & IFF_ALLMULTI)
> +		dev_set_allmulti(real_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
> +	if (change & IFF_PROMISC)
> +		dev_set_promiscuity(real_dev, dev->flags & IFF_PROMISC ? 1 : -1);
>  }
>  
>  static void vlan_dev_set_rx_mode(struct net_device *vlan_dev)

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

end of thread, other threads:[~2025-04-18 17:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-03 19:09 [PATCH AUTOSEL 5.10 01/15] page_pool: avoid infinite loop to schedule delayed worker Sasha Levin
2025-04-03 19:10 ` [PATCH AUTOSEL 5.10 13/15] net: vlan: don't propagate flags on open Sasha Levin
2025-04-18 17:01   ` Pavel Machek

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).