Messages in this thread | | | Date | Fri, 29 Nov 2019 23:16:09 +0100 | From | Greg KH <> | Subject | Re: [PATCH next 3/3] debugfs: introduce debugfs_create_seq[,_data] |
| |
On Fri, Nov 29, 2019 at 11:03:47PM +0800, Kefeng Wang wrote: > > > On 2019/11/29 22:22, Greg KH wrote: > > On Fri, Nov 29, 2019 at 05:27:52PM +0800, Kefeng Wang wrote: > >> Like proc_create_seq[,_data] in procfs, introduce a similar > >> debugfs_create_seq[,_data] function, which could drastically > >> reduces the boilerplate code. > >> > >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > >> --- > >> fs/debugfs/file.c | 62 ++++++++++++++++++++++++++++++++++++++++- > >> include/linux/debugfs.h | 16 +++++++++++ > >> 2 files changed, 77 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > >> index 68f0c4b19bef..77522717c9fb 100644 > >> --- a/fs/debugfs/file.c > >> +++ b/fs/debugfs/file.c > >> @@ -1107,7 +1107,10 @@ EXPORT_SYMBOL_GPL(debugfs_create_regset32); > >> #endif /* CONFIG_HAS_IOMEM */ > >> > >> struct debugfs_entry { > >> - int (*read)(struct seq_file *seq, void *data); > >> + union { > >> + const struct seq_operations *seq_ops; > >> + int (*read)(struct seq_file *seq, void *data); > >> + }; > >> void *data; > >> }; > >> > >> @@ -1196,3 +1199,60 @@ struct dentry *debugfs_create_single_data(const char *name, umode_t mode, > >> &debugfs_entry_ops); > >> } > >> EXPORT_SYMBOL_GPL(debugfs_create_single_data); > >> + > >> +static int debugfs_entry_seq_open(struct inode *inode, struct file *file) > >> +{ > >> + struct debugfs_entry *entry = inode->i_private; > >> + int ret; > >> + > >> + entry = debugfs_clear_lowest_bit(entry); > >> + > >> + ret = seq_open(file, entry->seq_ops); > >> + if (!ret && entry->data) { > >> + struct seq_file *seq = file->private_data; > >> + seq->private = entry->data; > >> + } > >> + > >> + return ret; > >> +} > >> + > >> +static const struct file_operations debugfs_seq_fops = { > >> + .open = debugfs_entry_seq_open, > >> + .read = seq_read, > >> + .llseek = seq_lseek, > >> + .release = seq_release, > >> +}; > >> + > >> +/** > >> + * debugfs_create_seq_data - create a file in the debugfs filesystem > >> + * @name: a pointer to a string containing the name of the file to create. > >> + * @mode: the permission that the file should have. > >> + * @parent: a pointer to the parent dentry for this file. This should be a > >> + * directory dentry if set. If this parameter is NULL, then the > >> + * file will be created in the root of the debugfs filesystem. > >> + * @data: a pointer to something that the caller will want to get to later > >> + * on. The inode.i_private pointer will point to this value on > >> + * the open() call. > >> + * @seq_ops: seq_operations pointer of the seqfile. > >> + * > >> + * This function creates a file in debugfs with the extra data and a seq_ops. > >> + */ > >> +struct dentry *debugfs_create_seq_data(const char *name, umode_t mode, > >> + struct dentry *parent, void *data, > >> + const struct seq_operations *seq_ops) > >> +{ > >> + struct debugfs_entry *entry; > >> + > >> + entry = kzalloc(sizeof(*entry), GFP_KERNEL); > >> + if (!entry) > >> + return ERR_PTR(-ENOMEM); > >> + > >> + entry->seq_ops = seq_ops; > >> + entry->data = data; > >> + > >> + entry = debugfs_set_lowest_bit(entry); > >> + > >> + return debugfs_create_file(name, mode, parent, entry, > >> + &debugfs_seq_fops); > >> +} > >> +EXPORT_SYMBOL_GPL(debugfs_create_seq_data); > >> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h > >> index 82171f183e93..d32d49983547 100644 > >> --- a/include/linux/debugfs.h > >> +++ b/include/linux/debugfs.h > >> @@ -147,6 +147,10 @@ struct dentry *debugfs_create_single_data(const char *name, umode_t mode, > >> int (*read_fn)(struct seq_file *s, > >> void *data)); > >> > >> +struct dentry *debugfs_create_seq_data(const char *name, umode_t mode, > >> + struct dentry *parent, void *data, > >> + const struct seq_operations *seq_ops); > >> + > >> bool debugfs_initialized(void); > >> > >> ssize_t debugfs_read_file_bool(struct file *file, char __user *user_buf, > > > > If you notice, I have been removing the return value of the > > debugfs_create_* functions over the past few kernel versions (look at > > 5.5-rc1 for a lot more). Please don't add any new functions that also > > return a dentry that I then need to go and remove. > > Hi Greg, see function debugfs_create_seq_data()and debugfs_create_single_data(), > they are wrapper function of debugfs_create_file(), when remove the debugfs file > called debugfs_remove(), some drivers could do such thing, we must return a dentry > to the caller.
Again, no, we should not return a dentry. Files should go in directories and then removed all at once if they need to, which is why I have been making these types of changes.
Yes, for some existing files that does not work, but again, please do not create a new api that does this. Only use it for the files that do not need their dentries saved.
thanks,
greg k-h
| |