From: "Alexis Lothoré" <alexis.lothore@bootlin.com>
To: linux-wireless@vger.kernel.org
Cc: "Ajay Singh" <ajay.kathat@microchip.com>,
"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
"Kalle Valo" <kvalo@kernel.org>,
"David Mosberger-Tang" <davidm@egauge.net>,
"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
linux-kernel@vger.kernel.org,
"Alexis Lothoré" <alexis.lothore@bootlin.com>
Subject: [PATCH 3/5] wilc: wifi: do not realloc workqueue everytime an interface is added
Date: Mon, 15 Jan 2024 15:56:32 +0100 [thread overview]
Message-ID: <20240115-wilc_1000_fixes-v1-3-54d29463a738@bootlin.com> (raw)
In-Reply-To: <20240115-wilc_1000_fixes-v1-0-54d29463a738@bootlin.com>
From: Ajay Singh <ajay.kathat@microchip.com>
Commit 09ed8bfc5215 ("wilc1000: Rename workqueue from "WILC_wq" to
"NETDEV-wq"") moved workqueue creation in wilc_netdev_ifc_init in order to
set the interface name in the workqueue name. However, while the driver
needs only one workqueue, the wilc_netdev_ifc_init is called each time we
add an interface over a phy, which in turns overwrite the workqueue with a
new one. This can be observed with the following commands:
for i in $(seq 0 10)
do
iw phy phy0 interface add wlan1 type managed
iw dev wlan1 del
done
ps -eo pid,comm|grep wlan
39 kworker/R-wlan0
98 kworker/R-wlan1
102 kworker/R-wlan1
105 kworker/R-wlan1
108 kworker/R-wlan1
111 kworker/R-wlan1
114 kworker/R-wlan1
117 kworker/R-wlan1
120 kworker/R-wlan1
123 kworker/R-wlan1
126 kworker/R-wlan1
129 kworker/R-wlan1
Fix this leakage by putting back hif_workqueue allocation in
wilc_cfg80211_init. Regarding the workqueue name, it is indeed relevant to
set it lowercase, however it is not attached to a specific netdev, so
enforcing netdev name in the name is not so relevant. Still, enrich the
name with the wiphy name to make it clear which phy is using the workqueue.
Fixes: 09ed8bfc5215 ("wilc1000: Rename workqueue from "WILC_wq" to "NETDEV-wq"")
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
Co-developed-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
This patch has initially been done by Ajay, and I slightly reworked it
09ed8bfc5215 ("wilc1000: Rename workqueue from "WILC_wq" to "NETDEV-wq"")
also mentions that this workqueue allocation move has also been done to
make wq alloc/dealloc symmetric: the revert set back the assymetry, and that
remains something to be fixed. Deallocation should likely be moved from
netdev.c to cfg80211, but that would be a dedicated rework topic
---
drivers/net/wireless/microchip/wilc1000/cfg80211.c | 11 ++++++++++-
drivers/net/wireless/microchip/wilc1000/netdev.c | 10 +---------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index ad2509d8c99a..2d0474e6404e 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -1804,15 +1804,24 @@ int wilc_cfg80211_init(struct wilc **wilc, struct device *dev, int io_type,
INIT_LIST_HEAD(&wl->rxq_head.list);
INIT_LIST_HEAD(&wl->vif_list);
+ wl->hif_workqueue = alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM,
+ wiphy_name(wl->wiphy));
+ if (!wl->hif_workqueue) {
+ ret = -ENOMEM;
+ goto free_cfg;
+ }
vif = wilc_netdev_ifc_init(wl, "wlan%d", WILC_STATION_MODE,
NL80211_IFTYPE_STATION, false);
if (IS_ERR(vif)) {
ret = PTR_ERR(vif);
- goto free_cfg;
+ goto free_hq;
}
return 0;
+free_hq:
+ destroy_workqueue(wl->hif_workqueue);
+
free_cfg:
wilc_wlan_cfg_deinit(wl);
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index f3b9709f8730..da52f4a9c1fe 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -989,13 +989,6 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
goto error;
}
- wl->hif_workqueue = alloc_ordered_workqueue("%s-wq", WQ_MEM_RECLAIM,
- ndev->name);
- if (!wl->hif_workqueue) {
- ret = -ENOMEM;
- goto unregister_netdev;
- }
-
ndev->needs_free_netdev = true;
vif->iftype = vif_type;
vif->idx = wilc_get_available_idx(wl);
@@ -1008,12 +1001,11 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
return vif;
-unregister_netdev:
+error:
if (rtnl_locked)
cfg80211_unregister_netdevice(ndev);
else
unregister_netdev(ndev);
- error:
free_netdev(ndev);
return ERR_PTR(ret);
}
--
2.42.1
next prev parent reply other threads:[~2024-01-15 15:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-15 14:56 [PATCH 0/5] wifi: wilc1000: minor fixes Alexis Lothoré
2024-01-15 14:56 ` [PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config() Alexis Lothoré
2024-01-18 9:31 ` Kalle Valo
2024-01-18 15:08 ` Alexis Lothoré
2024-01-18 16:52 ` Ajay.Kathat
2024-01-19 7:43 ` Alexis Lothoré
2024-01-19 18:19 ` Ajay.Kathat
2024-02-12 15:36 ` [1/5] " Kalle Valo
2024-01-15 14:56 ` [PATCH 2/5] wifi: wilc1000: fix driver_handler when committing initial configuration Alexis Lothoré
2024-01-18 9:35 ` Kalle Valo
2024-01-15 14:56 ` Alexis Lothoré [this message]
2024-01-15 14:56 ` [PATCH 4/5] wifi: wilc1000: fix incorrect power down sequence Alexis Lothoré
2024-01-15 14:56 ` [PATCH 5/5] wifi: wilc1000: fix multi-vif management when deleting a vif Alexis Lothoré
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=20240115-wilc_1000_fixes-v1-3-54d29463a738@bootlin.com \
--to=alexis.lothore@bootlin.com \
--cc=ajay.kathat@microchip.com \
--cc=claudiu.beznea@tuxon.dev \
--cc=davidm@egauge.net \
--cc=kvalo@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--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 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.