[BUG] staging: rtl8192u: Found a bug when removing the module

[BUG] staging: rtl8192u: Found a bug when removing the module

[Zheyu Ma]

Hello,

I found a bug in the rtl8192u module.

When removing the module, I got the following warning:

[   20.407360] remove_proc_entry: removing non-empty directory
'net/rtl819xU', leaking at least 'wlan0'
[   20.408609] WARNING: CPU: 4 PID: 451 at fs/proc/generic.c:718
remove_proc_entry+0x2e1/0x3e0
[   20.412316] RIP: 0010:remove_proc_entry+0x2e1/0x3e0
[   20.420050] Call Trace:
[   20.421178]  rtl8192_usb_module_exit+0x4a/0x63 [r8192u_usb]

I think the problem is the misuse of the proc entry, but I'm not
familiar with the driver, so I'm reporting the bug to you.

regards,

Zheyu Ma

Re: [BUG] staging: rtl8192u: Found a bug when removing the module

[Tong Zhang]

On Sat, Jul 16, 2022 at 5:16 AM Zheyu Ma <zheyuma97@gmail.com> wrote:
>
> Hello,
>
> I found a bug in the rtl8192u module.
>
> When removing the module, I got the following warning:
>
> [   20.407360] remove_proc_entry: removing non-empty directory
> 'net/rtl819xU', leaking at least 'wlan0'
> [   20.408609] WARNING: CPU: 4 PID: 451 at fs/proc/generic.c:718
> remove_proc_entry+0x2e1/0x3e0
> [   20.412316] RIP: 0010:remove_proc_entry+0x2e1/0x3e0
> [   20.420050] Call Trace:
> [   20.421178]  rtl8192_usb_module_exit+0x4a/0x63 [r8192u_usb]
>
> I think the problem is the misuse of the proc entry, but I'm not
> familiar with the driver, so I'm reporting the bug to you.
>
> regards,
>
> Zheyu Ma

Thanks for reporting. Looks like wlan0 is renamed to something else
later after proc fs files are created.
Could you test the following patch?
Thanks!
- Tong

[PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname]. Devname could
be wlan0 initially, however it could be renamed later to e.g. enx00e04c000002.
This will cause problem during debug file teardown since it uses
netdev->name which is no longer wlan0. To solve this problem, add a
notifier to handle device renaming.

Also, due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile         |   1 +
 drivers/staging/rtl8192u/r8192U.h         |   7 +
 drivers/staging/rtl8192u/r8192U_core.c    | 220 ++++------------------
 drivers/staging/rtl8192u/r8192U_debugfs.c | 185 ++++++++++++++++++
 4 files changed, 230 insertions(+), 183 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_debugfs.c

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 0be7426b6ebc..d32dfd89a606 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,6 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
+		  r8192U_debugfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 14ca00a2789b..34ed013aabf1 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
 	struct delayed_work gpio_change_rf_wq;
 	struct delayed_work initialgain_operate_wq;
 	struct workqueue_struct *priv_wq;
+
+	/* debugfs */
+	struct dentry *debugfs_dir;
 } r8192_priv;
 
 /* For rtl8187B */
@@ -1117,4 +1120,8 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
+void rtl8192_debugfs_init(struct net_device *dev);
+void rtl8192_debugfs_exit(struct net_device *dev);
+void rtl8192_debugfs_rename(struct net_device *dev);
+
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 2ca925f35830..2fba1cf59372 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -56,7 +56,6 @@ double __extendsfdf2(float a)
 #include "r8192U_dm.h"
 #include <linux/usb.h>
 #include <linux/slab.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 /* FIXME: check if 2.6.7 is ok */
 
@@ -452,179 +451,6 @@ static struct net_device_stats *rtl8192_stats(struct net_device *dev);
 static void rtl8192_restart(struct work_struct *work);
 static void watch_dog_timer_callback(struct timer_list *t);
 
-/****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
- ****************************************************************************/
-
-static struct proc_dir_entry *rtl8192_proc;
-
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-	struct ieee80211_device *ieee = priv->ieee80211;
-	struct ieee80211_network *target;
-
-	list_for_each_entry(target, &ieee->network_list, list) {
-		const char *wpa = "non_WPA";
-
-		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
-			wpa = "WPA";
-
-		seq_printf(m, "%s %s\n", target->ssid, wpa);
-	}
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	int i, n, max = 0xff;
-	u8 byte_rd;
-
-	seq_puts(m, "\n####################page 0##################\n ");
-
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x000 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 1##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x100 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 3##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x300 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_putc(m, '\n');
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "TX VI priority ok int: %lu\n"
-		   "TX VI priority error int: %lu\n"
-		   "TX VO priority ok int: %lu\n"
-		   "TX VO priority error int: %lu\n"
-		   "TX BE priority ok int: %lu\n"
-		   "TX BE priority error int: %lu\n"
-		   "TX BK priority ok int: %lu\n"
-		   "TX BK priority error int: %lu\n"
-		   "TX MANAGE priority ok int: %lu\n"
-		   "TX MANAGE priority error int: %lu\n"
-		   "TX BEACON priority ok int: %lu\n"
-		   "TX BEACON priority error int: %lu\n"
-		   "TX queue resume: %lu\n"
-		   "TX queue stopped?: %d\n"
-		   "TX fifo overflow: %lu\n"
-		   "TX VI queue: %d\n"
-		   "TX VO queue: %d\n"
-		   "TX BE queue: %d\n"
-		   "TX BK queue: %d\n"
-		   "TX VI dropped: %lu\n"
-		   "TX VO dropped: %lu\n"
-		   "TX BE dropped: %lu\n"
-		   "TX BK dropped: %lu\n"
-		   "TX total data packets %lu\n",
-		   priv->stats.txviokint,
-		   priv->stats.txvierr,
-		   priv->stats.txvookint,
-		   priv->stats.txvoerr,
-		   priv->stats.txbeokint,
-		   priv->stats.txbeerr,
-		   priv->stats.txbkokint,
-		   priv->stats.txbkerr,
-		   priv->stats.txmanageokint,
-		   priv->stats.txmanageerr,
-		   priv->stats.txbeaconokint,
-		   priv->stats.txbeaconerr,
-		   priv->stats.txresumed,
-		   netif_queue_stopped(dev),
-		   priv->stats.txoverflow,
-		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
-		   priv->stats.txvidrop,
-		   priv->stats.txvodrop,
-		   priv->stats.txbedrop,
-		   priv->stats.txbkdrop,
-		   priv->stats.txdatapkt
-		);
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "RX packets: %lu\n"
-		   "RX urb status error: %lu\n"
-		   "RX invalid urb error: %lu\n",
-		   priv->stats.rxoktotal,
-		   priv->stats.rxstaterr,
-		   priv->stats.rxurberr);
-
-	return 0;
-}
-
-static void rtl8192_proc_module_init(void)
-{
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
-}
-
-static void rtl8192_proc_init_one(struct net_device *dev)
-{
-	struct proc_dir_entry *dir;
-
-	if (!rtl8192_proc)
-		return;
-
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
-
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
-}
-
-static void rtl8192_proc_remove_one(struct net_device *dev)
-{
-	remove_proc_subtree(dev->name, rtl8192_proc);
-}
-
 /****************************************************************************
  *  -----------------------------MISC STUFF-------------------------
  *****************************************************************************/
@@ -4730,7 +4556,7 @@ static int rtl8192_usb_probe(struct usb_interface *intf,
 		goto fail2;
 
 	RT_TRACE(COMP_INIT, "dev name=======> %s\n", dev->name);
-	rtl8192_proc_init_one(dev);
+	rtl8192_debugfs_init(dev);
 
 	RT_TRACE(COMP_INIT, "Driver probe completed\n");
 	return 0;
@@ -4764,11 +4590,9 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	struct net_device *dev = usb_get_intfdata(intf);
 	struct r8192_priv *priv = ieee80211_priv(dev);
 
-	unregister_netdev(dev);
-
 	RT_TRACE(COMP_DOWN, "=============>wlan driver to be removed\n");
-	rtl8192_proc_remove_one(dev);
-
+	rtl8192_debugfs_exit(dev);
+	unregister_netdev(dev);
 	rtl8192_down(dev);
 	kfree(priv->pFirmware);
 	priv->pFirmware = NULL;
@@ -4779,6 +4603,32 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	RT_TRACE(COMP_DOWN, "wlan driver removed\n");
 }
 
+static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
+		void *data)
+{
+	struct net_device *netdev = netdev_notifier_info_to_dev(data);
+
+	if (netdev->netdev_ops != &rtl8192_netdev_ops)
+		goto out;
+
+	switch (event) {
+		case NETDEV_CHANGENAME:
+			rtl8192_debugfs_rename(netdev);
+			break;
+
+		default:
+			break;
+	}
+
+out:
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block rtl8192_usb_netdev_notifier = {
+	.notifier_call = rtl8192_usb_netdev_event,
+};
+
+
 static int __init rtl8192_usb_module_init(void)
 {
 	int ret;
@@ -4788,10 +4638,14 @@ static int __init rtl8192_usb_module_init(void)
 	RT_TRACE(COMP_INIT, "Initializing module");
 	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
 
+	ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
+	if (ret)
+		return ret;
+
 	ret = ieee80211_debug_init();
 	if (ret) {
 		pr_err("ieee80211_debug_init() failed %d\n", ret);
-		return ret;
+		goto debug_init;
 	}
 
 	ret = ieee80211_crypto_init();
@@ -4818,14 +4672,12 @@ static int __init rtl8192_usb_module_init(void)
 		goto crypto_ccmp_exit;
 	}
 
-	rtl8192_proc_module_init();
 	ret = usb_register(&rtl8192_usb_driver);
 	if (ret)
 		goto rtl8192_proc_module_exit;
 	return ret;
 
 rtl8192_proc_module_exit:
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
 	ieee80211_crypto_wep_exit();
 crypto_ccmp_exit:
 	ieee80211_crypto_ccmp_exit();
@@ -4835,18 +4687,20 @@ static int __init rtl8192_usb_module_init(void)
 	ieee80211_crypto_deinit();
 debug_exit:
 	ieee80211_debug_exit();
+debug_init:
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	return ret;
 }
 
 static void __exit rtl8192_usb_module_exit(void)
 {
 	usb_deregister(&rtl8192_usb_driver);
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
 	ieee80211_crypto_wep_exit();
 	ieee80211_crypto_ccmp_exit();
 	ieee80211_crypto_tkip_exit();
 	ieee80211_crypto_deinit();
 	ieee80211_debug_exit();
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	RT_TRACE(COMP_DOWN, "Exiting");
 }
 
diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
new file mode 100644
index 000000000000..96cdc245bd03
--- /dev/null
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -0,0 +1,185 @@
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/module.h>
+#include "r8192U.h"
+
+static int rtl8192_usb_stats_rx_show(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+			"RX packets: %lu\n"
+			"RX urb status error: %lu\n"
+			"RX invalid urb error: %lu\n",
+			priv->stats.rxoktotal,
+			priv->stats.rxstaterr,
+			priv->stats.rxurberr);
+
+	return 0;
+}
+
+static int rtl8192_usb_stats_tx_show(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+			"TX VI priority ok int: %lu\n"
+			"TX VI priority error int: %lu\n"
+			"TX VO priority ok int: %lu\n"
+			"TX VO priority error int: %lu\n"
+			"TX BE priority ok int: %lu\n"
+			"TX BE priority error int: %lu\n"
+			"TX BK priority ok int: %lu\n"
+			"TX BK priority error int: %lu\n"
+			"TX MANAGE priority ok int: %lu\n"
+			"TX MANAGE priority error int: %lu\n"
+			"TX BEACON priority ok int: %lu\n"
+			"TX BEACON priority error int: %lu\n"
+			"TX queue resume: %lu\n"
+			"TX queue stopped?: %d\n"
+			"TX fifo overflow: %lu\n"
+			"TX VI queue: %d\n"
+			"TX VO queue: %d\n"
+			"TX BE queue: %d\n"
+			"TX BK queue: %d\n"
+			"TX VI dropped: %lu\n"
+			"TX VO dropped: %lu\n"
+			"TX BE dropped: %lu\n"
+			"TX BK dropped: %lu\n"
+			"TX total data packets %lu\n",
+		priv->stats.txviokint,
+		priv->stats.txvierr,
+		priv->stats.txvookint,
+		priv->stats.txvoerr,
+		priv->stats.txbeokint,
+		priv->stats.txbeerr,
+		priv->stats.txbkokint,
+		priv->stats.txbkerr,
+		priv->stats.txmanageokint,
+		priv->stats.txmanageerr,
+		priv->stats.txbeaconokint,
+		priv->stats.txbeaconerr,
+		priv->stats.txresumed,
+		netif_queue_stopped(dev),
+		priv->stats.txoverflow,
+		atomic_read(&(priv->tx_pending[VI_PRIORITY])),
+		atomic_read(&(priv->tx_pending[VO_PRIORITY])),
+		atomic_read(&(priv->tx_pending[BE_PRIORITY])),
+		atomic_read(&(priv->tx_pending[BK_PRIORITY])),
+		priv->stats.txvidrop,
+		priv->stats.txvodrop,
+		priv->stats.txbedrop,
+		priv->stats.txbkdrop,
+		priv->stats.txdatapkt
+			);
+
+	return 0;
+}
+
+static int rtl8192_usb_stats_ap_show(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct ieee80211_device *ieee = priv->ieee80211;
+	struct ieee80211_network *target;
+
+	list_for_each_entry(target, &ieee->network_list, list) {
+		const char *wpa = "non_WPA";
+
+		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
+			wpa = "WPA";
+
+		seq_printf(m, "%s %s\n", target->ssid, wpa);
+	}
+
+	return 0;
+}
+
+static int rtl8192_usb_registers_show(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	int i, n, max = 0xff;
+	u8 byte_rd;
+
+	seq_puts(m, "\n####################page 0##################\n ");
+
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x000 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 1##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x100 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 3##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x300 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_putc(m, '\n');
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_rx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_tx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_ap);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_registers);
+
+void rtl8192_debugfs_init(struct net_device *dev)
+{
+	struct dentry *dir;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	dir = debugfs_create_dir(dev->name, NULL);
+	if (IS_ERR_OR_NULL(dir))
+		return;
+
+	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
+	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
+	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
+	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
+
+	priv->debugfs_dir = dir;
+}
+
+void rtl8192_debugfs_exit(struct net_device *dev)
+{
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	if (!priv->debugfs_dir)
+		return;
+	debugfs_remove_recursive(priv->debugfs_dir);
+	priv->debugfs_dir = NULL;
+}
+
+void rtl8192_debugfs_rename(struct net_device *dev)
+{
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	if (!priv->debugfs_dir)
+		return;
+
+	if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
+		return;
+
+	debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
+			priv->debugfs_dir->d_parent, dev->name);
+}
+
-- 
2.25.1

Re: [PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Zheyu Ma]

