From: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "Andrew Lunn" <andrew+netdev@lunn.ch>,
davem@davemloft.net, "Eric Dumazet" <edumazet@google.com>,
"Paolo Abeni" <pabeni@redhat.com>,
"Simon Horman" <horms@kernel.org>,
"Russell King" <linux@armlinux.org.uk>,
"Kuniyuki Iwashima" <kuniyu@google.com>,
"Stanislav Fomichev" <sdf.kernel@gmail.com>,
thomas.petazzoni@bootlin.com,
"Alexis Lothoré" <alexis.lothore@bootlin.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net] net: keep the NAPI_DISABLE flag while napi is disabled
Date: Wed, 30 Sep 2026 22:57:28 +0200 [thread overview]
Message-ID: <82cd7194-bc61-46b1-ae44-a78b33047017@bootlin.com> (raw)
In-Reply-To: <20260930134109.0fc3824a@kernel.org>
On 9/30/26 22:41, Jakub Kicinski wrote:
> On Mon, 28 Sep 2026 09:08:29 +0200 Maxime Chevallier wrote:
>> On 9/28/26 00:41, Jakub Kicinski wrote:
>>> On Sat, 26 Sep 2026 21:47:13 +0200 Maxime Chevallier wrote:
>>>> This seems to be more general than stmmac though, there are lots of
>>>> drivers that create the napi instances in .probe(), so they still live
>>>> outside of .open()/.close(). It was verified by running the following on
>>>> an mvneta board :
>>>
>>> Not sure this works, you assume ownership if DISABLE is set but we set
>>> it to start the shutdown, not once we stopped the poller.
>>>
>>> We've been tempted to "fix" this multiple times. I'd prefer to fix
>>> drivers.
>>
>> There's 2 classes of problems then
>>
>> - drivers that create a napi instance but don't use it even when
>> admin up (stmmac, I think it used to be the case for i40e looking
>> at the history). Fixing the driver makes sense then
>>
>> - drivers that add napi instances at .probe() time, in which
>> case setting threaded to 1 -> 0 when the interface is admin down
>> is always going to hang. There's quite a few of them :
>>
>> git grep -p netif_napi_add | grep probe | wc -l
>> 75
>
> TIL git grep -p, nice!
>
>> So at least 75 drivers call netif_napi_add from their probe
>> functions.
>
> Mostly old drivers, half of which we should probably delete :S
>
> Can we not refuse disabling the threaded state if the device
> is down and has NAPIs?
I don't see why not :) TBH this is a bit outside of my comfort zone,
I'll spin something and give it a test on as much HW as possible.
Thanks,
Maxime
prev parent reply other threads:[~2026-09-30 20:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-26 19:47 Maxime Chevallier
2026-09-27 22:41 ` Jakub Kicinski
2026-09-28 7:08 ` Maxime Chevallier
2026-09-30 20:41 ` Jakub Kicinski
2026-09-30 20:57 ` Maxime Chevallier [this message]
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=82cd7194-bc61-46b1-ae44-a78b33047017@bootlin.com \
--to=maxime.chevallier@bootlin.com \
--cc=alexis.lothore@bootlin.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=kuniyu@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf.kernel@gmail.com \
--cc=thomas.petazzoni@bootlin.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®