* atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
@ 2022-01-21 21:57 Sergei Trofimovich
2022-01-21 23:45 ` Andrew Lunn
0 siblings, 1 reply; 8+ messages in thread
From: Sergei Trofimovich @ 2022-01-21 21:57 UTC (permalink / raw)
To: netdev
Hia atl1c maintainers!
This cosmetics bothered me for some time: atl1c driver
shows unexpanded % in kernel thread names. Looks like a
minor bug:
$ ping -f 172.16.0.1 # host1
$ top # host2
...
621 root 20 0 0 0 0 S 11.0 0.0 0:05.01 napi/eth%d-385
622 root 20 0 0 0 0 S 5.6 0.0 0:02.64 napi/eth%d-386
...
Was happening for a few years. Likely not a recent regression.
System:
- linux-5.16.1
- x86_64
- 02:00.0 Ethernet controller: Qualcomm Atheros AR8151 v2.0 Gigabit Ethernet (rev c0)
From what I understand thread name comes from somewhere around:
net/core/dev.c:
int dev_set_threaded(struct net_device *dev, bool threaded)
...
err = napi_kthread_create(napi);
...
static int napi_kthread_create(struct napi_struct *n)
...
n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
drivers/net/ethernet/atheros/atl1c/atl1c_main.c:
static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
...
dev_set_threaded(netdev, true);
${somewhere} (not sure where):
...
strcpy(netdev->name, "eth%d");
I was not able to pinpoint where expansion should ideally happen.
Looks like many driver do `strcpy(netdev->name, "eth%d");` style
initialization and almost none call `dev_set_threaded(netdev, true);`.
Can you help me find it out how it should be fixed?
Thank you!
--
Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-21 21:57 atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d Sergei Trofimovich
@ 2022-01-21 23:45 ` Andrew Lunn
2022-01-22 1:03 ` Stephen Hemminger
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2022-01-21 23:45 UTC (permalink / raw)
To: Sergei Trofimovich; +Cc: netdev
On Fri, Jan 21, 2022 at 09:57:47PM +0000, Sergei Trofimovich wrote:
> Hia atl1c maintainers!
>
> This cosmetics bothered me for some time: atl1c driver
> shows unexpanded % in kernel thread names. Looks like a
> minor bug:
>
> $ ping -f 172.16.0.1 # host1
> $ top # host2
> ...
> 621 root 20 0 0 0 0 S 11.0 0.0 0:05.01 napi/eth%d-385
> 622 root 20 0 0 0 0 S 5.6 0.0 0:02.64 napi/eth%d-386
> ...
>
> Was happening for a few years. Likely not a recent regression.
>
> System:
> - linux-5.16.1
> - x86_64
> - 02:00.0 Ethernet controller: Qualcomm Atheros AR8151 v2.0 Gigabit Ethernet (rev c0)
>
> >From what I understand thread name comes from somewhere around:
>
> net/core/dev.c:
> int dev_set_threaded(struct net_device *dev, bool threaded)
> ...
> err = napi_kthread_create(napi);
> ...
> static int napi_kthread_create(struct napi_struct *n)
> ...
> n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
>
> drivers/net/ethernet/atheros/atl1c/atl1c_main.c:
> static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> ...
> dev_set_threaded(netdev, true);
>
> ${somewhere} (not sure where):
> ...
> strcpy(netdev->name, "eth%d");
>
> I was not able to pinpoint where expansion should ideally happen.
> Looks like many driver do `strcpy(netdev->name, "eth%d");` style
> initialization and almost none call `dev_set_threaded(netdev, true);`.
>
> Can you help me find it out how it should be fixed?
Hi Sergei
This is a fun one.
So, the driver does the usual alloc_etherdev_mq()
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/atheros/atl1c/atl1c_main.c#L2703
which ends up here:
https://elixir.bootlin.com/linux/latest/source/net/ethernet/eth.c#L391
struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
unsigned int rxqs)
{
return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
ether_setup, txqs, rxqs);
}
So at this point in time, the device has the name "eth%d".
The normal flow is that sometime later in probe, it calls
register_netdev().
https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L10454
if you follow that down, you get to: __dev_alloc_name(), which does
the expansion of the %d to an actual number:
https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L1087
So between alloc_etherdev_mq() and register_netdev(), the device name
is not valid. And as you pointed out, dev_set_threaded() tries to use
the name, and is called between these two.
The atl1c driver appears to be the only driver actually doing
this. There is a sysfs interface which can call dev_set_threaded(),
but the sysfs interface is probably not available until after
register_netdev() has given the interface its name.
There is a fix for atl1c. Any time after alloc_etherdev_mq(), the
driver can call dev_alloc_name().
So please give this a try. I've not even compile tested it...
iff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index da595242bc13..983a52f77bda 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -2706,6 +2706,10 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_alloc_etherdev;
}
+ err = dev_alloc_name(netdev, netdev->name);
+ if (err < 0)
+ goto err_init_netdev;
+
err = atl1c_init_netdev(netdev, pdev);
if (err) {
dev_err(&pdev->dev, "init netdevice failed\n");
If this works, i can turn it into a real patch submission.
Andrew
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-21 23:45 ` Andrew Lunn
@ 2022-01-22 1:03 ` Stephen Hemminger
2022-01-22 1:53 ` Andrew Lunn
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2022-01-22 1:03 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Sergei Trofimovich, netdev
On Sat, 22 Jan 2022 00:45:53 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Jan 21, 2022 at 09:57:47PM +0000, Sergei Trofimovich wrote:
> > Hia atl1c maintainers!
> >
> > This cosmetics bothered me for some time: atl1c driver
> > shows unexpanded % in kernel thread names. Looks like a
> > minor bug:
> >
> > $ ping -f 172.16.0.1 # host1
> > $ top # host2
> > ...
> > 621 root 20 0 0 0 0 S 11.0 0.0 0:05.01 napi/eth%d-385
> > 622 root 20 0 0 0 0 S 5.6 0.0 0:02.64 napi/eth%d-386
> > ...
> >
> > Was happening for a few years. Likely not a recent regression.
> >
> > System:
> > - linux-5.16.1
> > - x86_64
> > - 02:00.0 Ethernet controller: Qualcomm Atheros AR8151 v2.0 Gigabit Ethernet (rev c0)
> >
> > >From what I understand thread name comes from somewhere around:
> >
> > net/core/dev.c:
> > int dev_set_threaded(struct net_device *dev, bool threaded)
> > ...
> > err = napi_kthread_create(napi);
> > ...
> > static int napi_kthread_create(struct napi_struct *n)
> > ...
> > n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
> >
> > drivers/net/ethernet/atheros/atl1c/atl1c_main.c:
> > static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > ...
> > dev_set_threaded(netdev, true);
> >
> > ${somewhere} (not sure where):
> > ...
> > strcpy(netdev->name, "eth%d");
> >
> > I was not able to pinpoint where expansion should ideally happen.
> > Looks like many driver do `strcpy(netdev->name, "eth%d");` style
> > initialization and almost none call `dev_set_threaded(netdev, true);`.
> >
> > Can you help me find it out how it should be fixed?
>
> Hi Sergei
>
> This is a fun one.
>
> So, the driver does the usual alloc_etherdev_mq()
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/atheros/atl1c/atl1c_main.c#L2703
>
> which ends up here:
>
> https://elixir.bootlin.com/linux/latest/source/net/ethernet/eth.c#L391
>
> struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
> unsigned int rxqs)
> {
> return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
> ether_setup, txqs, rxqs);
> }
>
> So at this point in time, the device has the name "eth%d".
>
> The normal flow is that sometime later in probe, it calls
> register_netdev().
>
> https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L10454
>
> if you follow that down, you get to: __dev_alloc_name(), which does
> the expansion of the %d to an actual number:
>
> https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L1087
>
> So between alloc_etherdev_mq() and register_netdev(), the device name
> is not valid. And as you pointed out, dev_set_threaded() tries to use
> the name, and is called between these two.
>
> The atl1c driver appears to be the only driver actually doing
> this. There is a sysfs interface which can call dev_set_threaded(),
> but the sysfs interface is probably not available until after
> register_netdev() has given the interface its name.
>
> There is a fix for atl1c. Any time after alloc_etherdev_mq(), the
> driver can call dev_alloc_name().
>
> So please give this a try. I've not even compile tested it...
>
> iff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> index da595242bc13..983a52f77bda 100644
> --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> @@ -2706,6 +2706,10 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_alloc_etherdev;
> }
>
> + err = dev_alloc_name(netdev, netdev->name);
> + if (err < 0)
> + goto err_init_netdev;
> +
> err = atl1c_init_netdev(netdev, pdev);
> if (err) {
> dev_err(&pdev->dev, "init netdevice failed\n");
>
> If this works, i can turn it into a real patch submission.
>
> Andrew
This may not work right because probe is not called with RTNL.
And the alloc_name is using RTNL to prevent two devices from
getting the same name.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-22 1:03 ` Stephen Hemminger
@ 2022-01-22 1:53 ` Andrew Lunn
2022-01-22 12:12 ` Sergei Trofimovich
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2022-01-22 1:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Sergei Trofimovich, netdev
> > So please give this a try. I've not even compile tested it...
> >
> > iff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > index da595242bc13..983a52f77bda 100644
> > --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > @@ -2706,6 +2706,10 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > goto err_alloc_etherdev;
> > }
> >
> > + err = dev_alloc_name(netdev, netdev->name);
> > + if (err < 0)
> > + goto err_init_netdev;
> > +
> > err = atl1c_init_netdev(netdev, pdev);
> > if (err) {
> > dev_err(&pdev->dev, "init netdevice failed\n");
> >
> > If this works, i can turn it into a real patch submission.
> >
> > Andrew
>
>
> This may not work right because probe is not called with RTNL.
> And the alloc_name is using RTNL to prevent two devices from
> getting the same name.
Oh, yes. I looked at some of the users. And some do take rtnl before
calling it. And some don't!
Looking at register_netdev(), it seems we need something like:
if (rtnl_lock_killable()) {
err = -EINTR;
goto err_init_netdev;
}
err = dev_alloc_name(netdev, netdev->name);
rtnl_unlock();
if (err < 0)
goto err_init_netdev;
It might also be a good idea to put a ASSERT_RTNL() in
__dev_alloc_name() to catch any driver doing this wrong.
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-22 1:53 ` Andrew Lunn
@ 2022-01-22 12:12 ` Sergei Trofimovich
2022-01-22 15:54 ` Andrew Lunn
2022-01-22 19:40 ` Andrew Lunn
0 siblings, 2 replies; 8+ messages in thread
From: Sergei Trofimovich @ 2022-01-22 12:12 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Stephen Hemminger, netdev
On Sat, 22 Jan 2022 02:53:42 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> > > So please give this a try. I've not even compile tested it...
> > >
> > > iff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > > index da595242bc13..983a52f77bda 100644
> > > --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > > +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > > @@ -2706,6 +2706,10 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > goto err_alloc_etherdev;
> > > }
> > >
> > > + err = dev_alloc_name(netdev, netdev->name);
> > > + if (err < 0)
> > > + goto err_init_netdev;
> > > +
> > > err = atl1c_init_netdev(netdev, pdev);
> > > if (err) {
> > > dev_err(&pdev->dev, "init netdevice failed\n");
> > >
> > > If this works, i can turn it into a real patch submission.
> > >
> > > Andrew
> >
> >
> > This may not work right because probe is not called with RTNL.
> > And the alloc_name is using RTNL to prevent two devices from
> > getting the same name.
>
> Oh, yes. I looked at some of the users. And some do take rtnl before
> calling it. And some don't!
>
> Looking at register_netdev(), it seems we need something like:
>
> if (rtnl_lock_killable()) {
> err = -EINTR;
> goto err_init_netdev;
> }
> err = dev_alloc_name(netdev, netdev->name);
> rtnl_unlock();
> if (err < 0)
> goto err_init_netdev;
>
>
> It might also be a good idea to put a ASSERT_RTNL() in
> __dev_alloc_name() to catch any driver doing this wrong.
Thank you Andrew! I used this second version of your patch
against 5.16.1 and it seems to work:
$ sudo ping -f 172.16.0.1
613 root 20 0 0 0 0 S 11.0 0.0 0:07.46 napi/eth0-385
614 root 20 0 0 0 0 R 5.3 0.0 0:03.96 napi/eth0-386
Posting used diff as is just in case:
Tested-by: Sergei Trofimovich <slyich@gmail.com>
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -2706,6 +2706,15 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_alloc_etherdev;
}
+ if (rtnl_lock_killable()) {
+ err = -EINTR;
+ goto err_init_netdev;
+ }
+ err = dev_alloc_name(netdev, netdev->name);
+ rtnl_unlock();
+ if (err < 0)
+ goto err_init_netdev;
+
err = atl1c_init_netdev(netdev, pdev);
if (err) {
dev_err(&pdev->dev, "init netdevice failed\n");
--
Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-22 12:12 ` Sergei Trofimovich
@ 2022-01-22 15:54 ` Andrew Lunn
2022-01-22 19:40 ` Andrew Lunn
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2022-01-22 15:54 UTC (permalink / raw)
To: Sergei Trofimovich; +Cc: Stephen Hemminger, netdev
> Thank you Andrew! I used this second version of your patch
> against 5.16.1 and it seems to work:
>
> $ sudo ping -f 172.16.0.1
>
> 613 root 20 0 0 0 0 S 11.0 0.0 0:07.46 napi/eth0-385
> 614 root 20 0 0 0 0 R 5.3 0.0 0:03.96 napi/eth0-386
>
> Posting used diff as is just in case:
>
> Tested-by: Sergei Trofimovich <slyich@gmail.com>
Great, thanks for testing.
>
> --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> @@ -2706,6 +2706,15 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_alloc_etherdev;
> }
>
> + if (rtnl_lock_killable()) {
> + err = -EINTR;
> + goto err_init_netdev;
> + }
> + err = dev_alloc_name(netdev, netdev->name);
> + rtnl_unlock();
> + if (err < 0)
> + goto err_init_netdev;
Since there are multiple users of dev_alloc_name() and it appears some
get locking wrong, it makes sense to add a helper in the code which
does the locking. So i will work on a patchset to add such a helper
and convert other drivers.
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-22 12:12 ` Sergei Trofimovich
2022-01-22 15:54 ` Andrew Lunn
@ 2022-01-22 19:40 ` Andrew Lunn
2022-01-22 22:01 ` Sergei Trofimovich
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2022-01-22 19:40 UTC (permalink / raw)
To: Sergei Trofimovich; +Cc: Stephen Hemminger, netdev
> > Oh, yes. I looked at some of the users. And some do take rtnl before
> > calling it. And some don't!
> >
> > Looking at register_netdev(), it seems we need something like:
> >
> > if (rtnl_lock_killable()) {
> > err = -EINTR;
> > goto err_init_netdev;
> > }
> > err = dev_alloc_name(netdev, netdev->name);
> > rtnl_unlock();
> > if (err < 0)
> > goto err_init_netdev;
> >
> >
> > It might also be a good idea to put a ASSERT_RTNL() in
> > __dev_alloc_name() to catch any driver doing this wrong.
I looked at it some more, and some of the current users. And this does
not really work. There is a race condition.
Taking rtnl means you at least get a valid name, while you hold
rtnl. But it does not keep track of the name it just gave out. As a
result, you can release rtnl, and another device can jump in and be
given the same name in register_netdev(). When this driver then calls
register_netdev() the core will notice the clash and return -EEXISTS,
causing the probe to fail.
There are some drivers which take rtnl and keep it until after calling
register_netdevice(), rather than register_netdev(), but this is
rather ugly. And there are some drivers which don't take the lock, and
just hope they don't hit the race.
Maybe a better fix for this driver is:
From a5fc0e127bdc4b6ba4fb923012729cbf3d529996 Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 22 Jan 2022 13:33:58 -0600
Subject: [PATCH] net: ethernet: atl1c: Move dev_set_threaded() after
register_netdev()
dev_set_threaded() creates new kernel threads to perform napi. The
threads are given a name based on the interface name. However, the
interface is not allocated a name until register_netdev() is called.
By moving the call to dev_set_threaded() to later in the probe
function, odd thread names like napi/eth%d-385 are avoided.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index da595242bc13..9b8088905946 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -2728,7 +2728,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;
- dev_set_threaded(netdev, true);
+
for (i = 0; i < adapter->rx_queue_count; ++i)
netif_napi_add(netdev, &adapter->rrd_ring[i].napi,
atl1c_clean_rx, 64);
@@ -2781,6 +2781,8 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_register;
}
+ dev_set_threaded(netdev, true);
+
cards_found++;
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d
2022-01-22 19:40 ` Andrew Lunn
@ 2022-01-22 22:01 ` Sergei Trofimovich
0 siblings, 0 replies; 8+ messages in thread
From: Sergei Trofimovich @ 2022-01-22 22:01 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Stephen Hemminger, netdev
On Sat, 22 Jan 2022 20:40:51 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> > > Oh, yes. I looked at some of the users. And some do take rtnl before
> > > calling it. And some don't!
> > >
> > > Looking at register_netdev(), it seems we need something like:
> > >
> > > if (rtnl_lock_killable()) {
> > > err = -EINTR;
> > > goto err_init_netdev;
> > > }
> > > err = dev_alloc_name(netdev, netdev->name);
> > > rtnl_unlock();
> > > if (err < 0)
> > > goto err_init_netdev;
> > >
> > >
> > > It might also be a good idea to put a ASSERT_RTNL() in
> > > __dev_alloc_name() to catch any driver doing this wrong.
>
> I looked at it some more, and some of the current users. And this does
> not really work. There is a race condition.
>
> Taking rtnl means you at least get a valid name, while you hold
> rtnl. But it does not keep track of the name it just gave out. As a
> result, you can release rtnl, and another device can jump in and be
> given the same name in register_netdev(). When this driver then calls
> register_netdev() the core will notice the clash and return -EEXISTS,
> causing the probe to fail.
>
> There are some drivers which take rtnl and keep it until after calling
> register_netdevice(), rather than register_netdev(), but this is
> rather ugly. And there are some drivers which don't take the lock, and
> just hope they don't hit the race.
>
> Maybe a better fix for this driver is:
>
> From a5fc0e127bdc4b6ba4fb923012729cbf3d529996 Mon Sep 17 00:00:00 2001
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Sat, 22 Jan 2022 13:33:58 -0600
> Subject: [PATCH] net: ethernet: atl1c: Move dev_set_threaded() after
> register_netdev()
>
> dev_set_threaded() creates new kernel threads to perform napi. The
> threads are given a name based on the interface name. However, the
> interface is not allocated a name until register_netdev() is called.
> By moving the call to dev_set_threaded() to later in the probe
> function, odd thread names like napi/eth%d-385 are avoided.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
This patch also works:
687 root 20 0 0 0 0 R 6.7 0.0 0:00.15 napi/eth0-386
688 root 20 0 0 0 0 S 6.7 0.0 0:00.32 napi/eth0-385
tested in the same environment on top of 5.16.1:
Tested-by: Sergei Trofimovich <slyich@gmail.com>
Thank you!
> ---
> drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> index da595242bc13..9b8088905946 100644
> --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> @@ -2728,7 +2728,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;
> - dev_set_threaded(netdev, true);
> +
> for (i = 0; i < adapter->rx_queue_count; ++i)
> netif_napi_add(netdev, &adapter->rrd_ring[i].napi,
> atl1c_clean_rx, 64);
> @@ -2781,6 +2781,8 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_register;
> }
>
> + dev_set_threaded(netdev, true);
> +
> cards_found++;
> return 0;
>
> --
> 2.34.1
--
Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-22 22:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-21 21:57 atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d Sergei Trofimovich
2022-01-21 23:45 ` Andrew Lunn
2022-01-22 1:03 ` Stephen Hemminger
2022-01-22 1:53 ` Andrew Lunn
2022-01-22 12:12 ` Sergei Trofimovich
2022-01-22 15:54 ` Andrew Lunn
2022-01-22 19:40 ` Andrew Lunn
2022-01-22 22:01 ` Sergei Trofimovich
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).