On Sun, Jul 17, 2022 at 3:02 PM Tong Zhang <ztong0001@gmail.com> wrote:
>
> There are 4 debug files created under /proc/net/[Devname]. Devname could
> be wlan0 initially, however it could be renamed later to e.g. enx00e04c000002.
> This will cause problem during debug file teardown since it uses
> netdev->name which is no longer wlan0. To solve this problem, add a
> notifier to handle device renaming.
>
> Also, due to this is purely for debuging as files are created read only,
> move this to debugfs like other NIC drivers do instead of using procfs.
>
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> ---
>  drivers/staging/rtl8192u/Makefile         |   1 +
>  drivers/staging/rtl8192u/r8192U.h         |   7 +
>  drivers/staging/rtl8192u/r8192U_core.c    | 220 ++++------------------
>  drivers/staging/rtl8192u/r8192U_debugfs.c | 185 ++++++++++++++++++
>  4 files changed, 230 insertions(+), 183 deletions(-)
>  create mode 100644 drivers/staging/rtl8192u/r8192U_debugfs.c
>
> diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
> index 0be7426b6ebc..d32dfd89a606 100644
> --- a/drivers/staging/rtl8192u/Makefile
> +++ b/drivers/staging/rtl8192u/Makefile
> @@ -8,6 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
>  r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o                \
>                   r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o        \
>                   r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o     \
> +                 r8192U_debugfs.o                                      \
>                   ieee80211/ieee80211_crypt.o                           \
>                   ieee80211/ieee80211_crypt_tkip.o                      \
>                   ieee80211/ieee80211_crypt_ccmp.o                      \
> diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
> index 14ca00a2789b..34ed013aabf1 100644
> --- a/drivers/staging/rtl8192u/r8192U.h
> +++ b/drivers/staging/rtl8192u/r8192U.h
> @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
>         struct delayed_work gpio_change_rf_wq;
>         struct delayed_work initialgain_operate_wq;
>         struct workqueue_struct *priv_wq;
> +
> +       /* debugfs */
> +       struct dentry *debugfs_dir;
>  } r8192_priv;
>
>  /* For rtl8187B */
> @@ -1117,4 +1120,8 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
>  void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
>             const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
>
> +void rtl8192_debugfs_init(struct net_device *dev);
> +void rtl8192_debugfs_exit(struct net_device *dev);
> +void rtl8192_debugfs_rename(struct net_device *dev);
> +
>  #endif
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 2ca925f35830..2fba1cf59372 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -56,7 +56,6 @@ double __extendsfdf2(float a)
>  #include "r8192U_dm.h"
>  #include <linux/usb.h>
>  #include <linux/slab.h>
> -#include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
>  /* FIXME: check if 2.6.7 is ok */
>
> @@ -452,179 +451,6 @@ static struct net_device_stats *rtl8192_stats(struct net_device *dev);
>  static void rtl8192_restart(struct work_struct *work);
>  static void watch_dog_timer_callback(struct timer_list *t);
>
> -/****************************************************************************
> - *   -----------------------------PROCFS STUFF-------------------------
> - ****************************************************************************/
> -
> -static struct proc_dir_entry *rtl8192_proc;
> -
> -static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
> -{
> -       struct net_device *dev = m->private;
> -       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> -       struct ieee80211_device *ieee = priv->ieee80211;
> -       struct ieee80211_network *target;
> -
> -       list_for_each_entry(target, &ieee->network_list, list) {
> -               const char *wpa = "non_WPA";
> -
> -               if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
> -                       wpa = "WPA";
> -
> -               seq_printf(m, "%s %s\n", target->ssid, wpa);
> -       }
> -
> -       return 0;
> -}
> -
> -static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
> -{
> -       struct net_device *dev = m->private;
> -       int i, n, max = 0xff;
> -       u8 byte_rd;
> -
> -       seq_puts(m, "\n####################page 0##################\n ");
> -
> -       for (n = 0; n <= max;) {
> -               seq_printf(m, "\nD:  %2x > ", n);
> -
> -               for (i = 0; i < 16 && n <= max; i++, n++) {
> -                       read_nic_byte(dev, 0x000 | n, &byte_rd);
> -                       seq_printf(m, "%2x ", byte_rd);
> -               }
> -       }
> -
> -       seq_puts(m, "\n####################page 1##################\n ");
> -       for (n = 0; n <= max;) {
> -               seq_printf(m, "\nD:  %2x > ", n);
> -
> -               for (i = 0; i < 16 && n <= max; i++, n++) {
> -                       read_nic_byte(dev, 0x100 | n, &byte_rd);
> -                       seq_printf(m, "%2x ", byte_rd);
> -               }
> -       }
> -
> -       seq_puts(m, "\n####################page 3##################\n ");
> -       for (n = 0; n <= max;) {
> -               seq_printf(m, "\nD:  %2x > ", n);
> -
> -               for (i = 0; i < 16 && n <= max; i++, n++) {
> -                       read_nic_byte(dev, 0x300 | n, &byte_rd);
> -                       seq_printf(m, "%2x ", byte_rd);
> -               }
> -       }
> -
> -       seq_putc(m, '\n');
> -       return 0;
> -}
> -
> -static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
> -{
> -       struct net_device *dev = m->private;
> -       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> -
> -       seq_printf(m,
> -                  "TX VI priority ok int: %lu\n"
> -                  "TX VI priority error int: %lu\n"
> -                  "TX VO priority ok int: %lu\n"
> -                  "TX VO priority error int: %lu\n"
> -                  "TX BE priority ok int: %lu\n"
> -                  "TX BE priority error int: %lu\n"
> -                  "TX BK priority ok int: %lu\n"
> -                  "TX BK priority error int: %lu\n"
> -                  "TX MANAGE priority ok int: %lu\n"
> -                  "TX MANAGE priority error int: %lu\n"
> -                  "TX BEACON priority ok int: %lu\n"
> -                  "TX BEACON priority error int: %lu\n"
> -                  "TX queue resume: %lu\n"
> -                  "TX queue stopped?: %d\n"
> -                  "TX fifo overflow: %lu\n"
> -                  "TX VI queue: %d\n"
> -                  "TX VO queue: %d\n"
> -                  "TX BE queue: %d\n"
> -                  "TX BK queue: %d\n"
> -                  "TX VI dropped: %lu\n"
> -                  "TX VO dropped: %lu\n"
> -                  "TX BE dropped: %lu\n"
> -                  "TX BK dropped: %lu\n"
> -                  "TX total data packets %lu\n",
> -                  priv->stats.txviokint,
> -                  priv->stats.txvierr,
> -                  priv->stats.txvookint,
> -                  priv->stats.txvoerr,
> -                  priv->stats.txbeokint,
> -                  priv->stats.txbeerr,
> -                  priv->stats.txbkokint,
> -                  priv->stats.txbkerr,
> -                  priv->stats.txmanageokint,
> -                  priv->stats.txmanageerr,
> -                  priv->stats.txbeaconokint,
> -                  priv->stats.txbeaconerr,
> -                  priv->stats.txresumed,
> -                  netif_queue_stopped(dev),
> -                  priv->stats.txoverflow,
> -                  atomic_read(&(priv->tx_pending[VI_PRIORITY])),
> -                  atomic_read(&(priv->tx_pending[VO_PRIORITY])),
> -                  atomic_read(&(priv->tx_pending[BE_PRIORITY])),
> -                  atomic_read(&(priv->tx_pending[BK_PRIORITY])),
> -                  priv->stats.txvidrop,
> -                  priv->stats.txvodrop,
> -                  priv->stats.txbedrop,
> -                  priv->stats.txbkdrop,
> -                  priv->stats.txdatapkt
> -               );
> -
> -       return 0;
> -}
> -
> -static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
> -{
> -       struct net_device *dev = m->private;
> -       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> -
> -       seq_printf(m,
> -                  "RX packets: %lu\n"
> -                  "RX urb status error: %lu\n"
> -                  "RX invalid urb error: %lu\n",
> -                  priv->stats.rxoktotal,
> -                  priv->stats.rxstaterr,
> -                  priv->stats.rxurberr);
> -
> -       return 0;
> -}
> -
> -static void rtl8192_proc_module_init(void)
> -{
> -       RT_TRACE(COMP_INIT, "Initializing proc filesystem");
> -       rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
> -}
> -
> -static void rtl8192_proc_init_one(struct net_device *dev)
> -{
> -       struct proc_dir_entry *dir;
> -
> -       if (!rtl8192_proc)
> -               return;
> -
> -       dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
> -       if (!dir)
> -               return;
> -
> -       proc_create_single("stats-rx", S_IFREG | 0444, dir,
> -                          proc_get_stats_rx);
> -       proc_create_single("stats-tx", S_IFREG | 0444, dir,
> -                          proc_get_stats_tx);
> -       proc_create_single("stats-ap", S_IFREG | 0444, dir,
> -                          proc_get_stats_ap);
> -       proc_create_single("registers", S_IFREG | 0444, dir,
> -                          proc_get_registers);
> -}
> -
> -static void rtl8192_proc_remove_one(struct net_device *dev)
> -{
> -       remove_proc_subtree(dev->name, rtl8192_proc);
> -}
> -
>  /****************************************************************************
>   *  -----------------------------MISC STUFF-------------------------
>   *****************************************************************************/
> @@ -4730,7 +4556,7 @@ static int rtl8192_usb_probe(struct usb_interface *intf,
>                 goto fail2;
>
>         RT_TRACE(COMP_INIT, "dev name=======> %s\n", dev->name);
> -       rtl8192_proc_init_one(dev);
> +       rtl8192_debugfs_init(dev);
>
>         RT_TRACE(COMP_INIT, "Driver probe completed\n");
>         return 0;
> @@ -4764,11 +4590,9 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
>         struct net_device *dev = usb_get_intfdata(intf);
>         struct r8192_priv *priv = ieee80211_priv(dev);
>
> -       unregister_netdev(dev);
> -
>         RT_TRACE(COMP_DOWN, "=============>wlan driver to be removed\n");
> -       rtl8192_proc_remove_one(dev);
> -
> +       rtl8192_debugfs_exit(dev);
> +       unregister_netdev(dev);
>         rtl8192_down(dev);
>         kfree(priv->pFirmware);
>         priv->pFirmware = NULL;
> @@ -4779,6 +4603,32 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
>         RT_TRACE(COMP_DOWN, "wlan driver removed\n");
>  }
>
> +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> +               void *data)
> +{
> +       struct net_device *netdev = netdev_notifier_info_to_dev(data);
> +
> +       if (netdev->netdev_ops != &rtl8192_netdev_ops)
> +               goto out;
> +
> +       switch (event) {
> +               case NETDEV_CHANGENAME:
> +                       rtl8192_debugfs_rename(netdev);
> +                       break;
> +
> +               default:
> +                       break;
> +       }
> +
> +out:
> +       return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rtl8192_usb_netdev_notifier = {
> +       .notifier_call = rtl8192_usb_netdev_event,
> +};
> +
> +
>  static int __init rtl8192_usb_module_init(void)
>  {
>         int ret;
> @@ -4788,10 +4638,14 @@ static int __init rtl8192_usb_module_init(void)
>         RT_TRACE(COMP_INIT, "Initializing module");
>         RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
>
> +       ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> +       if (ret)
> +               return ret;
> +
>         ret = ieee80211_debug_init();
>         if (ret) {
>                 pr_err("ieee80211_debug_init() failed %d\n", ret);
> -               return ret;
> +               goto debug_init;
>         }
>
>         ret = ieee80211_crypto_init();
> @@ -4818,14 +4672,12 @@ static int __init rtl8192_usb_module_init(void)
>                 goto crypto_ccmp_exit;
>         }
>
> -       rtl8192_proc_module_init();
>         ret = usb_register(&rtl8192_usb_driver);
>         if (ret)
>                 goto rtl8192_proc_module_exit;
>         return ret;
>
>  rtl8192_proc_module_exit:
> -       remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
>         ieee80211_crypto_wep_exit();
>  crypto_ccmp_exit:
>         ieee80211_crypto_ccmp_exit();
> @@ -4835,18 +4687,20 @@ static int __init rtl8192_usb_module_init(void)
>         ieee80211_crypto_deinit();
>  debug_exit:
>         ieee80211_debug_exit();
> +debug_init:
> +       unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
>         return ret;
>  }
>
>  static void __exit rtl8192_usb_module_exit(void)
>  {
>         usb_deregister(&rtl8192_usb_driver);
> -       remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
>         ieee80211_crypto_wep_exit();
>         ieee80211_crypto_ccmp_exit();
>         ieee80211_crypto_tkip_exit();
>         ieee80211_crypto_deinit();
>         ieee80211_debug_exit();
> +       unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
>         RT_TRACE(COMP_DOWN, "Exiting");
>  }
>
> diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
> new file mode 100644
> index 000000000000..96cdc245bd03
> --- /dev/null
> +++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
> @@ -0,0 +1,185 @@
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +#include <linux/module.h>
> +#include "r8192U.h"
> +
> +static int rtl8192_usb_stats_rx_show(struct seq_file *m, void *v)
> +{
> +       struct net_device *dev = m->private;
> +       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +       seq_printf(m,
> +                       "RX packets: %lu\n"
> +                       "RX urb status error: %lu\n"
> +                       "RX invalid urb error: %lu\n",
> +                       priv->stats.rxoktotal,
> +                       priv->stats.rxstaterr,
> +                       priv->stats.rxurberr);
> +
> +       return 0;
> +}
> +
> +static int rtl8192_usb_stats_tx_show(struct seq_file *m, void *v)
> +{
> +       struct net_device *dev = m->private;
> +       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +       seq_printf(m,
> +                       "TX VI priority ok int: %lu\n"
> +                       "TX VI priority error int: %lu\n"
> +                       "TX VO priority ok int: %lu\n"
> +                       "TX VO priority error int: %lu\n"
> +                       "TX BE priority ok int: %lu\n"
> +                       "TX BE priority error int: %lu\n"
> +                       "TX BK priority ok int: %lu\n"
> +                       "TX BK priority error int: %lu\n"
> +                       "TX MANAGE priority ok int: %lu\n"
> +                       "TX MANAGE priority error int: %lu\n"
> +                       "TX BEACON priority ok int: %lu\n"
> +                       "TX BEACON priority error int: %lu\n"
> +                       "TX queue resume: %lu\n"
> +                       "TX queue stopped?: %d\n"
> +                       "TX fifo overflow: %lu\n"
> +                       "TX VI queue: %d\n"
> +                       "TX VO queue: %d\n"
> +                       "TX BE queue: %d\n"
> +                       "TX BK queue: %d\n"
> +                       "TX VI dropped: %lu\n"
> +                       "TX VO dropped: %lu\n"
> +                       "TX BE dropped: %lu\n"
> +                       "TX BK dropped: %lu\n"
> +                       "TX total data packets %lu\n",
> +               priv->stats.txviokint,
> +               priv->stats.txvierr,
> +               priv->stats.txvookint,
> +               priv->stats.txvoerr,
> +               priv->stats.txbeokint,
> +               priv->stats.txbeerr,
> +               priv->stats.txbkokint,
> +               priv->stats.txbkerr,
> +               priv->stats.txmanageokint,
> +               priv->stats.txmanageerr,
> +               priv->stats.txbeaconokint,
> +               priv->stats.txbeaconerr,
> +               priv->stats.txresumed,
> +               netif_queue_stopped(dev),
> +               priv->stats.txoverflow,
> +               atomic_read(&(priv->tx_pending[VI_PRIORITY])),
> +               atomic_read(&(priv->tx_pending[VO_PRIORITY])),
> +               atomic_read(&(priv->tx_pending[BE_PRIORITY])),
> +               atomic_read(&(priv->tx_pending[BK_PRIORITY])),
> +               priv->stats.txvidrop,
> +               priv->stats.txvodrop,
> +               priv->stats.txbedrop,
> +               priv->stats.txbkdrop,
> +               priv->stats.txdatapkt
> +                       );
> +
> +       return 0;
> +}
> +
> +static int rtl8192_usb_stats_ap_show(struct seq_file *m, void *v)
> +{
> +       struct net_device *dev = m->private;
> +       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +       struct ieee80211_device *ieee = priv->ieee80211;
> +       struct ieee80211_network *target;
> +
> +       list_for_each_entry(target, &ieee->network_list, list) {
> +               const char *wpa = "non_WPA";
> +
> +               if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
> +                       wpa = "WPA";
> +
> +               seq_printf(m, "%s %s\n", target->ssid, wpa);
> +       }
> +
> +       return 0;
> +}
> +
> +static int rtl8192_usb_registers_show(struct seq_file *m, void *v)
> +{
> +       struct net_device *dev = m->private;
> +       int i, n, max = 0xff;
> +       u8 byte_rd;
> +
> +       seq_puts(m, "\n####################page 0##################\n ");
> +
> +       for (n = 0; n <= max;) {
> +               seq_printf(m, "\nD:  %2x > ", n);
> +
> +               for (i = 0; i < 16 && n <= max; i++, n++) {
> +                       read_nic_byte(dev, 0x000 | n, &byte_rd);
> +                       seq_printf(m, "%2x ", byte_rd);
> +               }
> +       }
> +
> +       seq_puts(m, "\n####################page 1##################\n ");
> +       for (n = 0; n <= max;) {
> +               seq_printf(m, "\nD:  %2x > ", n);
> +
> +               for (i = 0; i < 16 && n <= max; i++, n++) {
> +                       read_nic_byte(dev, 0x100 | n, &byte_rd);
> +                       seq_printf(m, "%2x ", byte_rd);
> +               }
> +       }
> +
> +       seq_puts(m, "\n####################page 3##################\n ");
> +       for (n = 0; n <= max;) {
> +               seq_printf(m, "\nD:  %2x > ", n);
> +
> +               for (i = 0; i < 16 && n <= max; i++, n++) {
> +                       read_nic_byte(dev, 0x300 | n, &byte_rd);
> +                       seq_printf(m, "%2x ", byte_rd);
> +               }
> +       }
> +
> +       seq_putc(m, '\n');
> +       return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_rx);
> +DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_tx);
> +DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_ap);
> +DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_registers);
> +
> +void rtl8192_debugfs_init(struct net_device *dev)
> +{
> +       struct dentry *dir;
> +       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +       dir = debugfs_create_dir(dev->name, NULL);
> +       if (IS_ERR_OR_NULL(dir))
> +               return;
> +
> +       debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> +       debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> +       debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> +       debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> +
> +       priv->debugfs_dir = dir;
> +}
> +
> +void rtl8192_debugfs_exit(struct net_device *dev)
> +{
> +       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +       if (!priv->debugfs_dir)
> +               return;
> +       debugfs_remove_recursive(priv->debugfs_dir);
> +       priv->debugfs_dir = NULL;
> +}
> +
> +void rtl8192_debugfs_rename(struct net_device *dev)
> +{
> +       struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +       if (!priv->debugfs_dir)
> +               return;
> +
> +       if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
> +               return;
> +
> +       debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> +                       priv->debugfs_dir->d_parent, dev->name);
> +}
> +
> --
> 2.25.1

Thanks for your patch, it works for me :)

Tested-by: Zheyu Ma <zheyuma97@gmail.com>

regards,

Zheyu Ma

Re: [PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Dan Carpenter]

