From: Jakub Kicinski <kuba@kernel.org>
To: Jinjian Song <jinjian.song@fibocom.com>
Cc: chandrashekar.devegowda@intel.com,
chiranjeevi.rapolu@linux.intel.com, haijun.liu@mediatek.com,
m.chetan.kumar@linux.intel.com, ricardo.martinez@linux.intel.com,
loic.poulain@linaro.org, ryazanov.s.a@gmail.com,
johannes@sipsolutions.net, davem@davemloft.net,
edumazet@google.com, pabeni@redhat.com,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org,
angelogioacchino.delregno@collabora.com,
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
corbet@lwn.net, linux-mediatek@lists.infradead.org,
helgaas@kernel.org, danielwinkler@google.com,
andrew+netdev@lunn.ch, horms@kernel.org
Subject: Re: [net v1] net: wwan: t7xx: Fix napi rx poll issue
Date: Thu, 15 May 2025 17:52:51 -0700 [thread overview]
Message-ID: <20250515175251.58b5123f@kernel.org> (raw)
In-Reply-To: <20250515031743.246178-1-jinjian.song@fibocom.com>
On Thu, 15 May 2025 11:17:42 +0800 Jinjian Song wrote:
> diff --git a/drivers/net/wwan/t7xx/t7xx_netdev.c b/drivers/net/wwan/t7xx/t7xx_netdev.c
> index 91fa082e9cab..2116ff81728b 100644
> --- a/drivers/net/wwan/t7xx/t7xx_netdev.c
> +++ b/drivers/net/wwan/t7xx/t7xx_netdev.c
> @@ -324,6 +324,7 @@ static void t7xx_ccmni_wwan_dellink(void *ctxt, struct net_device *dev, struct l
> if (WARN_ON(ctlb->ccmni_inst[if_id] != ccmni))
> return;
>
> + ctlb->ccmni_inst[if_id] = NULL;
> unregister_netdevice(dev);
I don't see any synchronization between this write and NAPI processing.
Is this safe? NAPI can be at any point of processing as we set the ptr
to NULL
--
pw-bot: cr
WARNING: multiple messages have this Message-ID (diff)
From: Jinjian Song <jinjian.song@fibocom.com>
To: kuba@kernel.org
Cc: andrew+netdev@lunn.ch, angelogioacchino.delregno@collabora.com,
chandrashekar.devegowda@intel.com,
chiranjeevi.rapolu@linux.intel.com, corbet@lwn.net,
danielwinkler@google.com, davem@davemloft.net,
edumazet@google.com, haijun.liu@mediatek.com, helgaas@kernel.org,
horms@kernel.org, jinjian.song@fibocom.com,
johannes@sipsolutions.net, linux-arm-kernel@lists.infradead.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mediatek@lists.infradead.org, loic.poulain@linaro.org,
m.chetan.kumar@linux.intel.com, matthias.bgg@gmail.com,
netdev@vger.kernel.org, pabeni@redhat.com,
ricardo.martinez@linux.intel.com, ryazanov.s.a@gmail.com
Subject: Re: [net v1] net: wwan: t7xx: Fix napi rx poll issue
Date: Fri, 16 May 2025 15:30:38 +0800 [thread overview]
Message-ID: <20250515175251.58b5123f@kernel.org> (raw)
Message-ID: <20250516073038.4aFr2lnLRV7tNDOwELbt_bBreBl3juz5DzwU4P8OCgw@z> (raw)
In-Reply-To: <20250515031743.246178-1-jinjian.song@fibocom.com>
>On Thu, 15 May 2025 11:17:42 +0800 Jinjian Song wrote:
>> diff --git a/drivers/net/wwan/t7xx/t7xx_netdev.c b/drivers/net/wwan/t7xx/t7xx_netdev.c
>> index 91fa082e9cab..2116ff81728b 100644
>> --- a/drivers/net/wwan/t7xx/t7xx_netdev.c
>> +++ b/drivers/net/wwan/t7xx/t7xx_netdev.c
>> @@ -324,6 +324,7 @@ static void t7xx_ccmni_wwan_dellink(void *ctxt, struct net_device *dev, struct l
>> if (WARN_ON(ctlb->ccmni_inst[if_id] != ccmni))
>> return;
>>
>> + ctlb->ccmni_inst[if_id] = NULL;
>> unregister_netdevice(dev);
>
>I don't see any synchronization between this write and NAPI processing.
>Is this safe? NAPI can be at any point of processing as we set the ptr
>to NULL
This panic occured in the scenario where there are frequent disconnect
and connect WWAN cellular on UI.
I debug the panic with gdb and found it as caused by an invalid net_device
during this process:
1.-> t7xx_dpmaif_napi_rx_poll
2.-> t7xx_ccmni_recv_skb
3.-> napi_gro_receive
4.-> dev_gro_receive
5.-> netif_elide_gro
One way, the net_device using in step 5 is valid, so "dev->features .." panic,
this net_device pass from t7xx_ccmni_recv_skb:
void t7xx_ccmni_recv_skb(...) {
[...]
ccmni = ccmni_ctlb->ccmni_inst[netif_id];
if (!ccmni) {
dev_kfree_skb(skb);
return;
}
net_dev = ccmni->dev;
skb->dev = net_dev;
[...]
napi_gro_receive(napi, skb);
[...]
}
Another way, WWAN disconnect -> wwan_ops.dellink -> t7xx_ccmni_wwan_dellink
-> unregister_netdevice(dev).
netdevice has been invalid, so t7xx_dpmaif_napi_rx_poll can't use it any more.
I mark ccmni_inst[if_id] = NULL with netdevice invalid at the same time.
It seems that a judgment is made every time ccmni_inst[x] is used in the driver,
and the synchronization on the 2 way might have been done when NAPI triggers
polling by napi_schedule and when WWAN trigger dellink.
So this should be safe.
Jinjian,
Best Regards.
next prev parent reply other threads:[~2025-05-16 0:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-15 3:17 [net v1] net: wwan: t7xx: Fix napi rx poll issue Jinjian Song
2025-05-16 0:52 ` Jakub Kicinski [this message]
2025-05-16 7:30 ` Jinjian Song
2025-05-16 15:48 ` Jakub Kicinski
2025-05-20 7:05 ` Jinjian Song
2025-05-20 22:39 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250515175251.58b5123f@kernel.org \
--to=kuba@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=angelogioacchino.delregno@collabora.com \
--cc=chandrashekar.devegowda@intel.com \
--cc=chiranjeevi.rapolu@linux.intel.com \
--cc=corbet@lwn.net \
--cc=danielwinkler@google.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=haijun.liu@mediatek.com \
--cc=helgaas@kernel.org \
--cc=horms@kernel.org \
--cc=jinjian.song@fibocom.com \
--cc=johannes@sipsolutions.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=loic.poulain@linaro.org \
--cc=m.chetan.kumar@linux.intel.com \
--cc=matthias.bgg@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=ricardo.martinez@linux.intel.com \
--cc=ryazanov.s.a@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.