Messages in this thread |  | | Date | Fri, 29 Nov 2019 23:23:10 +0100 | From | Greg KH <> | Subject | Re: [PATCH next 0/3] debugfs: introduce debugfs_create_single/seq[,_data] |
| |
On Fri, Nov 29, 2019 at 11:19:38PM +0100, Greg KH wrote: > 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?
I refer to DEFINE_SIMPLE_ATTRIBUTE(), sorry for not saying that here.
And if they do not work, how about creating: DEFINE_SEQ_ATTRIBUTE() in much the same way for the whole kernel to use.
thanks,
greg k-h
|  |