On Sun, Jul 17, 2022 at 12:01:57AM -0700, Tong Zhang wrote:
> There are 4 debug files created under /proc/net/[Devname]. Devname could
> be wlan0 initially, however it could be renamed later to e.g. enx00e04c000002.
> This will cause problem during debug file teardown since it uses
> netdev->name which is no longer wlan0. To solve this problem, add a
> notifier to handle device renaming.
> 
> Also, due to this is purely for debuging as files are created read only,
> move this to debugfs like other NIC drivers do instead of using procfs.
> 

You're doing too many things in one patch.  Maybe do it like this:
patch 1: Move the functions (no functional change).
patch 2: Change to debugfs instead of procfs
patch 3: Add the notifier

regards,
dan carpenter

Re: [PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Dan Carpenter]

On Sun, Jul 17, 2022 at 12:01:57AM -0700, Tong Zhang wrote:
> -static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
> -{
> -	struct net_device *dev = m->private;
> -	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> -
> -	seq_printf(m,
> -		   "RX packets: %lu\n"
> -		   "RX urb status error: %lu\n"
> -		   "RX invalid urb error: %lu\n",
> -		   priv->stats.rxoktotal,
> -		   priv->stats.rxstaterr,
> -		   priv->stats.rxurberr);
> -
> -	return 0;
> -}

> +static int rtl8192_usb_stats_rx_show(struct seq_file *m, void *v)
> +{
> +	struct net_device *dev = m->private;
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +	seq_printf(m,
> +			"RX packets: %lu\n"
> +			"RX urb status error: %lu\n"
> +			"RX invalid urb error: %lu\n",
> +			priv->stats.rxoktotal,
> +			priv->stats.rxstaterr,
> +			priv->stats.rxurberr);
> +
> +	return 0;
> +}

When you're moving function around, then try to avoid changing the white
space at all.  It just makes it more complicated to review.  But
especially in this case, the white space was correct-ish in the original
and now it is bad.

regards,
dan carpenter

Re: [PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Dan Carpenter]

On Sun, Jul 17, 2022 at 12:01:57AM -0700, Tong Zhang wrote:
> +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> +		void *data)
> +{
> +	struct net_device *netdev = netdev_notifier_info_to_dev(data);
> +
> +	if (netdev->netdev_ops != &rtl8192_netdev_ops)
> +		goto out;
> +
> +	switch (event) {
> +		case NETDEV_CHANGENAME:
> +			rtl8192_debugfs_rename(netdev);
> +			break;
> +
> +		default:
> +			break;
> +	}

This isn't indented properly.  Don't forget to run ./scripts/checkpatch.pl
on your patch.

> +
> +out:
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rtl8192_usb_netdev_notifier = {
> +	.notifier_call = rtl8192_usb_netdev_event,
> +};
> +
> +

Here too.

>  static int __init rtl8192_usb_module_init(void)
>  {
>  	int ret;
> @@ -4788,10 +4638,14 @@ static int __init rtl8192_usb_module_init(void)

[ snip ]

> +void rtl8192_debugfs_init(struct net_device *dev)
> +{
> +	struct dentry *dir;
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +	dir = debugfs_create_dir(dev->name, NULL);
> +	if (IS_ERR_OR_NULL(dir))
> +		return;

In olden times the debugfs_create_dir() function used to return a mix
of error pointers and NULL.  But the idea with that function is that
most people are not supposed to check for errors.  But instead of that
they added all kind of *buggy* checks.  So then we made it just return
error pointers.

This code *does* care about it because it uses the
priv->debugfs_dir->d_name.name in rtl8192_debugfs_rename().

But caring about the debugfs dir and creating a rtl8192_debugfs_rename()
function is really unusual.  And normally when we have to do something
unusual that means we are doing something wrong...  :/

Anyway, just check for if (IS_ERR(dir)) {

> +
> +	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> +	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> +	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> +	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> +
> +	priv->debugfs_dir = dir;
> +}
> +
> +void rtl8192_debugfs_exit(struct net_device *dev)
> +{
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +	if (!priv->debugfs_dir)
> +		return;
> +	debugfs_remove_recursive(priv->debugfs_dir);
> +	priv->debugfs_dir = NULL;
> +}
> +
> +void rtl8192_debugfs_rename(struct net_device *dev)
> +{
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +
> +	if (!priv->debugfs_dir)
> +		return;
> +
> +	if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
> +		return;
> +
> +	debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> +			priv->debugfs_dir->d_parent, dev->name);
> +}
> +

regards,
dan carpenter

[PATCH v2 0/3] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname]. Devname could
be wlan0 initially, however it could be renamed later to e.g. enx00e04c000002.
This will cause problem during debug file teardown since it uses
netdev->name which is no longer wlan0. To solve this problem, add a
notifier to handle device renaming.

Also, due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Tested-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>

v2: break down patch and fix pointer check

Tong Zhang (3):
  staging: rtl8192u: move debug stuff to its own file
  staging: rtl8192u: move debug files to debugfs
  staging: rtl8192u: fix rmmod warn when wlan0 is renamed

 drivers/staging/rtl8192u/Makefile         |   1 +
 drivers/staging/rtl8192u/r8192U.h         |   6 +
 drivers/staging/rtl8192u/r8192U_core.c    | 223 ++++------------------
 drivers/staging/rtl8192u/r8192U_debugfs.c | 188 ++++++++++++++++++
 4 files changed, 235 insertions(+), 183 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_debugfs.c

-- 
2.25.1

Re: [PATCH v2 0/3] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Dan Carpenter]

On Mon, Jul 18, 2022 at 10:50:35PM -0700, Tong Zhang wrote:
> There are 4 debug files created under /proc/net/[Devname]. Devname could
> be wlan0 initially, however it could be renamed later to e.g. enx00e04c000002.
> This will cause problem during debug file teardown since it uses
> netdev->name which is no longer wlan0. To solve this problem, add a
> notifier to handle device renaming.
> 
> Also, due to this is purely for debuging as files are created read only,
> move this to debugfs like other NIC drivers do instead of using procfs.
> 
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Tested-by: Zheyu Ma <zheyuma97@gmail.com>
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> 
> v2: break down patch and fix pointer check
> 
> Tong Zhang (3):
>   staging: rtl8192u: move debug stuff to its own file
>   staging: rtl8192u: move debug files to debugfs
>   staging: rtl8192u: fix rmmod warn when wlan0 is renamed

Thanks!  This is much better.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

[PATCH v2 1/3] staging: rtl8192u: move debug stuff to its own file

[Tong Zhang]

This is to prepare for moving them to debugfs and fix rmmod warn issue
when wlan0 is renamed to something else.

Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile        |   1 +
 drivers/staging/rtl8192u/r8192U.h        |   4 +
 drivers/staging/rtl8192u/r8192U_core.c   | 173 ----------------------
 drivers/staging/rtl8192u/r8192U_procfs.c | 176 +++++++++++++++++++++++
 4 files changed, 181 insertions(+), 173 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_procfs.c

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 0be7426b6ebc..5aef46cc90ef 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,6 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
+		  r8192U_procfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 14ca00a2789b..e8b6da2adc4d 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1117,4 +1117,8 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
+void rtl8192_proc_module_init(void);
+void rtl8192_proc_init_one(struct net_device *dev);
+void rtl8192_proc_remove_one(struct net_device *dev);
+
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 2ca925f35830..9e0861fdc64e 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -452,179 +452,6 @@ static struct net_device_stats *rtl8192_stats(struct net_device *dev);
 static void rtl8192_restart(struct work_struct *work);
 static void watch_dog_timer_callback(struct timer_list *t);
 
-/****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
- ****************************************************************************/
-
-static struct proc_dir_entry *rtl8192_proc;
-
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-	struct ieee80211_device *ieee = priv->ieee80211;
-	struct ieee80211_network *target;
-
-	list_for_each_entry(target, &ieee->network_list, list) {
-		const char *wpa = "non_WPA";
-
-		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
-			wpa = "WPA";
-
-		seq_printf(m, "%s %s\n", target->ssid, wpa);
-	}
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	int i, n, max = 0xff;
-	u8 byte_rd;
-
-	seq_puts(m, "\n####################page 0##################\n ");
-
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x000 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 1##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x100 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 3##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x300 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_putc(m, '\n');
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "TX VI priority ok int: %lu\n"
-		   "TX VI priority error int: %lu\n"
-		   "TX VO priority ok int: %lu\n"
-		   "TX VO priority error int: %lu\n"
-		   "TX BE priority ok int: %lu\n"
-		   "TX BE priority error int: %lu\n"
-		   "TX BK priority ok int: %lu\n"
-		   "TX BK priority error int: %lu\n"
-		   "TX MANAGE priority ok int: %lu\n"
-		   "TX MANAGE priority error int: %lu\n"
-		   "TX BEACON priority ok int: %lu\n"
-		   "TX BEACON priority error int: %lu\n"
-		   "TX queue resume: %lu\n"
-		   "TX queue stopped?: %d\n"
-		   "TX fifo overflow: %lu\n"
-		   "TX VI queue: %d\n"
-		   "TX VO queue: %d\n"
-		   "TX BE queue: %d\n"
-		   "TX BK queue: %d\n"
-		   "TX VI dropped: %lu\n"
-		   "TX VO dropped: %lu\n"
-		   "TX BE dropped: %lu\n"
-		   "TX BK dropped: %lu\n"
-		   "TX total data packets %lu\n",
-		   priv->stats.txviokint,
-		   priv->stats.txvierr,
-		   priv->stats.txvookint,
-		   priv->stats.txvoerr,
-		   priv->stats.txbeokint,
-		   priv->stats.txbeerr,
-		   priv->stats.txbkokint,
-		   priv->stats.txbkerr,
-		   priv->stats.txmanageokint,
-		   priv->stats.txmanageerr,
-		   priv->stats.txbeaconokint,
-		   priv->stats.txbeaconerr,
-		   priv->stats.txresumed,
-		   netif_queue_stopped(dev),
-		   priv->stats.txoverflow,
-		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
-		   priv->stats.txvidrop,
-		   priv->stats.txvodrop,
-		   priv->stats.txbedrop,
-		   priv->stats.txbkdrop,
-		   priv->stats.txdatapkt
-		);
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "RX packets: %lu\n"
-		   "RX urb status error: %lu\n"
-		   "RX invalid urb error: %lu\n",
-		   priv->stats.rxoktotal,
-		   priv->stats.rxstaterr,
-		   priv->stats.rxurberr);
-
-	return 0;
-}
-
-static void rtl8192_proc_module_init(void)
-{
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
-}
-
-static void rtl8192_proc_init_one(struct net_device *dev)
-{
-	struct proc_dir_entry *dir;
-
-	if (!rtl8192_proc)
-		return;
-
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
-
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
-}
-
-static void rtl8192_proc_remove_one(struct net_device *dev)
-{
-	remove_proc_subtree(dev->name, rtl8192_proc);
-}
-
 /****************************************************************************
  *  -----------------------------MISC STUFF-------------------------
  *****************************************************************************/
diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_procfs.c
new file mode 100644
index 000000000000..cc69d78d5152
--- /dev/null
+++ b/drivers/staging/rtl8192u/r8192U_procfs.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/****************************************************************************
+ *   -----------------------------PROCFS STUFF-------------------------
+ ****************************************************************************/
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include "r8192U.h"
+
+static struct proc_dir_entry *rtl8192_proc;
+static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct ieee80211_device *ieee = priv->ieee80211;
+	struct ieee80211_network *target;
+
+	list_for_each_entry(target, &ieee->network_list, list) {
+		const char *wpa = "non_WPA";
+
+		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
+			wpa = "WPA";
+
+		seq_printf(m, "%s %s\n", target->ssid, wpa);
+	}
+
+	return 0;
+}
+
+static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	int i, n, max = 0xff;
+	u8 byte_rd;
+
+	seq_puts(m, "\n####################page 0##################\n ");
+
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x000 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 1##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x100 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 3##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x300 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_putc(m, '\n');
+	return 0;
+}
+
+static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+		   "TX VI priority ok int: %lu\n"
+		   "TX VI priority error int: %lu\n"
+		   "TX VO priority ok int: %lu\n"
+		   "TX VO priority error int: %lu\n"
+		   "TX BE priority ok int: %lu\n"
+		   "TX BE priority error int: %lu\n"
+		   "TX BK priority ok int: %lu\n"
+		   "TX BK priority error int: %lu\n"
+		   "TX MANAGE priority ok int: %lu\n"
+		   "TX MANAGE priority error int: %lu\n"
+		   "TX BEACON priority ok int: %lu\n"
+		   "TX BEACON priority error int: %lu\n"
+		   "TX queue resume: %lu\n"
+		   "TX queue stopped?: %d\n"
+		   "TX fifo overflow: %lu\n"
+		   "TX VI queue: %d\n"
+		   "TX VO queue: %d\n"
+		   "TX BE queue: %d\n"
+		   "TX BK queue: %d\n"
+		   "TX VI dropped: %lu\n"
+		   "TX VO dropped: %lu\n"
+		   "TX BE dropped: %lu\n"
+		   "TX BK dropped: %lu\n"
+		   "TX total data packets %lu\n",
+		   priv->stats.txviokint,
+		   priv->stats.txvierr,
+		   priv->stats.txvookint,
+		   priv->stats.txvoerr,
+		   priv->stats.txbeokint,
+		   priv->stats.txbeerr,
+		   priv->stats.txbkokint,
+		   priv->stats.txbkerr,
+		   priv->stats.txmanageokint,
+		   priv->stats.txmanageerr,
+		   priv->stats.txbeaconokint,
+		   priv->stats.txbeaconerr,
+		   priv->stats.txresumed,
+		   netif_queue_stopped(dev),
+		   priv->stats.txoverflow,
+		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
+		   priv->stats.txvidrop,
+		   priv->stats.txvodrop,
+		   priv->stats.txbedrop,
+		   priv->stats.txbkdrop,
+		   priv->stats.txdatapkt
+		);
+
+	return 0;
+}
+
+static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+		   "RX packets: %lu\n"
+		   "RX urb status error: %lu\n"
+		   "RX invalid urb error: %lu\n",
+		   priv->stats.rxoktotal,
+		   priv->stats.rxstaterr,
+		   priv->stats.rxurberr);
+
+	return 0;
+}
+
+void rtl8192_proc_module_init(void)
+{
+	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
+	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
+}
+
+void rtl8192_proc_init_one(struct net_device *dev)
+{
+	struct proc_dir_entry *dir;
+
+	if (!rtl8192_proc)
+		return;
+
+	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
+	if (!dir)
+		return;
+
+	proc_create_single("stats-rx", S_IFREG | 0444, dir,
+			   proc_get_stats_rx);
+	proc_create_single("stats-tx", S_IFREG | 0444, dir,
+			   proc_get_stats_tx);
+	proc_create_single("stats-ap", S_IFREG | 0444, dir,
+			   proc_get_stats_ap);
+	proc_create_single("registers", S_IFREG | 0444, dir,
+			   proc_get_registers);
+}
+
+void rtl8192_proc_remove_one(struct net_device *dev)
+{
+	remove_proc_subtree(dev->name, rtl8192_proc);
+}
+
-- 
2.25.1

[PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname]. Devname could
Due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.
This is also to prepare for address rmmod warn issue.

Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile             |  2 +-
 drivers/staging/rtl8192u/r8192U.h             |  9 +--
 drivers/staging/rtl8192u/r8192U_core.c        | 15 ++---
 .../{r8192U_procfs.c => r8192U_debugfs.c}     | 55 +++++++++----------
 4 files changed, 39 insertions(+), 42 deletions(-)
 rename drivers/staging/rtl8192u/{r8192U_procfs.c => r8192U_debugfs.c} (73%)

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 5aef46cc90ef..d32dfd89a606 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,7 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
-		  r8192U_procfs.o					\
+		  r8192U_debugfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index e8b6da2adc4d..e8860bb2b607 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
 	struct delayed_work gpio_change_rf_wq;
 	struct delayed_work initialgain_operate_wq;
 	struct workqueue_struct *priv_wq;
+
+	/* debugfs */
+	struct dentry *debugfs_dir;
 } r8192_priv;
 
 /* For rtl8187B */
@@ -1117,8 +1120,6 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
-void rtl8192_proc_module_init(void);
-void rtl8192_proc_init_one(struct net_device *dev);
-void rtl8192_proc_remove_one(struct net_device *dev);
-
+void rtl8192_debugfs_init(struct net_device *dev);
+void rtl8192_debugfs_exit(struct net_device *dev);
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 9e0861fdc64e..ac3716550505 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -56,7 +56,6 @@ double __extendsfdf2(float a)
 #include "r8192U_dm.h"
 #include <linux/usb.h>
 #include <linux/slab.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 /* FIXME: check if 2.6.7 is ok */
 
