netdev - Re: [PATCH net-next v7 2/3] net: Use netif_set_threaded_hint instead of netif_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAywjhTgBCrSQ9EhaNgoXYaKtqWn0Ks+8=nXo_-rnCU_hV4irw@mail.gmail.com>
Date: Tue, 22 Jul 2025 17:21:30 -0700
From: Samiullah Khawaja <skhawaja@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Eric Dumazet <edumazet@...gle.com>, "David S . Miller" <davem@...emloft.net>, 
	Paolo Abeni <pabeni@...hat.com>, almasrymina@...gle.com, willemb@...gle.com, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v7 2/3] net: Use netif_set_threaded_hint instead
 of netif_set_threaded in drivers

On Tue, Jul 22, 2025 at 3:41 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Tue, 22 Jul 2025 01:21:58 -0700 Eric Dumazet wrote:
> > > diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > > index 3a9ad4a9c1cb..ee7d07c86dcf 100644
> > > --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > > +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > > @@ -2688,7 +2688,7 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >         adapter->mii.mdio_write = atl1c_mdio_write;
> > >         adapter->mii.phy_id_mask = 0x1f;
> > >         adapter->mii.reg_num_mask = MDIO_CTRL_REG_MASK;
> > > -       netif_set_threaded(netdev, true);
> > > +       netif_set_threaded_hint(netdev);
> >
> > I have not seen a cover letter for this series ?
> >
> > netif_set_threaded_hint() name seems a bit strange, it seems drivers
> > intent is to enable threaded mode ?
> >
> > netif_threaded_enable() might be a better name.
+1
>
> Cover letter or at least a link to where this was suggested would indeed
> be useful. I may have suggested the name, and if so the thinking was
> that the API is for the driver to "recommend" that we default to
> threaded NAPI, likely because the device is IRQ-challenged.
> But no strong feelings if you prefer netif_set_threaded_enabled().
Yes. You suggested it, but I am changing it to netif_threaded_enable
as it maybe is more clear.
>
> Since this is a driver-facing API a kdoc may be useful:
>
> /**
>  * netif_set_threaded_hint() - default to using threaded NAPIs
>  * @dev: net_device instance
>  *
>  * Default NAPI instances of the device to run in threaded mode.
>  * This may be useful for devices where multiple NAPI instances
>  * get scheduled by a single interrupt. Threaded NAPI allows moving
>  * the NAPI processing to cores other than the core where IRQ is mapped.
>  *
>  * This function should be called before @dev is registered.
>  */
+1
>
> Since this is just a hint the function should be void. No caller cares
> about the return value anyway.
+1
>
> BTW I think we should delete the debugfs file for threaded config
> from mt76. debugfs is not uAPI, presumably.
+1

Do you think I should send a patch for that with this series? I was
planning to remove that separately.