Messages in this thread | | | Date | Fri, 29 Nov 2019 23:19:38 +0100 | From | Greg KH <> | Subject | Re: [PATCH next 0/3] debugfs: introduce debugfs_create_single/seq[,_data] |
| |
On Fri, Nov 29, 2019 at 11:16:38PM +0800, Kefeng Wang wrote: > > > On 2019/11/29 22:21, Greg KH wrote: > > On Fri, Nov 29, 2019 at 05:27:49PM +0800, Kefeng Wang wrote: > >> Like proc_create_single/seq[,_data] in procfs, we could provide similar debugfs > >> helper to reduce losts of boilerplate code. > >> > >> debugfs_create_single[,_data] > >> creates a file in debugfs with the extra data and a seq_file show callback. > >> debugfs_create_seq[,_data] > >> creates a file in debugfs with the extra data and a seq_operations. > >> > >> There is a object dynamically allocated in the helper, which is used to store > >> extra data, we need free it when remove the debugfs file. > >> > >> If the change is acceptable, we could change the caller one by one. > > > > I would like to see a user of this and how you would convert it, in > > order to see if this is worth it or not. > > I have some diff patches, the conversion is in progress. current statistics > are as follows, > > 1) debugfs: switch to debugfs_create_seq[,_data] > 19 files changed, 85 insertions(+), 620 deletions(-) > 2) debugfs: switch to debugfs_create_single[,_data] > 70 files changed, 249 insertions(+), 1482 deletions(-) > > Here are some examples, > 1) debugfs_create_seq > diff --git a/mm/vmstat.c b/mm/vmstat.c > index 78d53378db99..62c26772f24c 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -2057,18 +2057,6 @@ static const struct seq_operations unusable_op = { > .show = unusable_show, > }; > > -static int unusable_open(struct inode *inode, struct file *file) > -{ > - return seq_open(file, &unusable_op); > -} > - > -static const struct file_operations unusable_file_ops = { > - .open = unusable_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release = seq_release, > -}; > -
Can't this file just use the normal file macro/interface for debugfs files instead? Hm, maybe not, it seems the celf code wants to do much the same as above, but is seq_read() really needed for these?
There are loads of places where open-coded debugfs file ops can just be converted to use the existing macros, maybe do that work first before adding new ones?
thanks,
greg k-h
| |