lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Jul 2022 23:30:48 -0700 From: Tong Zhang <ztong0001@...il.com> To: Greg Kroah-Hartman <gregkh@...uxfoundation.org> Cc: Jakub Kicinski <kuba@...nel.org>, Colin Ian King <colin.king@...el.com>, Saurav Girepunje <saurav.girepunje@...il.com>, Nathan Chancellor <nathan@...nel.org>, Johan Hovold <johan@...nel.org>, open list <linux-kernel@...r.kernel.org>, linux-staging@...ts.linux.dev, Dan Carpenter <dan.carpenter@...cle.com> Subject: Re: [PATCH v2 2/3] staging: rtl8192u: move debug files to debugfs On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman <gregkh@...uxfoundation.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
Powered by blists - more mailing lists