@@ -4557,7 +4556,7 @@ static int rtl8192_usb_probe(struct usb_interface *intf,
 		goto fail2;
 
 	RT_TRACE(COMP_INIT, "dev name=======> %s\n", dev->name);
-	rtl8192_proc_init_one(dev);
+	rtl8192_debugfs_init(dev);
 
 	RT_TRACE(COMP_INIT, "Driver probe completed\n");
 	return 0;
@@ -4591,10 +4590,11 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	struct net_device *dev = usb_get_intfdata(intf);
 	struct r8192_priv *priv = ieee80211_priv(dev);
 
-	unregister_netdev(dev);
 
 	RT_TRACE(COMP_DOWN, "=============>wlan driver to be removed\n");
-	rtl8192_proc_remove_one(dev);
+	rtl8192_debugfs_exit(dev);
+
+	unregister_netdev(dev);
 
 	rtl8192_down(dev);
 	kfree(priv->pFirmware);
@@ -4645,14 +4645,12 @@ static int __init rtl8192_usb_module_init(void)
 		goto crypto_ccmp_exit;
 	}
 
-	rtl8192_proc_module_init();
 	ret = usb_register(&rtl8192_usb_driver);
 	if (ret)
-		goto rtl8192_proc_module_exit;
+		goto crypto_wep_exit;
 	return ret;
 
-rtl8192_proc_module_exit:
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
+crypto_wep_exit:
 	ieee80211_crypto_wep_exit();
 crypto_ccmp_exit:
 	ieee80211_crypto_ccmp_exit();
@@ -4668,7 +4666,6 @@ static int __init rtl8192_usb_module_init(void)
 static void __exit rtl8192_usb_module_exit(void)
 {
 	usb_deregister(&rtl8192_usb_driver);
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
 	ieee80211_crypto_wep_exit();
 	ieee80211_crypto_ccmp_exit();
 	ieee80211_crypto_tkip_exit();
diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
similarity index 73%
rename from drivers/staging/rtl8192u/r8192U_procfs.c
rename to drivers/staging/rtl8192u/r8192U_debugfs.c
index cc69d78d5152..5c9376e50889 100644
--- a/drivers/staging/rtl8192u/r8192U_procfs.c
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -1,13 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
 /****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
+ *   -----------------------------DEGUGFS STUFF-------------------------
  ****************************************************************************/
-#include <linux/proc_fs.h>
+#include <linux/debugfs.h>
 #include <linux/seq_file.h>
 #include "r8192U.h"
 
-static struct proc_dir_entry *rtl8192_proc;
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_ap_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
@@ -26,7 +25,7 @@ static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
+static int rtl8192_usb_registers_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	int i, n, max = 0xff;
@@ -67,7 +66,7 @@ static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_tx_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
@@ -126,7 +125,7 @@ static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_rx_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
@@ -142,35 +141,35 @@ static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
 	return 0;
 }
 
-void rtl8192_proc_module_init(void)
-{
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
-}
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_rx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_tx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_ap);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_registers);
 
-void rtl8192_proc_init_one(struct net_device *dev)
+void rtl8192_debugfs_init(struct net_device *dev)
 {
-	struct proc_dir_entry *dir;
+	struct dentry *dir;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
 
-	if (!rtl8192_proc)
+	dir = debugfs_create_dir(dev->name, NULL);
+	if (IS_ERR(dir))
 		return;
 
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
+	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
+	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
+	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
+	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
 
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
+	priv->debugfs_dir = dir;
 }
 
-void rtl8192_proc_remove_one(struct net_device *dev)
+void rtl8192_debugfs_exit(struct net_device *dev)
 {
-	remove_proc_subtree(dev->name, rtl8192_proc);
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	if (!priv->debugfs_dir)
+		return;
+	debugfs_remove_recursive(priv->debugfs_dir);
+	priv->debugfs_dir = NULL;
 }
 
-- 
2.25.1

Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Greg Kroah-Hartman]

On Mon, Jul 18, 2022 at 10:50:37PM -0700, Tong Zhang wrote:
> There are 4 debug files created under /proc/net/[Devname]. Devname could
> Due to this is purely for debuging as files are created read only,
> move this to debugfs like other NIC drivers do instead of using procfs.
> This is also to prepare for address rmmod warn issue.

Minor comments based on good debugfs usage:

> --- a/drivers/staging/rtl8192u/r8192U.h
> +++ b/drivers/staging/rtl8192u/r8192U.h
> @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
>  	struct delayed_work gpio_change_rf_wq;
>  	struct delayed_work initialgain_operate_wq;
>  	struct workqueue_struct *priv_wq;
> +
> +	/* debugfs */
> +	struct dentry *debugfs_dir;

Why do you need to save this dentry?  Can't you just look it up when you
want to remove the files?

> +void rtl8192_debugfs_init(struct net_device *dev)
>  {
> -	struct proc_dir_entry *dir;
> +	struct dentry *dir;
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);

No need to cast this.  Same for later on in this file.

> -	if (!rtl8192_proc)
> +	dir = debugfs_create_dir(dev->name, NULL);
> +	if (IS_ERR(dir))
>  		return;

No need to check, just keep moving on.

>  
> -	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
> -	if (!dir)
> -		return;
> +	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> +	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> +	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> +	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
>  
> -	proc_create_single("stats-rx", S_IFREG | 0444, dir,
> -			   proc_get_stats_rx);
> -	proc_create_single("stats-tx", S_IFREG | 0444, dir,
> -			   proc_get_stats_tx);
> -	proc_create_single("stats-ap", S_IFREG | 0444, dir,
> -			   proc_get_stats_ap);
> -	proc_create_single("registers", S_IFREG | 0444, dir,
> -			   proc_get_registers);
> +	priv->debugfs_dir = dir;

Again, no need to save this, just look it up when removing the
directory.

thanks,

greg k-h

Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Tong Zhang]

On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 18, 2022 at 10:50:37PM -0700, Tong Zhang wrote:
> > There are 4 debug files created under /proc/net/[Devname]. Devname could
> > Due to this is purely for debuging as files are created read only,
> > move this to debugfs like other NIC drivers do instead of using procfs.
> > This is also to prepare for address rmmod warn issue.
>
> Minor comments based on good debugfs usage:
>
> > --- a/drivers/staging/rtl8192u/r8192U.h
> > +++ b/drivers/staging/rtl8192u/r8192U.h
> > @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
> >       struct delayed_work gpio_change_rf_wq;
> >       struct delayed_work initialgain_operate_wq;
> >       struct workqueue_struct *priv_wq;
> > +
> > +     /* debugfs */
> > +     struct dentry *debugfs_dir;
>
> Why do you need to save this dentry?  Can't you just look it up when you
> want to remove the files?
>

Hi Greg,
Thanks for the comments.

I am thinking whether it is possible to rename the device and run
rmmod to remove something it shouldn't.
If we are using debugfs_lookup(dev->name, NULL), say, the existing
directories/files are

  /sys/kernel/debug/DIRA
  /sys/kernel/debug/wlan0

