| Message ID | 20220422080854.490379-1-zhaojunkui2008@126.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] mediatek/mt7601u: add debugfs exit function | expand |
On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote: > When mt7601u loaded, there are two cases: > First when mt7601u is loaded, in function mt7601u_probe, if > function mt7601u_probe run into error lable err_hw, > mt7601u_cleanup didn`t cleanup the debugfs node. > Second when the module disconnect, in function mt7601u_disconnect, > mt7601u_cleanup didn`t cleanup the debugfs node. > This patch add debugfs exit function and try to cleanup debugfs > node when mt7601u loaded fail or unloaded. > > Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com> Ah, missed that there was a v2. My point stands, wiphy debugfs dir should do the cleanup. Do you encounter problems in practice or are you sending this patches based on reading / static analysis of the code only.
At 2022-04-23 03:47:04, "Jakub Kicinski" <kubakici@wp.pl> wrote: >On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote: >> When mt7601u loaded, there are two cases: >> First when mt7601u is loaded, in function mt7601u_probe, if >> function mt7601u_probe run into error lable err_hw, >> mt7601u_cleanup didn`t cleanup the debugfs node. >> Second when the module disconnect, in function mt7601u_disconnect, >> mt7601u_cleanup didn`t cleanup the debugfs node. >> This patch add debugfs exit function and try to cleanup debugfs >> node when mt7601u loaded fail or unloaded. >> >> Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com> > >Ah, missed that there was a v2. My point stands, wiphy debugfs dir >should do the cleanup. > >Do you encounter problems in practice or are you sending this patches >based on reading / static analysis of the code only. Hi Jakub Kicinski: The issue here is found by reading code. I read the drivers/net/wireless code and found that many modules are not cleanup the debugfs. I sorted out the modules that were not cleaned up the debugfs: ./ti/wl18xx ./ti/wl12xx ./intel/iwlwifi ./intel/iwlwifi ./mediatek/mt76 I am not sure whether this part is welcome to kernel so I submitted a patch. If you have any suggestions, welcome to put forward for discussion, thank you！ BR//Bernard
z <zhaojunkui2008@126.com> writes: > At 2022-04-23 03:47:04, "Jakub Kicinski" <kubakici@wp.pl> wrote: >>On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote: >>> When mt7601u loaded, there are two cases: >>> First when mt7601u is loaded, in function mt7601u_probe, if >>> function mt7601u_probe run into error lable err_hw, >>> mt7601u_cleanup didn`t cleanup the debugfs node. >>> Second when the module disconnect, in function mt7601u_disconnect, >>> mt7601u_cleanup didn`t cleanup the debugfs node. >>> This patch add debugfs exit function and try to cleanup debugfs >>> node when mt7601u loaded fail or unloaded. >>> >>> Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com> >> >>Ah, missed that there was a v2. My point stands, wiphy debugfs dir >>should do the cleanup. >> >>Do you encounter problems in practice or are you sending this patches >>based on reading / static analysis of the code only. > > Hi Jakub Kicinski: > > The issue here is found by reading code. > I read the drivers/net/wireless code and found that many modules are > not cleanup the debugfs. > I sorted out the modules that were not cleaned up the debugfs: > ./ti/wl18xx > ./ti/wl12xx > ./intel/iwlwifi > ./intel/iwlwifi > ./mediatek/mt76 > I am not sure whether this part is welcome to kernel so I submitted a patch. > If you have any suggestions, welcome to put forward for discussion, thank you！ Jakub is saying that wiphy_unregister() recursively removes the debugfs directories: /* * First remove the hardware from everywhere, this makes * it impossible to find from userspace. */ debugfs_remove_recursive(rdev->wiphy.debugfsdir); So AFAICS there is no bug. But if you are testing this on a real hardware and something is wrong, please provide more info.
diff --git a/drivers/net/wireless/mediatek/mt7601u/debugfs.c b/drivers/net/wireless/mediatek/mt7601u/debugfs.c index 20669eacb66e..efc575a1147c 100644 --- a/drivers/net/wireless/mediatek/mt7601u/debugfs.c +++ b/drivers/net/wireless/mediatek/mt7601u/debugfs.c @@ -124,17 +124,20 @@ DEFINE_SHOW_ATTRIBUTE(mt7601u_eeprom_param); void mt7601u_init_debugfs(struct mt7601u_dev *dev) { - struct dentry *dir; - - dir = debugfs_create_dir("mt7601u", dev->hw->wiphy->debugfsdir); - if (!dir) + dev->root_dir = debugfs_create_dir("mt7601u", dev->hw->wiphy->debugfsdir); + if (!dev->root_dir) return; - debugfs_create_u8("temperature", 0400, dir, &dev->raw_temp); - debugfs_create_u32("temp_mode", 0400, dir, &dev->temp_mode); + debugfs_create_u8("temperature", 0400, dev->root_dir, &dev->raw_temp); + debugfs_create_u32("temp_mode", 0400, dev->root_dir, &dev->temp_mode); + + debugfs_create_u32("regidx", 0600, dev->root_dir, &dev->debugfs_reg); + debugfs_create_file("regval", 0600, dev->root_dir, dev, &fops_regval); + debugfs_create_file("ampdu_stat", 0400, dev->root_dir, dev, &mt7601u_ampdu_stat_fops); + debugfs_create_file("eeprom_param", 0400, dev->root_dir, dev, &mt7601u_eeprom_param_fops); +} - debugfs_create_u32("regidx", 0600, dir, &dev->debugfs_reg); - debugfs_create_file("regval", 0600, dir, dev, &fops_regval); - debugfs_create_file("ampdu_stat", 0400, dir, dev, &mt7601u_ampdu_stat_fops); - debugfs_create_file("eeprom_param", 0400, dir, dev, &mt7601u_eeprom_param_fops); +void mt7601u_exit_debugfs(struct mt7601u_dev *dev) +{ + debugfs_remove(dev->root_dir); } diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c b/drivers/net/wireless/mediatek/mt7601u/init.c index 5d9e952b2966..eacdd5785fa6 100644 --- a/drivers/net/wireless/mediatek/mt7601u/init.c +++ b/drivers/net/wireless/mediatek/mt7601u/init.c @@ -427,6 +427,7 @@ void mt7601u_cleanup(struct mt7601u_dev *dev) mt7601u_stop_hardware(dev); mt7601u_dma_cleanup(dev); mt7601u_mcu_cmd_deinit(dev); + mt7601u_exit_debugfs(dev); } struct mt7601u_dev *mt7601u_alloc_device(struct device *pdev) diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h index a122f1dd38f6..06c190a3f54c 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h +++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h @@ -242,6 +242,8 @@ struct mt7601u_dev { u32 rf_pa_mode[2]; struct mac_stats stats; + + struct dentry *root_dir; }; struct mt7601u_tssi_params { @@ -279,6 +281,7 @@ struct mt7601u_rxwi; extern const struct ieee80211_ops mt7601u_ops; void mt7601u_init_debugfs(struct mt7601u_dev *dev); +void mt7601u_exit_debugfs(struct mt7601u_dev *dev); u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset); void mt7601u_wr(struct mt7601u_dev *dev, u32 offset, u32 val);
When mt7601u loaded, there are two cases: First when mt7601u is loaded, in function mt7601u_probe, if function mt7601u_probe run into error lable err_hw, mt7601u_cleanup didn`t cleanup the debugfs node. Second when the module disconnect, in function mt7601u_disconnect, mt7601u_cleanup didn`t cleanup the debugfs node. This patch add debugfs exit function and try to cleanup debugfs node when mt7601u loaded fail or unloaded. Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com> Changes since V1: *Remove CONFIG_DEBUG_FS check. --- .../net/wireless/mediatek/mt7601u/debugfs.c | 23 +++++++++++-------- drivers/net/wireless/mediatek/mt7601u/init.c | 1 + .../net/wireless/mediatek/mt7601u/mt7601u.h | 3 +++ 3 files changed, 17 insertions(+), 10 deletions(-)