I then rename device wlan0 to DIRA, after that I remove the module by
doing rmmod.
Apparently either the wlan0 directory will not be renamed successfully
due to collision or DIRA directory might be overwritten? (this part I
haven't checked yet)
Either Way,  later on if we do rmmod, the driver will try to do
debugfs_lookup("fileA", NULL) and remove /sys/kernel/debug/DIRA which
it shouldn't.
Or if it is possible to rename the device to some wacky string
containing wildcard or .. to launch an attack.

Maybe I am being naive but please correct me if I am wrong.

Or maybe we should put those debug files under the module's own
directory and do lookup from there instead. like the following dir
structure

/sys/kernel/debug/r8192u_usb/wlan0/stats-rx
/sys/kernel/debug/r8192u_usb/wlan0/stats-rx
/sys/kernel/debug/r8192u_usb/wlan0/stats-ap
/sys/kernel/debug/r8192u_usb/wlan0/registers


> > +void rtl8192_debugfs_init(struct net_device *dev)
> >  {
> > -     struct proc_dir_entry *dir;
> > +     struct dentry *dir;
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
>
> No need to cast this.  Same for later on in this file.
>

agreed, will remove those redundant checks.

> > -     if (!rtl8192_proc)
> > +     dir = debugfs_create_dir(dev->name, NULL);
> > +     if (IS_ERR(dir))
> >               return;
>
> No need to check, just keep moving on.
>
> >
> > -     dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
> > -     if (!dir)
> > -             return;
> > +     debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> > +     debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> > +     debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> > +     debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> >
> > -     proc_create_single("stats-rx", S_IFREG | 0444, dir,
> > -                        proc_get_stats_rx);
> > -     proc_create_single("stats-tx", S_IFREG | 0444, dir,
> > -                        proc_get_stats_tx);
> > -     proc_create_single("stats-ap", S_IFREG | 0444, dir,
> > -                        proc_get_stats_ap);
> > -     proc_create_single("registers", S_IFREG | 0444, dir,
> > -                        proc_get_registers);
> > +     priv->debugfs_dir = dir;
>
> Again, no need to save this, just look it up when removing the
> directory.
>
> thanks,
>
> greg k-h

Thanks and have a good one!
- Tong

Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Greg Kroah-Hartman]

On Tue, Jul 19, 2022 at 09:58:24PM -0700, Tong Zhang wrote:
> On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jul 18, 2022 at 10:50:37PM -0700, Tong Zhang wrote:
> > > There are 4 debug files created under /proc/net/[Devname]. Devname could
> > > Due to this is purely for debuging as files are created read only,
> > > move this to debugfs like other NIC drivers do instead of using procfs.
> > > This is also to prepare for address rmmod warn issue.
> >
> > Minor comments based on good debugfs usage:
> >
> > > --- a/drivers/staging/rtl8192u/r8192U.h
> > > +++ b/drivers/staging/rtl8192u/r8192U.h
> > > @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
> > >       struct delayed_work gpio_change_rf_wq;
> > >       struct delayed_work initialgain_operate_wq;
> > >       struct workqueue_struct *priv_wq;
> > > +
> > > +     /* debugfs */
> > > +     struct dentry *debugfs_dir;
> >
> > Why do you need to save this dentry?  Can't you just look it up when you
> > want to remove the files?
> >
> 
> Hi Greg,
> Thanks for the comments.
> 
> I am thinking whether it is possible to rename the device and run
> rmmod to remove something it shouldn't.
> If we are using debugfs_lookup(dev->name, NULL), say, the existing
> directories/files are
> 
>   /sys/kernel/debug/DIRA
>   /sys/kernel/debug/wlan0

Ick, wait, that's not good.  We need a driver subdirectory name in there
so that this driver doesn't stomp over some other driver that might be
doing the same foolish thing and using the root of debugfs.

> I then rename device wlan0 to DIRA, after that I remove the module by
> doing rmmod.
> Apparently either the wlan0 directory will not be renamed successfully
> due to collision or DIRA directory might be overwritten? (this part I
> haven't checked yet)
> Either Way,  later on if we do rmmod, the driver will try to do
> debugfs_lookup("fileA", NULL) and remove /sys/kernel/debug/DIRA which
> it shouldn't.
> Or if it is possible to rename the device to some wacky string
> containing wildcard or .. to launch an attack.
> 
> Maybe I am being naive but please correct me if I am wrong.
> 
> Or maybe we should put those debug files under the module's own
> directory and do lookup from there instead. like the following dir
> structure
> 
> /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
> /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
> /sys/kernel/debug/r8192u_usb/wlan0/stats-ap
> /sys/kernel/debug/r8192u_usb/wlan0/registers

Yes, that would be much much better.  That way you "know" you can look
up the name again correctly.

thanks,

greg k-h

Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Tong Zhang]

On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 18, 2022 at 10:50:37PM -0700, Tong Zhang wrote:
> > There are 4 debug files created under /proc/net/[Devname]. Devname could
> > Due to this is purely for debuging as files are created read only,
> > move this to debugfs like other NIC drivers do instead of using procfs.
> > This is also to prepare for address rmmod warn issue.
>
> Minor comments based on good debugfs usage:
>
> > --- a/drivers/staging/rtl8192u/r8192U.h
> > +++ b/drivers/staging/rtl8192u/r8192U.h
> > @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
> >       struct delayed_work gpio_change_rf_wq;
> >       struct delayed_work initialgain_operate_wq;
> >       struct workqueue_struct *priv_wq;
> > +
> > +     /* debugfs */
> > +     struct dentry *debugfs_dir;
>
> Why do you need to save this dentry?  Can't you just look it up when you
> want to remove the files?
>
> > +void rtl8192_debugfs_init(struct net_device *dev)
> >  {
> > -     struct proc_dir_entry *dir;
> > +     struct dentry *dir;
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
>
> No need to cast this.  Same for later on in this file.
>
> > -     if (!rtl8192_proc)
> > +     dir = debugfs_create_dir(dev->name, NULL);
> > +     if (IS_ERR(dir))
> >               return;
>

I'm reading this code and your comment again.
Adding this check will avoid calling into debugfs_create_file() and 4
function calls and doing checks from there, probably will save a
couple of CPU cycles and avoid branch prediction penalty if there is
any.
I don't think the compiler can optimize for this case though it's not
performance critical. Anyho I personally feel it is better to keep
this.

> No need to check, just keep moving on.
>
> >
> > -     dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
> > -     if (!dir)
> > -             return;
> > +     debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> > +     debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> > +     debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> > +     debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> >
> > -     proc_create_single("stats-rx", S_IFREG | 0444, dir,
> > -                        proc_get_stats_rx);
> > -     proc_create_single("stats-tx", S_IFREG | 0444, dir,
> > -                        proc_get_stats_tx);
> > -     proc_create_single("stats-ap", S_IFREG | 0444, dir,
> > -                        proc_get_stats_ap);
> > -     proc_create_single("registers", S_IFREG | 0444, dir,
> > -                        proc_get_registers);
> > +     priv->debugfs_dir = dir;

Please correct me if I am wrong (yes probably).
As I mentioned in another email, I am not sure if there is another way
to look up the old entry so I'm saving the old entry here.
I cheated a little bit by using similar routine as in

net/mac80211/debugfs_netdev.c:865 void
ieee80211_debugfs_rename_netdev(struct ieee80211_sub_if_data *sdata)
drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c: 520 void
xgbe_debugfs_rename(struct xgbe_prv_data *pdata)

>
> Again, no need to save this, just look it up when removing the
> directory.
>
> thanks,
>
> greg k-h


Thanks and have a good one!
- Tong

Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Greg Kroah-Hartman]

On Tue, Jul 19, 2022 at 11:30:48PM -0700, Tong Zhang wrote:
> On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jul 18, 2022 at 10:50:37PM -0700, Tong Zhang wrote:
> > > There are 4 debug files created under /proc/net/[Devname]. Devname could
> > > Due to this is purely for debuging as files are created read only,
> > > move this to debugfs like other NIC drivers do instead of using procfs.
> > > This is also to prepare for address rmmod warn issue.
> >
> > Minor comments based on good debugfs usage:
> >
> > > --- a/drivers/staging/rtl8192u/r8192U.h
> > > +++ b/drivers/staging/rtl8192u/r8192U.h
> > > @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
> > >       struct delayed_work gpio_change_rf_wq;
> > >       struct delayed_work initialgain_operate_wq;
> > >       struct workqueue_struct *priv_wq;
> > > +
> > > +     /* debugfs */
> > > +     struct dentry *debugfs_dir;
> >
> > Why do you need to save this dentry?  Can't you just look it up when you
> > want to remove the files?
> >
> > > +void rtl8192_debugfs_init(struct net_device *dev)
> > >  {
> > > -     struct proc_dir_entry *dir;
> > > +     struct dentry *dir;
> > > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> >
> > No need to cast this.  Same for later on in this file.
> >
> > > -     if (!rtl8192_proc)
> > > +     dir = debugfs_create_dir(dev->name, NULL);
> > > +     if (IS_ERR(dir))
> > >               return;
> >
> 
> I'm reading this code and your comment again.
> Adding this check will avoid calling into debugfs_create_file() and 4
> function calls and doing checks from there, probably will save a
> couple of CPU cycles and avoid branch prediction penalty if there is
> any.
> I don't think the compiler can optimize for this case though it's not
> performance critical. Anyho I personally feel it is better to keep
> this.

It's not an optimization issue, it's a "we never care about the results
of a call to debugfs_*() issue".

That's all, debugfs is intended to be easy to use, and you should never
care about the return values of if it worked or not, so your code should
not check it and do anything different based on it.

Yes, it's not like "normal" kernel code, but debugfs is not normal at
all, and should never expect to work as it's only for debugging.

thanks,

greg k-h

Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs

[Tong Zhang]

On Tue, Jul 26, 2022 at 11:37 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jul 19, 2022 at 11:30:48PM -0700, Tong Zhang wrote:
> > On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Jul 18, 2022 at 10:50:37PM -0700, Tong Zhang wrote:
> > > > There are 4 debug files created under /proc/net/[Devname]. Devname could
> > > > Due to this is purely for debuging as files are created read only,
> > > > move this to debugfs like other NIC drivers do instead of using procfs.
> > > > This is also to prepare for address rmmod warn issue.
> > >
> > > Minor comments based on good debugfs usage:
> > >
> > > > --- a/drivers/staging/rtl8192u/r8192U.h
> > > > +++ b/drivers/staging/rtl8192u/r8192U.h
> > > > @@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
> > > >       struct delayed_work gpio_change_rf_wq;
> > > >       struct delayed_work initialgain_operate_wq;
> > > >       struct workqueue_struct *priv_wq;
> > > > +
> > > > +     /* debugfs */
> > > > +     struct dentry *debugfs_dir;
> > >
> > > Why do you need to save this dentry?  Can't you just look it up when you
> > > want to remove the files?
> > >
> > > > +void rtl8192_debugfs_init(struct net_device *dev)
> > > >  {
> > > > -     struct proc_dir_entry *dir;
> > > > +     struct dentry *dir;
> > > > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > >
> > > No need to cast this.  Same for later on in this file.
> > >
> > > > -     if (!rtl8192_proc)
> > > > +     dir = debugfs_create_dir(dev->name, NULL);
> > > > +     if (IS_ERR(dir))
> > > >               return;
> > >
> >
> > I'm reading this code and your comment again.
> > Adding this check will avoid calling into debugfs_create_file() and 4
> > function calls and doing checks from there, probably will save a
> > couple of CPU cycles and avoid branch prediction penalty if there is
> > any.
> > I don't think the compiler can optimize for this case though it's not
> > performance critical. Anyho I personally feel it is better to keep
> > this.
>
> It's not an optimization issue, it's a "we never care about the results
> of a call to debugfs_*() issue".
>
> That's all, debugfs is intended to be easy to use, and you should never
> care about the return values of if it worked or not, so your code should
> not check it and do anything different based on it.
>
> Yes, it's not like "normal" kernel code, but debugfs is not normal at
> all, and should never expect to work as it's only for debugging.

Hi Greg,
Thanks for your review. I am mostly on the same page with you.
IMHO, since Ubuntu(or Debian derivative in general?) enables debugfs by default,
I think we need to make those ``debug''' code work although they might
not be designed to be used like this.
To my limited knowledge, iotop actually uses debugfs to show IO utilization.
(There might be more though)
Anyway, I incorporated your review and sent it as v3.
Thanks again!
- Tong

>
> thanks,
>
> greg k-h

[PATCH v3 0/3] staging: rtl8192u: fix rmmod warn when device is renamed

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname] by
rtl8192u_usb. Devname could be wlan0 initially, however it could be
renamed later to e.g. enx00e04c000002. This will cause problem during
debug file teardown since it uses netdev->name which is no longer wlan0.
To solve this problem, add a notifier to handle device renaming.

Also, due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.

The directory structure after this patch set will be like the following

  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-ap
  /sys/kernel/debug/r8192u_usb/wlan0/registers

Also note that we cannot simply do debugfs_lookup to find out old dentry
since by the time the notifier is called, netdev->name is already changed
to new name. So here we still save the original dentry.


Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Tested-by: Zheyu Ma <zheyuma97@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>

v2: break down patch and fix pointer check
v3: removed unnecessary checks, casts and move debug files under module's
own directory, only minor change compared to v2

Tong Zhang (3):
  staging: rtl8192u: move debug stuff to its own file
  staging: rtl8192u: move debug files to debugfs
  staging: rtl8192u: fix rmmod warn when device is renamed

 drivers/staging/rtl8192u/Makefile         |   1 +
 drivers/staging/rtl8192u/r8192U.h         |   9 +
 drivers/staging/rtl8192u/r8192U_core.c    | 226 ++++------------------
 drivers/staging/rtl8192u/r8192U_debugfs.c | 189 ++++++++++++++++++
 4 files changed, 242 insertions(+), 183 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_debugfs.c

-- 
2.25.1

[PATCH v3 1/3] staging: rtl8192u: move debug stuff to its own file

[Tong Zhang]

This is to prepare for moving them to debugfs and fix rmmod warn issue
when wlan0 is renamed to something else.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile        |   1 +
 drivers/staging/rtl8192u/r8192U.h        |   4 +
 drivers/staging/rtl8192u/r8192U_core.c   | 173 ----------------------
 drivers/staging/rtl8192u/r8192U_procfs.c | 176 +++++++++++++++++++++++
 4 files changed, 181 insertions(+), 173 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_procfs.c

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 0be7426b6ebc..5aef46cc90ef 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,6 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
+		  r8192U_procfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 14ca00a2789b..e8b6da2adc4d 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1117,4 +1117,8 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
+void rtl8192_proc_module_init(void);
+void rtl8192_proc_init_one(struct net_device *dev);
+void rtl8192_proc_remove_one(struct net_device *dev);
+
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 2ca925f35830..9e0861fdc64e 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -452,179 +452,6 @@ static struct net_device_stats *rtl8192_stats(struct net_device *dev);
 static void rtl8192_restart(struct work_struct *work);
 static void watch_dog_timer_callback(struct timer_list *t);
 
-/****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
- ****************************************************************************/
-
-static struct proc_dir_entry *rtl8192_proc;
-
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-	struct ieee80211_device *ieee = priv->ieee80211;
-	struct ieee80211_network *target;
-
-	list_for_each_entry(target, &ieee->network_list, list) {
-		const char *wpa = "non_WPA";
-
-		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
-			wpa = "WPA";
-
-		seq_printf(m, "%s %s\n", target->ssid, wpa);
-	}
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	int i, n, max = 0xff;
-	u8 byte_rd;
-
-	seq_puts(m, "\n####################page 0##################\n ");
-
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x000 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 1##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x100 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 3##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x300 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_putc(m, '\n');
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "TX VI priority ok int: %lu\n"
-		   "TX VI priority error int: %lu\n"
-		   "TX VO priority ok int: %lu\n"
-		   "TX VO priority error int: %lu\n"
-		   "TX BE priority ok int: %lu\n"
-		   "TX BE priority error int: %lu\n"
-		   "TX BK priority ok int: %lu\n"
-		   "TX BK priority error int: %lu\n"
-		   "TX MANAGE priority ok int: %lu\n"
-		   "TX MANAGE priority error int: %lu\n"
-		   "TX BEACON priority ok int: %lu\n"
-		   "TX BEACON priority error int: %lu\n"
-		   "TX queue resume: %lu\n"
-		   "TX queue stopped?: %d\n"
-		   "TX fifo overflow: %lu\n"
-		   "TX VI queue: %d\n"
-		   "TX VO queue: %d\n"
-		   "TX BE queue: %d\n"
-		   "TX BK queue: %d\n"
-		   "TX VI dropped: %lu\n"
-		   "TX VO dropped: %lu\n"
-		   "TX BE dropped: %lu\n"
-		   "TX BK dropped: %lu\n"
-		   "TX total data packets %lu\n",
-		   priv->stats.txviokint,
-		   priv->stats.txvierr,
-		   priv->stats.txvookint,
-		   priv->stats.txvoerr,
-		   priv->stats.txbeokint,
-		   priv->stats.txbeerr,
-		   priv->stats.txbkokint,
-		   priv->stats.txbkerr,
-		   priv->stats.txmanageokint,
-		   priv->stats.txmanageerr,
-		   priv->stats.txbeaconokint,
-		   priv->stats.txbeaconerr,
-		   priv->stats.txresumed,
-		   netif_queue_stopped(dev),
-		   priv->stats.txoverflow,
-		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
-		   priv->stats.txvidrop,
-		   priv->stats.txvodrop,
-		   priv->stats.txbedrop,
-		   priv->stats.txbkdrop,
-		   priv->stats.txdatapkt
-		);
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "RX packets: %lu\n"
-		   "RX urb status error: %lu\n"
-		   "RX invalid urb error: %lu\n",
-		   priv->stats.rxoktotal,
-		   priv->stats.rxstaterr,
-		   priv->stats.rxurberr);
-
-	return 0;
-}
-
-static void rtl8192_proc_module_init(void)
-{
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
-}
-
-static void rtl8192_proc_init_one(struct net_device *dev)
-{
-	struct proc_dir_entry *dir;
-
-	if (!rtl8192_proc)
-		return;
-
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
-
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
-}
-
-static void rtl8192_proc_remove_one(struct net_device *dev)
-{
-	remove_proc_subtree(dev->name, rtl8192_proc);
-}
-
 /****************************************************************************
  *  -----------------------------MISC STUFF-------------------------
  *****************************************************************************/
diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_procfs.c
new file mode 100644
index 000000000000..cc69d78d5152
--- /dev/null
+++ b/drivers/staging/rtl8192u/r8192U_procfs.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/****************************************************************************
+ *   -----------------------------PROCFS STUFF-------------------------
+ ****************************************************************************/
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include "r8192U.h"
+
+static struct proc_dir_entry *rtl8192_proc;
+static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct ieee80211_device *ieee = priv->ieee80211;
+	struct ieee80211_network *target;
+
+	list_for_each_entry(target, &ieee->network_list, list) {
+		const char *wpa = "non_WPA";
+
+		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
+			wpa = "WPA";
+
+		seq_printf(m, "%s %s\n", target->ssid, wpa);
+	}
+
+	return 0;
+}
+
+static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	int i, n, max = 0xff;
+	u8 byte_rd;
+
+	seq_puts(m, "\n####################page 0##################\n ");
+
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x000 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 1##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x100 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 3##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x300 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_putc(m, '\n');
+	return 0;
+}
+
+static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+		   "TX VI priority ok int: %lu\n"
+		   "TX VI priority error int: %lu\n"
+		   "TX VO priority ok int: %lu\n"
+		   "TX VO priority error int: %lu\n"
+		   "TX BE priority ok int: %lu\n"
+		   "TX BE priority error int: %lu\n"
+		   "TX BK priority ok int: %lu\n"
+		   "TX BK priority error int: %lu\n"
+		   "TX MANAGE priority ok int: %lu\n"
+		   "TX MANAGE priority error int: %lu\n"
+		   "TX BEACON priority ok int: %lu\n"
+		   "TX BEACON priority error int: %lu\n"
+		   "TX queue resume: %lu\n"
+		   "TX queue stopped?: %d\n"
+		   "TX fifo overflow: %lu\n"
+		   "TX VI queue: %d\n"
+		   "TX VO queue: %d\n"
+		   "TX BE queue: %d\n"
+		   "TX BK queue: %d\n"
+		   "TX VI dropped: %lu\n"
+		   "TX VO dropped: %lu\n"
+		   "TX BE dropped: %lu\n"
+		   "TX BK dropped: %lu\n"
+		   "TX total data packets %lu\n",
+		   priv->stats.txviokint,
+		   priv->stats.txvierr,
+		   priv->stats.txvookint,
+		   priv->stats.txvoerr,
+		   priv->stats.txbeokint,
+		   priv->stats.txbeerr,
+		   priv->stats.txbkokint,
+		   priv->stats.txbkerr,
+		   priv->stats.txmanageokint,
+		   priv->stats.txmanageerr,
+		   priv->stats.txbeaconokint,
+		   priv->stats.txbeaconerr,
+		   priv->stats.txresumed,
+		   netif_queue_stopped(dev),
+		   priv->stats.txoverflow,
+		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
+		   priv->stats.txvidrop,
+		   priv->stats.txvodrop,
+		   priv->stats.txbedrop,
+		   priv->stats.txbkdrop,
+		   priv->stats.txdatapkt
+		);
+
+	return 0;
+}
+
+static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+		   "RX packets: %lu\n"
+		   "RX urb status error: %lu\n"
+		   "RX invalid urb error: %lu\n",
+		   priv->stats.rxoktotal,
+		   priv->stats.rxstaterr,
+		   priv->stats.rxurberr);
+
+	return 0;
+}
+
+void rtl8192_proc_module_init(void)
+{
+	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
+	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
+}
+
+void rtl8192_proc_init_one(struct net_device *dev)
+{
+	struct proc_dir_entry *dir;
+
+	if (!rtl8192_proc)
+		return;
+
+	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
+	if (!dir)
+		return;
+
+	proc_create_single("stats-rx", S_IFREG | 0444, dir,
+			   proc_get_stats_rx);
+	proc_create_single("stats-tx", S_IFREG | 0444, dir,
+			   proc_get_stats_tx);
+	proc_create_single("stats-ap", S_IFREG | 0444, dir,
+			   proc_get_stats_ap);
+	proc_create_single("registers", S_IFREG | 0444, dir,
+			   proc_get_registers);
+}
+
+void rtl8192_proc_remove_one(struct net_device *dev)
+{
+	remove_proc_subtree(dev->name, rtl8192_proc);
+}
+
-- 
2.25.1

Re: [PATCH v3 1/3] staging: rtl8192u: move debug stuff to its own file

[Philipp Hortmann]

On 7/29/22 05:52, Tong Zhang wrote:
> This is to prepare for moving them to debugfs and fix rmmod warn issue
> when wlan0 is renamed to something else.
> 
> Reviewed-by: Dan Carpenter<dan.carpenter@oracle.com>
> Signed-off-by: Tong Zhang<ztong0001@gmail.com>
> ---

When I applied your first patch I got this hint:

Applying: staging: rtl8192u: move debug stuff to its own file
.git/rebase-apply/patch:399: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

Not really an functional issue...but

By Philipp

Re: [PATCH v3 1/3] staging: rtl8192u: move debug stuff to its own file

[Tong Zhang]

On Thu, Jul 28, 2022 at 11:23 PM Philipp Hortmann
<philipp.g.hortmann@gmail.com> wrote:
>
> On 7/29/22 05:52, Tong Zhang wrote:
> > This is to prepare for moving them to debugfs and fix rmmod warn issue
> > when wlan0 is renamed to something else.
> >
> > Reviewed-by: Dan Carpenter<dan.carpenter@oracle.com>
> > Signed-off-by: Tong Zhang<ztong0001@gmail.com>
> > ---
>
> When I applied your first patch I got this hint:
>
> Applying: staging: rtl8192u: move debug stuff to its own file
> .git/rebase-apply/patch:399: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
> Not really an functional issue...but
>
> By Philipp

Hi Philipp,
Thanks for the review. I fixed it in v4.
Thanks and have a good one!
- Tong

[PATCH v3 2/3] staging: rtl8192u: move debug files to debugfs

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname].
Due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.
The directory structure will be like the following

  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-ap
  /sys/kernel/debug/r8192u_usb/wlan0/registers

This is also to prepare for address rmmod warn issue.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile             |  2 +-
 drivers/staging/rtl8192u/r8192U.h             | 10 ++-
 drivers/staging/rtl8192u/r8192U_core.c        | 21 +++---
 .../{r8192U_procfs.c => r8192U_debugfs.c}     | 65 ++++++++++---------
 4 files changed, 54 insertions(+), 44 deletions(-)
 rename drivers/staging/rtl8192u/{r8192U_procfs.c => r8192U_debugfs.c} (69%)

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 5aef46cc90ef..d32dfd89a606 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,7 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
-		  r8192U_procfs.o					\
+		  r8192U_debugfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index e8b6da2adc4d..920a6a154860 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
 	struct delayed_work gpio_change_rf_wq;
 	struct delayed_work initialgain_operate_wq;
 	struct workqueue_struct *priv_wq;
+
+	/* debugfs */
+	struct dentry *debugfs_dir;
 } r8192_priv;
 
 /* For rtl8187B */
@@ -1117,8 +1120,9 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
-void rtl8192_proc_module_init(void);
-void rtl8192_proc_init_one(struct net_device *dev);
-void rtl8192_proc_remove_one(struct net_device *dev);
+void rtl8192_debugfs_init_one(struct net_device *dev);
+void rtl8192_debugfs_exit_one(struct net_device *dev);
+void rtl8192_debugfs_init(void);
+void rtl8192_debugfs_exit(void);
 
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 9e0861fdc64e..865bc0dc7c71 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -56,7 +56,6 @@ double __extendsfdf2(float a)
 #include "r8192U_dm.h"
 #include <linux/usb.h>
 #include <linux/slab.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 /* FIXME: check if 2.6.7 is ok */
 
@@ -4557,7 +4556,7 @@ static int rtl8192_usb_probe(struct usb_interface *intf,
 		goto fail2;
 
 	RT_TRACE(COMP_INIT, "dev name=======> %s\n", dev->name);
-	rtl8192_proc_init_one(dev);
+	rtl8192_debugfs_init_one(dev);
 
 	RT_TRACE(COMP_INIT, "Driver probe completed\n");
 	return 0;
@@ -4591,10 +4590,11 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	struct net_device *dev = usb_get_intfdata(intf);
 	struct r8192_priv *priv = ieee80211_priv(dev);
 
-	unregister_netdev(dev);
 
 	RT_TRACE(COMP_DOWN, "=============>wlan driver to be removed\n");
-	rtl8192_proc_remove_one(dev);
+	rtl8192_debugfs_exit_one(dev);
+
+	unregister_netdev(dev);
 
 	rtl8192_down(dev);
 	kfree(priv->pFirmware);
@@ -4615,10 +4615,11 @@ static int __init rtl8192_usb_module_init(void)
 	RT_TRACE(COMP_INIT, "Initializing module");
 	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
 
+	rtl8192_debugfs_init();
 	ret = ieee80211_debug_init();
 	if (ret) {
 		pr_err("ieee80211_debug_init() failed %d\n", ret);
-		return ret;
+		goto debugfs_exit;
 	}
 
 	ret = ieee80211_crypto_init();
@@ -4645,14 +4646,12 @@ static int __init rtl8192_usb_module_init(void)
 		goto crypto_ccmp_exit;
 	}
 
-	rtl8192_proc_module_init();
 	ret = usb_register(&rtl8192_usb_driver);
 	if (ret)
-		goto rtl8192_proc_module_exit;
+		goto crypto_wep_exit;
 	return ret;
 
-rtl8192_proc_module_exit:
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
+crypto_wep_exit:
 	ieee80211_crypto_wep_exit();
 crypto_ccmp_exit:
 	ieee80211_crypto_ccmp_exit();
@@ -4662,18 +4661,20 @@ static int __init rtl8192_usb_module_init(void)
 	ieee80211_crypto_deinit();
 debug_exit:
 	ieee80211_debug_exit();
+debugfs_exit:
+	rtl8192_debugfs_exit();
 	return ret;
 }
 
 static void __exit rtl8192_usb_module_exit(void)
 {
 	usb_deregister(&rtl8192_usb_driver);
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
 	ieee80211_crypto_wep_exit();
 	ieee80211_crypto_ccmp_exit();
 	ieee80211_crypto_tkip_exit();
 	ieee80211_crypto_deinit();
 	ieee80211_debug_exit();
+	rtl8192_debugfs_exit();
 	RT_TRACE(COMP_DOWN, "Exiting");
 }
 
diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
similarity index 69%
rename from drivers/staging/rtl8192u/r8192U_procfs.c
rename to drivers/staging/rtl8192u/r8192U_debugfs.c
index cc69d78d5152..79cd095114ca 100644
--- a/drivers/staging/rtl8192u/r8192U_procfs.c
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -1,13 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0
 /****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
+ *   -----------------------------DEGUGFS STUFF-------------------------
  ****************************************************************************/
-#include <linux/proc_fs.h>
+#include <linux/debugfs.h>
 #include <linux/seq_file.h>
 #include "r8192U.h"
 
-static struct proc_dir_entry *rtl8192_proc;
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
+#define R8192U_DEBUGFS_DIR_NAME "r8192u_usb"
+
+static int rtl8192_usb_stats_ap_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
@@ -26,7 +27,7 @@ static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
+static int rtl8192_usb_registers_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	int i, n, max = 0xff;
@@ -67,7 +68,7 @@ static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_tx_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
@@ -126,7 +127,7 @@ static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_rx_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
@@ -142,35 +143,39 @@ static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
 	return 0;
 }
 
-void rtl8192_proc_module_init(void)
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_rx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_tx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_ap);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_registers);
+
+void rtl8192_debugfs_init_one(struct net_device *dev)
+{
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct dentry *parent_dir = debugfs_lookup(R8192U_DEBUGFS_DIR_NAME, NULL);
+	struct dentry *dir = debugfs_create_dir(dev->name, parent_dir);
+
+	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
+	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
+	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
+	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
+
+	priv->debugfs_dir = dir;
+}
+
+void rtl8192_debugfs_exit_one(struct net_device *dev)
 {
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	debugfs_remove_recursive(priv->debugfs_dir);
 }
 
-void rtl8192_proc_init_one(struct net_device *dev)
+void rtl8192_debugfs_init(void)
 {
-	struct proc_dir_entry *dir;
-
-	if (!rtl8192_proc)
-		return;
-
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
-
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
+	debugfs_create_dir(R8192U_DEBUGFS_DIR_NAME, NULL);
 }
 
-void rtl8192_proc_remove_one(struct net_device *dev)
+void rtl8192_debugfs_exit(void)
 {
-	remove_proc_subtree(dev->name, rtl8192_proc);
+	debugfs_remove_recursive(debugfs_lookup(R8192U_DEBUGFS_DIR_NAME, NULL));
 }
 
-- 
2.25.1

Re: [PATCH v3 2/3] staging: rtl8192u: move debug files to debugfs

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 08:52:19PM -0700, Tong Zhang wrote:
>  
> -static struct proc_dir_entry *rtl8192_proc;
> -static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
> +#define R8192U_DEBUGFS_DIR_NAME "r8192u_usb"

KBUILD_MODNAME is a better thing to use here.

> +void rtl8192_debugfs_init_one(struct net_device *dev)
> +{
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);

No need to cast.

> +	struct dentry *parent_dir = debugfs_lookup(R8192U_DEBUGFS_DIR_NAME, NULL);
> +	struct dentry *dir = debugfs_create_dir(dev->name, parent_dir);
> +
> +	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> +	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> +	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> +	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> +
> +	priv->debugfs_dir = dir;
> +}
> +
> +void rtl8192_debugfs_exit_one(struct net_device *dev)
>  {
> -	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
> -	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);

No need for a cast.

thanks,

greg k-h

[PATCH v3 3/3] staging: rtl8192u: fix rmmod warn when device is renamed

[Tong Zhang]

This driver creates 4 debug files under [devname] folder. The devname
could be wlan0 initially, however it could be renamed later to e.g.
enx00e04c00000. This will cause problem during debug file teardown since
it uses netdev->name, which is no longer wlan0. To solve this problem,
add a notifier to handle device renaming. Also note that we cannot
simply do debugfs_lookup to find out old dentry since by the time the
notifier is called, netdev->name is already changed to new name.

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Tested-by: Zheyu Ma <zheyuma97@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/r8192U.h         |  1 +
 drivers/staging/rtl8192u/r8192U_core.c    | 32 +++++++++++++++++++++++
 drivers/staging/rtl8192u/r8192U_debugfs.c | 12 +++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 920a6a154860..420ab696fc3e 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1122,6 +1122,7 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 
 void rtl8192_debugfs_init_one(struct net_device *dev);
 void rtl8192_debugfs_exit_one(struct net_device *dev);
+void rtl8192_debugfs_rename_one(struct net_device *dev);
 void rtl8192_debugfs_init(void);
 void rtl8192_debugfs_exit(void);
 
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 865bc0dc7c71..0a60ef20107c 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4606,6 +4606,30 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	RT_TRACE(COMP_DOWN, "wlan driver removed\n");
 }
 
+static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
+				    void *data)
+{
+	struct net_device *netdev = netdev_notifier_info_to_dev(data);
+
+	if (netdev->netdev_ops != &rtl8192_netdev_ops)
+		goto out;
+
+	switch (event) {
+	case NETDEV_CHANGENAME:
+		rtl8192_debugfs_rename_one(netdev);
+		break;
+	default:
+		break;
+	}
+
+out:
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block rtl8192_usb_netdev_notifier = {
+	.notifier_call = rtl8192_usb_netdev_event,
+};
+
 static int __init rtl8192_usb_module_init(void)
 {
 	int ret;
@@ -4615,6 +4639,12 @@ static int __init rtl8192_usb_module_init(void)
 	RT_TRACE(COMP_INIT, "Initializing module");
 	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
 
+	ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
+	if (ret) {
+		pr_err("register_netdevice_notifier failed %d\n", ret);
+		return ret;
+	}
+
 	rtl8192_debugfs_init();
 	ret = ieee80211_debug_init();
 	if (ret) {
@@ -4663,6 +4693,7 @@ static int __init rtl8192_usb_module_init(void)
 	ieee80211_debug_exit();
 debugfs_exit:
 	rtl8192_debugfs_exit();
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	return ret;
 }
 
@@ -4675,6 +4706,7 @@ static void __exit rtl8192_usb_module_exit(void)
 	ieee80211_crypto_deinit();
 	ieee80211_debug_exit();
 	rtl8192_debugfs_exit();
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	RT_TRACE(COMP_DOWN, "Exiting");
 }
 
diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
index 79cd095114ca..e80cfe24facf 100644
--- a/drivers/staging/rtl8192u/r8192U_debugfs.c
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -150,7 +150,7 @@ DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_registers);
 
 void rtl8192_debugfs_init_one(struct net_device *dev)
 {
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct r8192_priv *priv = ieee80211_priv(dev);
 	struct dentry *parent_dir = debugfs_lookup(R8192U_DEBUGFS_DIR_NAME, NULL);
 	struct dentry *dir = debugfs_create_dir(dev->name, parent_dir);
 
@@ -164,11 +164,19 @@ void rtl8192_debugfs_init_one(struct net_device *dev)
 
 void rtl8192_debugfs_exit_one(struct net_device *dev)
 {
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct r8192_priv *priv = ieee80211_priv(dev);
 
 	debugfs_remove_recursive(priv->debugfs_dir);
 }
 
+void rtl8192_debugfs_rename_one(struct net_device *dev)
+{
+	struct r8192_priv *priv = ieee80211_priv(dev);
+	struct dentry *parent_dir = debugfs_lookup(R8192U_DEBUGFS_DIR_NAME, NULL);
+
+	debugfs_rename(parent_dir, priv->debugfs_dir, parent_dir, dev->name);
+}
+
 void rtl8192_debugfs_init(void)
 {
 	debugfs_create_dir(R8192U_DEBUGFS_DIR_NAME, NULL);
-- 
2.25.1

Re: [PATCH v3 3/3] staging: rtl8192u: fix rmmod warn when device is renamed

[Greg Kroah-Hartman]

On Thu, Jul 28, 2022 at 08:52:20PM -0700, Tong Zhang wrote:
> @@ -164,11 +164,19 @@ void rtl8192_debugfs_init_one(struct net_device *dev)
>  
>  void rtl8192_debugfs_exit_one(struct net_device *dev)
>  {
> -	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> +	struct r8192_priv *priv = ieee80211_priv(dev);

This change is not relevant for this patch :(

Re: [PATCH v3 3/3] staging: rtl8192u: fix rmmod warn when device is renamed

[Tong Zhang]

On Fri, Jul 29, 2022 at 12:27 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 28, 2022 at 08:52:20PM -0700, Tong Zhang wrote:
> > @@ -164,11 +164,19 @@ void rtl8192_debugfs_init_one(struct net_device *dev)
> >
> >  void rtl8192_debugfs_exit_one(struct net_device *dev)
> >  {
> > -     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +     struct r8192_priv *priv = ieee80211_priv(dev);
>
> This change is not relevant for this patch :(
>
Oh sorry sorry, my bad, I will move them to a separate commit.

[PATCH v4 0/4] staging: rtl8192u: fix rmmod warn when device is renamed

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname] by
rtl8192u_usb. Devname could be wlan0 initially, however it could be
renamed later to e.g. enx00e04c000002. This will cause problem during
debug file teardown since it uses netdev->name which is no longer wlan0.
To solve this problem, add a notifier to handle device renaming.

Also, due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.

The directory structure after this patch set will be like the following

  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-ap
  /sys/kernel/debug/r8192u_usb/wlan0/registers

Also note that we cannot simply do debugfs_lookup to find out old dentry
since by the time the notifier is called, netdev->name is already changed
to new name. So here we still save the original dentry.


Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Tested-by: Zheyu Ma <zheyuma97@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>

v2: break down patch and fix pointer check
v3: removed unnecessary checks, casts and move debug files under module's
own directory, only minor change compared to v2
v4: move cast fix to one commit. use KBUILD_MODNAME for debugfs dir name

Tong Zhang (4):
  staging: rtl8192u: move debug stuff to its own file
  staging: rtl8192u: remove unnecessary cast
  staging: rtl8192u: move debug files to debugfs
  staging: rtl8192u: fix rmmod warn when device is renamed

 drivers/staging/rtl8192u/Makefile         |   1 +
 drivers/staging/rtl8192u/r8192U.h         |   9 +
 drivers/staging/rtl8192u/r8192U_core.c    | 226 ++++------------------
 drivers/staging/rtl8192u/r8192U_debugfs.c | 188 ++++++++++++++++++
 4 files changed, 241 insertions(+), 183 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_debugfs.c

-- 
2.25.1

[PATCH v4 1/4] staging: rtl8192u: move debug stuff to its own file

[Tong Zhang]

This is to prepare for moving them to debugfs and fix rmmod warn issue
when wlan0 is renamed to something else.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile        |   1 +
 drivers/staging/rtl8192u/r8192U.h        |   4 +
 drivers/staging/rtl8192u/r8192U_core.c   | 173 ----------------------
 drivers/staging/rtl8192u/r8192U_procfs.c | 175 +++++++++++++++++++++++
 4 files changed, 180 insertions(+), 173 deletions(-)
 create mode 100644 drivers/staging/rtl8192u/r8192U_procfs.c

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 0be7426b6ebc..5aef46cc90ef 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,6 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
+		  r8192U_procfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 14ca00a2789b..e8b6da2adc4d 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1117,4 +1117,8 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
+void rtl8192_proc_module_init(void);
+void rtl8192_proc_init_one(struct net_device *dev);
+void rtl8192_proc_remove_one(struct net_device *dev);
+
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 2ca925f35830..9e0861fdc64e 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -452,179 +452,6 @@ static struct net_device_stats *rtl8192_stats(struct net_device *dev);
 static void rtl8192_restart(struct work_struct *work);
 static void watch_dog_timer_callback(struct timer_list *t);
 
-/****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
- ****************************************************************************/
-
-static struct proc_dir_entry *rtl8192_proc;
-
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-	struct ieee80211_device *ieee = priv->ieee80211;
-	struct ieee80211_network *target;
-
-	list_for_each_entry(target, &ieee->network_list, list) {
-		const char *wpa = "non_WPA";
-
-		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
-			wpa = "WPA";
-
-		seq_printf(m, "%s %s\n", target->ssid, wpa);
-	}
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	int i, n, max = 0xff;
-	u8 byte_rd;
-
-	seq_puts(m, "\n####################page 0##################\n ");
-
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x000 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 1##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x100 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_puts(m, "\n####################page 3##################\n ");
-	for (n = 0; n <= max;) {
-		seq_printf(m, "\nD:  %2x > ", n);
-
-		for (i = 0; i < 16 && n <= max; i++, n++) {
-			read_nic_byte(dev, 0x300 | n, &byte_rd);
-			seq_printf(m, "%2x ", byte_rd);
-		}
-	}
-
-	seq_putc(m, '\n');
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "TX VI priority ok int: %lu\n"
-		   "TX VI priority error int: %lu\n"
-		   "TX VO priority ok int: %lu\n"
-		   "TX VO priority error int: %lu\n"
-		   "TX BE priority ok int: %lu\n"
-		   "TX BE priority error int: %lu\n"
-		   "TX BK priority ok int: %lu\n"
-		   "TX BK priority error int: %lu\n"
-		   "TX MANAGE priority ok int: %lu\n"
-		   "TX MANAGE priority error int: %lu\n"
-		   "TX BEACON priority ok int: %lu\n"
-		   "TX BEACON priority error int: %lu\n"
-		   "TX queue resume: %lu\n"
-		   "TX queue stopped?: %d\n"
-		   "TX fifo overflow: %lu\n"
-		   "TX VI queue: %d\n"
-		   "TX VO queue: %d\n"
-		   "TX BE queue: %d\n"
-		   "TX BK queue: %d\n"
-		   "TX VI dropped: %lu\n"
-		   "TX VO dropped: %lu\n"
-		   "TX BE dropped: %lu\n"
-		   "TX BK dropped: %lu\n"
-		   "TX total data packets %lu\n",
-		   priv->stats.txviokint,
-		   priv->stats.txvierr,
-		   priv->stats.txvookint,
-		   priv->stats.txvoerr,
-		   priv->stats.txbeokint,
-		   priv->stats.txbeerr,
-		   priv->stats.txbkokint,
-		   priv->stats.txbkerr,
-		   priv->stats.txmanageokint,
-		   priv->stats.txmanageerr,
-		   priv->stats.txbeaconokint,
-		   priv->stats.txbeaconerr,
-		   priv->stats.txresumed,
-		   netif_queue_stopped(dev),
-		   priv->stats.txoverflow,
-		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
-		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
-		   priv->stats.txvidrop,
-		   priv->stats.txvodrop,
-		   priv->stats.txbedrop,
-		   priv->stats.txbkdrop,
-		   priv->stats.txdatapkt
-		);
-
-	return 0;
-}
-
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
-{
-	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
-
-	seq_printf(m,
-		   "RX packets: %lu\n"
-		   "RX urb status error: %lu\n"
-		   "RX invalid urb error: %lu\n",
-		   priv->stats.rxoktotal,
-		   priv->stats.rxstaterr,
-		   priv->stats.rxurberr);
-
-	return 0;
-}
-
-static void rtl8192_proc_module_init(void)
-{
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
-}
-
-static void rtl8192_proc_init_one(struct net_device *dev)
-{
-	struct proc_dir_entry *dir;
-
-	if (!rtl8192_proc)
-		return;
-
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
-
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
-}
-
-static void rtl8192_proc_remove_one(struct net_device *dev)
-{
-	remove_proc_subtree(dev->name, rtl8192_proc);
-}
-
 /****************************************************************************
  *  -----------------------------MISC STUFF-------------------------
  *****************************************************************************/
diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_procfs.c
new file mode 100644
index 000000000000..69cbafceecfe
--- /dev/null
+++ b/drivers/staging/rtl8192u/r8192U_procfs.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/****************************************************************************
+ *   -----------------------------PROCFS STUFF-------------------------
+ ****************************************************************************/
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include "r8192U.h"
+
+static struct proc_dir_entry *rtl8192_proc;
+static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct ieee80211_device *ieee = priv->ieee80211;
+	struct ieee80211_network *target;
+
+	list_for_each_entry(target, &ieee->network_list, list) {
+		const char *wpa = "non_WPA";
+
+		if (target->wpa_ie_len > 0 || target->rsn_ie_len > 0)
+			wpa = "WPA";
+
+		seq_printf(m, "%s %s\n", target->ssid, wpa);
+	}
+
+	return 0;
+}
+
+static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	int i, n, max = 0xff;
+	u8 byte_rd;
+
+	seq_puts(m, "\n####################page 0##################\n ");
+
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x000 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 1##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x100 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_puts(m, "\n####################page 3##################\n ");
+	for (n = 0; n <= max;) {
+		seq_printf(m, "\nD:  %2x > ", n);
+
+		for (i = 0; i < 16 && n <= max; i++, n++) {
+			read_nic_byte(dev, 0x300 | n, &byte_rd);
+			seq_printf(m, "%2x ", byte_rd);
+		}
+	}
+
+	seq_putc(m, '\n');
+	return 0;
+}
+
+static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+		   "TX VI priority ok int: %lu\n"
+		   "TX VI priority error int: %lu\n"
+		   "TX VO priority ok int: %lu\n"
+		   "TX VO priority error int: %lu\n"
+		   "TX BE priority ok int: %lu\n"
+		   "TX BE priority error int: %lu\n"
+		   "TX BK priority ok int: %lu\n"
+		   "TX BK priority error int: %lu\n"
+		   "TX MANAGE priority ok int: %lu\n"
+		   "TX MANAGE priority error int: %lu\n"
+		   "TX BEACON priority ok int: %lu\n"
+		   "TX BEACON priority error int: %lu\n"
+		   "TX queue resume: %lu\n"
+		   "TX queue stopped?: %d\n"
+		   "TX fifo overflow: %lu\n"
+		   "TX VI queue: %d\n"
+		   "TX VO queue: %d\n"
+		   "TX BE queue: %d\n"
+		   "TX BK queue: %d\n"
+		   "TX VI dropped: %lu\n"
+		   "TX VO dropped: %lu\n"
+		   "TX BE dropped: %lu\n"
+		   "TX BK dropped: %lu\n"
+		   "TX total data packets %lu\n",
+		   priv->stats.txviokint,
+		   priv->stats.txvierr,
+		   priv->stats.txvookint,
+		   priv->stats.txvoerr,
+		   priv->stats.txbeokint,
+		   priv->stats.txbeerr,
+		   priv->stats.txbkokint,
+		   priv->stats.txbkerr,
+		   priv->stats.txmanageokint,
+		   priv->stats.txmanageerr,
+		   priv->stats.txbeaconokint,
+		   priv->stats.txbeaconerr,
+		   priv->stats.txresumed,
+		   netif_queue_stopped(dev),
+		   priv->stats.txoverflow,
+		   atomic_read(&(priv->tx_pending[VI_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[VO_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[BE_PRIORITY])),
+		   atomic_read(&(priv->tx_pending[BK_PRIORITY])),
+		   priv->stats.txvidrop,
+		   priv->stats.txvodrop,
+		   priv->stats.txbedrop,
+		   priv->stats.txbkdrop,
+		   priv->stats.txdatapkt
+		);
+
+	return 0;
+}
+
+static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
+{
+	struct net_device *dev = m->private;
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	seq_printf(m,
+		   "RX packets: %lu\n"
+		   "RX urb status error: %lu\n"
+		   "RX invalid urb error: %lu\n",
+		   priv->stats.rxoktotal,
+		   priv->stats.rxstaterr,
+		   priv->stats.rxurberr);
+
+	return 0;
+}
+
+void rtl8192_proc_module_init(void)
+{
+	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
+	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
+}
+
+void rtl8192_proc_init_one(struct net_device *dev)
+{
+	struct proc_dir_entry *dir;
+
+	if (!rtl8192_proc)
+		return;
+
+	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
+	if (!dir)
+		return;
+
+	proc_create_single("stats-rx", S_IFREG | 0444, dir,
+			   proc_get_stats_rx);
+	proc_create_single("stats-tx", S_IFREG | 0444, dir,
+			   proc_get_stats_tx);
+	proc_create_single("stats-ap", S_IFREG | 0444, dir,
+			   proc_get_stats_ap);
+	proc_create_single("registers", S_IFREG | 0444, dir,
+			   proc_get_registers);
+}
+
+void rtl8192_proc_remove_one(struct net_device *dev)
+{
+	remove_proc_subtree(dev->name, rtl8192_proc);
+}
-- 
2.25.1

[PATCH v4 2/4] staging: rtl8192u: remove unnecessary cast

[Tong Zhang]

Cast is not needed when calling ieee80211_priv, so remove them.
No functional change in this commit.

Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_procfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_procfs.c
index 69cbafceecfe..d6f8401526c5 100644
--- a/drivers/staging/rtl8192u/r8192U_procfs.c
+++ b/drivers/staging/rtl8192u/r8192U_procfs.c
@@ -10,7 +10,7 @@ static struct proc_dir_entry *rtl8192_proc;
 static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct r8192_priv *priv = ieee80211_priv(dev);
 	struct ieee80211_device *ieee = priv->ieee80211;
 	struct ieee80211_network *target;
 
@@ -70,7 +70,7 @@ static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
 static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct r8192_priv *priv = ieee80211_priv(dev);
 
 	seq_printf(m,
 		   "TX VI priority ok int: %lu\n"
@@ -129,7 +129,7 @@ static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
 static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
-	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+	struct r8192_priv *priv = ieee80211_priv(dev);
 
 	seq_printf(m,
 		   "RX packets: %lu\n"
-- 
2.25.1

[PATCH v4 3/4] staging: rtl8192u: move debug files to debugfs

[Tong Zhang]

There are 4 debug files created under /proc/net/[Devname].
Due to this is purely for debuging as files are created read only,
move this to debugfs like other NIC drivers do instead of using procfs.
The directory structure will be like the following

  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-rx
  /sys/kernel/debug/r8192u_usb/wlan0/stats-ap
  /sys/kernel/debug/r8192u_usb/wlan0/registers

This is also to prepare for address rmmod warn issue.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/Makefile             |  2 +-
 drivers/staging/rtl8192u/r8192U.h             | 10 ++-
 drivers/staging/rtl8192u/r8192U_core.c        | 21 +++---
 .../{r8192U_procfs.c => r8192U_debugfs.c}     | 65 ++++++++++---------
 4 files changed, 54 insertions(+), 44 deletions(-)
 rename drivers/staging/rtl8192u/{r8192U_procfs.c => r8192U_debugfs.c} (70%)

diff --git a/drivers/staging/rtl8192u/Makefile b/drivers/staging/rtl8192u/Makefile
index 5aef46cc90ef..d32dfd89a606 100644
--- a/drivers/staging/rtl8192u/Makefile
+++ b/drivers/staging/rtl8192u/Makefile
@@ -8,7 +8,7 @@ ccflags-y += -DTHOMAS_BEACON -DTHOMAS_TASKLET -DTHOMAS_SKB -DTHOMAS_TURBO
 r8192u_usb-y := r8192U_core.o r8180_93cx6.o r8192U_wx.o		\
 		  r8190_rtl8256.o r819xU_phy.o r819xU_firmware.o	\
 		  r819xU_cmdpkt.o r8192U_dm.o r819xU_firmware_img.o	\
-		  r8192U_procfs.o					\
+		  r8192U_debugfs.o					\
 		  ieee80211/ieee80211_crypt.o				\
 		  ieee80211/ieee80211_crypt_tkip.o			\
 		  ieee80211/ieee80211_crypt_ccmp.o			\
diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index e8b6da2adc4d..920a6a154860 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1061,6 +1061,9 @@ typedef struct r8192_priv {
 	struct delayed_work gpio_change_rf_wq;
 	struct delayed_work initialgain_operate_wq;
 	struct workqueue_struct *priv_wq;
+
+	/* debugfs */
+	struct dentry *debugfs_dir;
 } r8192_priv;
 
 /* For rtl8187B */
@@ -1117,8 +1120,9 @@ void EnableHWSecurityConfig8192(struct net_device *dev);
 void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 	    const u8 *MacAddr, u8 DefaultKey, u32 *KeyContent);
 
-void rtl8192_proc_module_init(void);
-void rtl8192_proc_init_one(struct net_device *dev);
-void rtl8192_proc_remove_one(struct net_device *dev);
+void rtl8192_debugfs_init_one(struct net_device *dev);
+void rtl8192_debugfs_exit_one(struct net_device *dev);
+void rtl8192_debugfs_init(void);
+void rtl8192_debugfs_exit(void);
 
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 9e0861fdc64e..865bc0dc7c71 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -56,7 +56,6 @@ double __extendsfdf2(float a)
 #include "r8192U_dm.h"
 #include <linux/usb.h>
 #include <linux/slab.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 /* FIXME: check if 2.6.7 is ok */
 
@@ -4557,7 +4556,7 @@ static int rtl8192_usb_probe(struct usb_interface *intf,
 		goto fail2;
 
 	RT_TRACE(COMP_INIT, "dev name=======> %s\n", dev->name);
-	rtl8192_proc_init_one(dev);
+	rtl8192_debugfs_init_one(dev);
 
 	RT_TRACE(COMP_INIT, "Driver probe completed\n");
 	return 0;
@@ -4591,10 +4590,11 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	struct net_device *dev = usb_get_intfdata(intf);
 	struct r8192_priv *priv = ieee80211_priv(dev);
 
-	unregister_netdev(dev);
 
 	RT_TRACE(COMP_DOWN, "=============>wlan driver to be removed\n");
-	rtl8192_proc_remove_one(dev);
+	rtl8192_debugfs_exit_one(dev);
+
+	unregister_netdev(dev);
 
 	rtl8192_down(dev);
 	kfree(priv->pFirmware);
@@ -4615,10 +4615,11 @@ static int __init rtl8192_usb_module_init(void)
 	RT_TRACE(COMP_INIT, "Initializing module");
 	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
 
+	rtl8192_debugfs_init();
 	ret = ieee80211_debug_init();
 	if (ret) {
 		pr_err("ieee80211_debug_init() failed %d\n", ret);
-		return ret;
+		goto debugfs_exit;
 	}
 
 	ret = ieee80211_crypto_init();
@@ -4645,14 +4646,12 @@ static int __init rtl8192_usb_module_init(void)
 		goto crypto_ccmp_exit;
 	}
 
-	rtl8192_proc_module_init();
 	ret = usb_register(&rtl8192_usb_driver);
 	if (ret)
-		goto rtl8192_proc_module_exit;
+		goto crypto_wep_exit;
 	return ret;
 
-rtl8192_proc_module_exit:
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
+crypto_wep_exit:
 	ieee80211_crypto_wep_exit();
 crypto_ccmp_exit:
 	ieee80211_crypto_ccmp_exit();
@@ -4662,18 +4661,20 @@ static int __init rtl8192_usb_module_init(void)
 	ieee80211_crypto_deinit();
 debug_exit:
 	ieee80211_debug_exit();
+debugfs_exit:
+	rtl8192_debugfs_exit();
 	return ret;
 }
 
 static void __exit rtl8192_usb_module_exit(void)
 {
 	usb_deregister(&rtl8192_usb_driver);
-	remove_proc_entry(RTL819XU_MODULE_NAME, init_net.proc_net);
 	ieee80211_crypto_wep_exit();
 	ieee80211_crypto_ccmp_exit();
 	ieee80211_crypto_tkip_exit();
 	ieee80211_crypto_deinit();
 	ieee80211_debug_exit();
+	rtl8192_debugfs_exit();
 	RT_TRACE(COMP_DOWN, "Exiting");
 }
 
diff --git a/drivers/staging/rtl8192u/r8192U_procfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
similarity index 70%
rename from drivers/staging/rtl8192u/r8192U_procfs.c
rename to drivers/staging/rtl8192u/r8192U_debugfs.c
index d6f8401526c5..c64504346657 100644
--- a/drivers/staging/rtl8192u/r8192U_procfs.c
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -1,13 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0
 /****************************************************************************
- *   -----------------------------PROCFS STUFF-------------------------
+ *   -----------------------------DEGUGFS STUFF-------------------------
  ****************************************************************************/
-#include <linux/proc_fs.h>
+#include <linux/debugfs.h>
 #include <linux/seq_file.h>
 #include "r8192U.h"
 
-static struct proc_dir_entry *rtl8192_proc;
-static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
+#define KBUILD_MODNAME "r8192u_usb"
+
+static int rtl8192_usb_stats_ap_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = ieee80211_priv(dev);
@@ -26,7 +27,7 @@ static int __maybe_unused proc_get_stats_ap(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
+static int rtl8192_usb_registers_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	int i, n, max = 0xff;
@@ -67,7 +68,7 @@ static int __maybe_unused proc_get_registers(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_tx_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = ieee80211_priv(dev);
@@ -126,7 +127,7 @@ static int __maybe_unused proc_get_stats_tx(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
+static int rtl8192_usb_stats_rx_show(struct seq_file *m, void *v)
 {
 	struct net_device *dev = m->private;
 	struct r8192_priv *priv = ieee80211_priv(dev);
@@ -142,34 +143,38 @@ static int __maybe_unused proc_get_stats_rx(struct seq_file *m, void *v)
 	return 0;
 }
 
-void rtl8192_proc_module_init(void)
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_rx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_tx);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_stats_ap);
+DEFINE_SHOW_ATTRIBUTE(rtl8192_usb_registers);
+
+void rtl8192_debugfs_init_one(struct net_device *dev)
+{
+	struct r8192_priv *priv = ieee80211_priv(dev);
+	struct dentry *parent_dir = debugfs_lookup(KBUILD_MODNAME, NULL);
+	struct dentry *dir = debugfs_create_dir(dev->name, parent_dir);
+
+	debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
+	debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
+	debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
+	debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
+
+	priv->debugfs_dir = dir;
+}
+
+void rtl8192_debugfs_exit_one(struct net_device *dev)
 {
-	RT_TRACE(COMP_INIT, "Initializing proc filesystem");
-	rtl8192_proc = proc_mkdir(RTL819XU_MODULE_NAME, init_net.proc_net);
+	struct r8192_priv *priv = ieee80211_priv(dev);
+
+	debugfs_remove_recursive(priv->debugfs_dir);
 }
 
-void rtl8192_proc_init_one(struct net_device *dev)
+void rtl8192_debugfs_init(void)
 {
-	struct proc_dir_entry *dir;
-
-	if (!rtl8192_proc)
-		return;
-
-	dir = proc_mkdir_data(dev->name, 0, rtl8192_proc, dev);
-	if (!dir)
-		return;
-
-	proc_create_single("stats-rx", S_IFREG | 0444, dir,
-			   proc_get_stats_rx);
-	proc_create_single("stats-tx", S_IFREG | 0444, dir,
-			   proc_get_stats_tx);
-	proc_create_single("stats-ap", S_IFREG | 0444, dir,
-			   proc_get_stats_ap);
-	proc_create_single("registers", S_IFREG | 0444, dir,
-			   proc_get_registers);
+	debugfs_create_dir(KBUILD_MODNAME, NULL);
 }
 
-void rtl8192_proc_remove_one(struct net_device *dev)
+void rtl8192_debugfs_exit(void)
 {
-	remove_proc_subtree(dev->name, rtl8192_proc);
+	debugfs_remove_recursive(debugfs_lookup(KBUILD_MODNAME, NULL));
 }
-- 
2.25.1

[PATCH v4 4/4] staging: rtl8192u: fix rmmod warn when device is renamed

[Tong Zhang]

This driver creates 4 debug files under [devname] folder. The devname
could be wlan0 initially, however it could be renamed later to e.g.
enx00e04c00000. This will cause problem during debug file teardown since
it uses netdev->name, which is no longer wlan0. To solve this problem,
add a notifier to handle device renaming. Also note that we cannot
simply do debugfs_lookup to find out old dentry since by the time the
notifier is called, netdev->name is already changed to new name.

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Tested-by: Zheyu Ma <zheyuma97@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/r8192U.h         |  1 +
 drivers/staging/rtl8192u/r8192U_core.c    | 32 +++++++++++++++++++++++
 drivers/staging/rtl8192u/r8192U_debugfs.c |  8 ++++++
 3 files changed, 41 insertions(+)

diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index 920a6a154860..420ab696fc3e 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1122,6 +1122,7 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 
 void rtl8192_debugfs_init_one(struct net_device *dev);
 void rtl8192_debugfs_exit_one(struct net_device *dev);
+void rtl8192_debugfs_rename_one(struct net_device *dev);
 void rtl8192_debugfs_init(void);
 void rtl8192_debugfs_exit(void);
 
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 865bc0dc7c71..0a60ef20107c 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4606,6 +4606,30 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	RT_TRACE(COMP_DOWN, "wlan driver removed\n");
 }
 
+static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
+				    void *data)
+{
+	struct net_device *netdev = netdev_notifier_info_to_dev(data);
+
+	if (netdev->netdev_ops != &rtl8192_netdev_ops)
+		goto out;
+
+	switch (event) {
+	case NETDEV_CHANGENAME:
+		rtl8192_debugfs_rename_one(netdev);
+		break;
+	default:
+		break;
+	}
+
+out:
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block rtl8192_usb_netdev_notifier = {
+	.notifier_call = rtl8192_usb_netdev_event,
+};
+
 static int __init rtl8192_usb_module_init(void)
 {
 	int ret;
@@ -4615,6 +4639,12 @@ static int __init rtl8192_usb_module_init(void)
 	RT_TRACE(COMP_INIT, "Initializing module");
 	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
 
+	ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
+	if (ret) {
+		pr_err("register_netdevice_notifier failed %d\n", ret);
+		return ret;
+	}
+
 	rtl8192_debugfs_init();
 	ret = ieee80211_debug_init();
 	if (ret) {
@@ -4663,6 +4693,7 @@ static int __init rtl8192_usb_module_init(void)
 	ieee80211_debug_exit();
 debugfs_exit:
 	rtl8192_debugfs_exit();
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	return ret;
 }
 
@@ -4675,6 +4706,7 @@ static void __exit rtl8192_usb_module_exit(void)
 	ieee80211_crypto_deinit();
 	ieee80211_debug_exit();
 	rtl8192_debugfs_exit();
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	RT_TRACE(COMP_DOWN, "Exiting");
 }
 
diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
index c64504346657..fe8ef72506ee 100644
--- a/drivers/staging/rtl8192u/r8192U_debugfs.c
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -169,6 +169,14 @@ void rtl8192_debugfs_exit_one(struct net_device *dev)
 	debugfs_remove_recursive(priv->debugfs_dir);
 }
 
+void rtl8192_debugfs_rename_one(struct net_device *dev)
+{
+	struct r8192_priv *priv = ieee80211_priv(dev);
+	struct dentry *parent_dir = debugfs_lookup(KBUILD_MODNAME, NULL);
+
+	debugfs_rename(parent_dir, priv->debugfs_dir, parent_dir, dev->name);
+}
+
 void rtl8192_debugfs_init(void)
 {
 	debugfs_create_dir(KBUILD_MODNAME, NULL);
-- 
2.25.1

[PATCH v2 3/3] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Tong Zhang]

This driver creates 4 debug files under [devname] folder. The devname
could be wlan0 initially, however it could be renamed later to e.g.
enx00e04c00000. This will cause problem during debug file teardown since
it uses  netdev->name which is no longer wlan0. To solve this problem,
add a notifier to handle device renaming.

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Tested-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/staging/rtl8192u/r8192U.h         |  1 +
 drivers/staging/rtl8192u/r8192U_core.c    | 35 ++++++++++++++++++++++-
 drivers/staging/rtl8192u/r8192U_debugfs.c | 13 +++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
index e8860bb2b607..ccce37b7e2ae 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -1122,4 +1122,5 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
 
 void rtl8192_debugfs_init(struct net_device *dev);
 void rtl8192_debugfs_exit(struct net_device *dev);
+void rtl8192_debugfs_rename(struct net_device *dev);
 #endif
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index ac3716550505..5cc78c5bd706 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4606,6 +4606,30 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
 	RT_TRACE(COMP_DOWN, "wlan driver removed\n");
 }
 
+static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
+				    void *data)
+{
+	struct net_device *netdev = netdev_notifier_info_to_dev(data);
+
+	if (netdev->netdev_ops != &rtl8192_netdev_ops)
+		goto out;
+
+	switch (event) {
+	case NETDEV_CHANGENAME:
+		rtl8192_debugfs_rename(netdev);
+		break;
+	default:
+		break;
+	}
+
+out:
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block rtl8192_usb_netdev_notifier = {
+	.notifier_call = rtl8192_usb_netdev_event,
+};
+
 static int __init rtl8192_usb_module_init(void)
 {
 	int ret;
@@ -4615,10 +4639,16 @@ static int __init rtl8192_usb_module_init(void)
 	RT_TRACE(COMP_INIT, "Initializing module");
 	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
 
+	ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
+	if (ret) {
+		pr_err("register_netdevice_notifier failed %d\n", ret);
+		return ret;
+	}
+
 	ret = ieee80211_debug_init();
 	if (ret) {
 		pr_err("ieee80211_debug_init() failed %d\n", ret);
-		return ret;
+		goto unregister_notifier;
 	}
 
 	ret = ieee80211_crypto_init();
@@ -4660,6 +4690,8 @@ static int __init rtl8192_usb_module_init(void)
 	ieee80211_crypto_deinit();
 debug_exit:
 	ieee80211_debug_exit();
+unregister_notifier:
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	return ret;
 }
 
@@ -4671,6 +4703,7 @@ static void __exit rtl8192_usb_module_exit(void)
 	ieee80211_crypto_tkip_exit();
 	ieee80211_crypto_deinit();
 	ieee80211_debug_exit();
+	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
 	RT_TRACE(COMP_DOWN, "Exiting");
 }
 
diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
index 5c9376e50889..c94f5dfac96b 100644
--- a/drivers/staging/rtl8192u/r8192U_debugfs.c
+++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
@@ -173,3 +173,16 @@ void rtl8192_debugfs_exit(struct net_device *dev)
 	priv->debugfs_dir = NULL;
 }
 
+void rtl8192_debugfs_rename(struct net_device *dev)
+{
+	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
+
+	if (!priv->debugfs_dir)
+		return;
+
+	if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
+		return;
+
+	debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
+		       priv->debugfs_dir->d_parent, dev->name);
+}
-- 
2.25.1

Re: [PATCH v2 3/3] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Greg Kroah-Hartman]

On Mon, Jul 18, 2022 at 10:50:38PM -0700, Tong Zhang wrote:
> This driver creates 4 debug files under [devname] folder. The devname
> could be wlan0 initially, however it could be renamed later to e.g.
> enx00e04c00000. This will cause problem during debug file teardown since
> it uses  netdev->name which is no longer wlan0. To solve this problem,
> add a notifier to handle device renaming.
> 
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Tested-by: Zheyu Ma <zheyuma97@gmail.com>
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> ---
>  drivers/staging/rtl8192u/r8192U.h         |  1 +
>  drivers/staging/rtl8192u/r8192U_core.c    | 35 ++++++++++++++++++++++-
>  drivers/staging/rtl8192u/r8192U_debugfs.c | 13 +++++++++
>  3 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
> index e8860bb2b607..ccce37b7e2ae 100644
> --- a/drivers/staging/rtl8192u/r8192U.h
> +++ b/drivers/staging/rtl8192u/r8192U.h
> @@ -1122,4 +1122,5 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
>  
>  void rtl8192_debugfs_init(struct net_device *dev);
>  void rtl8192_debugfs_exit(struct net_device *dev);
> +void rtl8192_debugfs_rename(struct net_device *dev);
>  #endif
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index ac3716550505..5cc78c5bd706 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -4606,6 +4606,30 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
>  	RT_TRACE(COMP_DOWN, "wlan driver removed\n");
>  }
>  
> +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> +				    void *data)
> +{
> +	struct net_device *netdev = netdev_notifier_info_to_dev(data);
> +
> +	if (netdev->netdev_ops != &rtl8192_netdev_ops)
> +		goto out;
> +
> +	switch (event) {
> +	case NETDEV_CHANGENAME:
> +		rtl8192_debugfs_rename(netdev);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +out:
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rtl8192_usb_netdev_notifier = {
> +	.notifier_call = rtl8192_usb_netdev_event,
> +};
> +
>  static int __init rtl8192_usb_module_init(void)
>  {
>  	int ret;
> @@ -4615,10 +4639,16 @@ static int __init rtl8192_usb_module_init(void)
>  	RT_TRACE(COMP_INIT, "Initializing module");
>  	RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
>  
> +	ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> +	if (ret) {
> +		pr_err("register_netdevice_notifier failed %d\n", ret);
> +		return ret;
> +	}
> +
>  	ret = ieee80211_debug_init();
>  	if (ret) {
>  		pr_err("ieee80211_debug_init() failed %d\n", ret);
> -		return ret;
> +		goto unregister_notifier;
>  	}
>  
>  	ret = ieee80211_crypto_init();
> @@ -4660,6 +4690,8 @@ static int __init rtl8192_usb_module_init(void)
>  	ieee80211_crypto_deinit();
>  debug_exit:
>  	ieee80211_debug_exit();
> +unregister_notifier:
> +	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
>  	return ret;
>  }
>  
> @@ -4671,6 +4703,7 @@ static void __exit rtl8192_usb_module_exit(void)
>  	ieee80211_crypto_tkip_exit();
>  	ieee80211_crypto_deinit();
>  	ieee80211_debug_exit();
> +	unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
>  	RT_TRACE(COMP_DOWN, "Exiting");
>  }
>  
> diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
> index 5c9376e50889..c94f5dfac96b 100644
> --- a/drivers/staging/rtl8192u/r8192U_debugfs.c
> +++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
> @@ -173,3 +173,16 @@ void rtl8192_debugfs_exit(struct net_device *dev)
>  	priv->debugfs_dir = NULL;
>  }
>  
> +void rtl8192_debugfs_rename(struct net_device *dev)
> +{
> +	struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);

No need to cast.

> +
> +	if (!priv->debugfs_dir)
> +		return;

Why does it matter?

> +
> +	if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))

Ick, never poke around in a dentry from debugfs if you can help it.  You
know you are being renamed, why does it matter to check or not?

> +		return;
> +
> +	debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> +		       priv->debugfs_dir->d_parent, dev->name);

Don't poke around in the dentry for the parent here either.  That feels
racy and wrong.  Isn't there some other way to get the parent?

Also you can look up the dentry for this, no need to have it saved, like
I said in patch 2.

thanks,

greg k-h

Re: [PATCH v2 3/3] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Tong Zhang]

On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 18, 2022 at 10:50:38PM -0700, Tong Zhang wrote:
> > This driver creates 4 debug files under [devname] folder. The devname
> > could be wlan0 initially, however it could be renamed later to e.g.
> > enx00e04c00000. This will cause problem during debug file teardown since
> > it uses  netdev->name which is no longer wlan0. To solve this problem,
> > add a notifier to handle device renaming.
> >
> > Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> > Tested-by: Zheyu Ma <zheyuma97@gmail.com>
> > Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> > ---
> >  drivers/staging/rtl8192u/r8192U.h         |  1 +
> >  drivers/staging/rtl8192u/r8192U_core.c    | 35 ++++++++++++++++++++++-
> >  drivers/staging/rtl8192u/r8192U_debugfs.c | 13 +++++++++
> >  3 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
> > index e8860bb2b607..ccce37b7e2ae 100644
> > --- a/drivers/staging/rtl8192u/r8192U.h
> > +++ b/drivers/staging/rtl8192u/r8192U.h
> > @@ -1122,4 +1122,5 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
> >
> >  void rtl8192_debugfs_init(struct net_device *dev);
> >  void rtl8192_debugfs_exit(struct net_device *dev);
> > +void rtl8192_debugfs_rename(struct net_device *dev);
> >  #endif
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> > index ac3716550505..5cc78c5bd706 100644
> > --- a/drivers/staging/rtl8192u/r8192U_core.c
> > +++ b/drivers/staging/rtl8192u/r8192U_core.c
> > @@ -4606,6 +4606,30 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
> >       RT_TRACE(COMP_DOWN, "wlan driver removed\n");
> >  }
> >
> > +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> > +                                 void *data)
> > +{
> > +     struct net_device *netdev = netdev_notifier_info_to_dev(data);
> > +
> > +     if (netdev->netdev_ops != &rtl8192_netdev_ops)
> > +             goto out;
> > +
> > +     switch (event) {
> > +     case NETDEV_CHANGENAME:
> > +             rtl8192_debugfs_rename(netdev);
> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +out:
> > +     return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rtl8192_usb_netdev_notifier = {
> > +     .notifier_call = rtl8192_usb_netdev_event,
> > +};
> > +
> >  static int __init rtl8192_usb_module_init(void)
> >  {
> >       int ret;
> > @@ -4615,10 +4639,16 @@ static int __init rtl8192_usb_module_init(void)
> >       RT_TRACE(COMP_INIT, "Initializing module");
> >       RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
> >
> > +     ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> > +     if (ret) {
> > +             pr_err("register_netdevice_notifier failed %d\n", ret);
> > +             return ret;
> > +     }
> > +
> >       ret = ieee80211_debug_init();
> >       if (ret) {
> >               pr_err("ieee80211_debug_init() failed %d\n", ret);
> > -             return ret;
> > +             goto unregister_notifier;
> >       }
> >
> >       ret = ieee80211_crypto_init();
> > @@ -4660,6 +4690,8 @@ static int __init rtl8192_usb_module_init(void)
> >       ieee80211_crypto_deinit();
> >  debug_exit:
> >       ieee80211_debug_exit();
> > +unregister_notifier:
> > +     unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> >       return ret;
> >  }
> >
> > @@ -4671,6 +4703,7 @@ static void __exit rtl8192_usb_module_exit(void)
> >       ieee80211_crypto_tkip_exit();
> >       ieee80211_crypto_deinit();
> >       ieee80211_debug_exit();
> > +     unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> >       RT_TRACE(COMP_DOWN, "Exiting");
> >  }
> >
> > diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
> > index 5c9376e50889..c94f5dfac96b 100644
> > --- a/drivers/staging/rtl8192u/r8192U_debugfs.c
> > +++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
> > @@ -173,3 +173,16 @@ void rtl8192_debugfs_exit(struct net_device *dev)
> >       priv->debugfs_dir = NULL;
> >  }
> >
> > +void rtl8192_debugfs_rename(struct net_device *dev)
> > +{
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
>
> No need to cast.
>
> > +
> > +     if (!priv->debugfs_dir)
> > +             return;
>
> Why does it matter?
>
> > +
> > +     if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
>
> Ick, never poke around in a dentry from debugfs if you can help it.  You
> know you are being renamed, why does it matter to check or not?
>
> > +             return;
> > +
> > +     debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> > +                    priv->debugfs_dir->d_parent, dev->name);
>
> Don't poke around in the dentry for the parent here either.  That feels
> racy and wrong.  Isn't there some other way to get the parent?
>
> Also you can look up the dentry for this, no need to have it saved, like
> I said in patch 2.

I think we need to save the old entry here otherwise we don't know the
original name  since dev->name has already been overwritten.

> thanks,
>
> greg k-h

Re: [PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Tong Zhang]

On Mon, Jul 18, 2022 at 5:02 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Sun, Jul 17, 2022 at 12:01:57AM -0700, Tong Zhang wrote:
> > +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> > +             void *data)
> > +{
> > +     struct net_device *netdev = netdev_notifier_info_to_dev(data);
> > +
> > +     if (netdev->netdev_ops != &rtl8192_netdev_ops)
> > +             goto out;
> > +
> > +     switch (event) {
> > +             case NETDEV_CHANGENAME:
> > +                     rtl8192_debugfs_rename(netdev);
> > +                     break;
> > +
> > +             default:
> > +                     break;
> > +     }
>
> This isn't indented properly.  Don't forget to run ./scripts/checkpatch.pl
> on your patch.
>
> > +
> > +out:
> > +     return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rtl8192_usb_netdev_notifier = {
> > +     .notifier_call = rtl8192_usb_netdev_event,
> > +};
> > +
> > +
>
> Here too.
>
> >  static int __init rtl8192_usb_module_init(void)
> >  {
> >       int ret;
> > @@ -4788,10 +4638,14 @@ static int __init rtl8192_usb_module_init(void)
>
> [ snip ]
>
> > +void rtl8192_debugfs_init(struct net_device *dev)
> > +{
> > +     struct dentry *dir;
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +
> > +     dir = debugfs_create_dir(dev->name, NULL);
> > +     if (IS_ERR_OR_NULL(dir))
> > +             return;
>
> In olden times the debugfs_create_dir() function used to return a mix
> of error pointers and NULL.  But the idea with that function is that
> most people are not supposed to check for errors.  But instead of that
> they added all kind of *buggy* checks.  So then we made it just return
> error pointers.
>
> This code *does* care about it because it uses the
> priv->debugfs_dir->d_name.name in rtl8192_debugfs_rename().
>
> But caring about the debugfs dir and creating a rtl8192_debugfs_rename()
> function is really unusual.  And normally when we have to do something
> unusual that means we are doing something wrong...  :/
>
> Anyway, just check for if (IS_ERR(dir)) {
>
> > +
> > +     debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> > +     debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> > +     debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> > +     debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> > +
> > +     priv->debugfs_dir = dir;
> > +}
> > +
> > +void rtl8192_debugfs_exit(struct net_device *dev)
> > +{
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +     if (!priv->debugfs_dir)
> > +             return;
> > +     debugfs_remove_recursive(priv->debugfs_dir);
> > +     priv->debugfs_dir = NULL;
> > +}
> > +
> > +void rtl8192_debugfs_rename(struct net_device *dev)
> > +{
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +
> > +     if (!priv->debugfs_dir)
> > +             return;
> > +
> > +     if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
> > +             return;
> > +
> > +     debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> > +                     priv->debugfs_dir->d_parent, dev->name);
> > +}
> > +
>
> regards,
> dan carpenter

I modified the patch and sent it as v2.
Thanks and have a good one!
- Tong