From cb787f4ac0c2e439ea8d7e6387b925f74576bdf8 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 27 Sep 2024 02:56:11 +0100 Subject: [tree-wide] finally take no_llseek out no_llseek had been defined to NULL two years ago, in commit 868941b14441 ("fs: remove no_llseek") To quote that commit, At -rc1 we'll need do a mechanical removal of no_llseek - git grep -l -w no_llseek | grep -v porting.rst | while read i; do sed -i '/\/d' $i done would do it. Unfortunately, that hadn't been done. Linus, could you do that now, so that we could finally put that thing to rest? All instances are of the form .llseek = no_llseek, so it's obviously safe. Signed-off-by: Al Viro Signed-off-by: Linus Torvalds --- include/linux/debugfs.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/linux/debugfs.h') diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index c9c65b132c0f..0928a6c8ae1e 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -57,7 +57,6 @@ static const struct file_operations __fops = { \ .release = simple_attr_release, \ .read = debugfs_attr_read, \ .write = (__is_signed) ? debugfs_attr_write_signed : debugfs_attr_write, \ - .llseek = no_llseek, \ } #define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \ -- cgit v1.2.3 From 8dc6d81c6b2acc434b00c4585f0594739031c4e4 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Tue, 22 Oct 2024 15:18:34 +0200 Subject: debugfs: add small file operations for most files As struct file_operations is really big, but (most) debugfs files only use simple_open, read, write and perhaps seek, and don't need anything else, this wastes a lot of space for NULL pointers. Add a struct debugfs_short_fops and some bookkeeping code in debugfs so that users can use that with debugfs_create_file() using _Generic to figure out which function to use. Converting mac80211 to use it where possible saves quite a bit of space: 1010127 205064 1220 1216411 128f9b net/mac80211/mac80211.ko (before) 981199 205064 1220 1187483 121e9b net/mac80211/mac80211.ko (after) ------- -28928 = ~28KiB With a marginal space cost in debugfs: 8701 550 16 9267 2433 fs/debugfs/inode.o (before) 25233 325 32 25590 63f6 fs/debugfs/file.o (before) 8914 558 16 9488 2510 fs/debugfs/inode.o (after) 25380 325 32 25737 6489 fs/debugfs/file.o (after) --------------- +360 +8 (All on x86-64) A simple spatch suggests there are more than 300 instances, not even counting the ones hidden in macros like in mac80211, that could be trivially converted, for additional savings of about 240 bytes for each. Reviewed-by: Greg Kroah-Hartman Link: https://patch.msgid.link/20241022151838.26f9925fb959.Ia80b55e934bbfc45ce0df42a3233d34b35508046@changeid Signed-off-by: Johannes Berg --- fs/debugfs/file.c | 100 +++++++++++++++++++++++++++++++++--------------- fs/debugfs/inode.c | 63 +++++++++++++----------------- fs/debugfs/internal.h | 6 +++ include/linux/debugfs.h | 62 ++++++++++++++++++++++++++++-- 4 files changed, 160 insertions(+), 71 deletions(-) (limited to 'include/linux/debugfs.h') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 67299e8b734e..47dc96dfe386 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -100,8 +100,16 @@ int debugfs_file_get(struct dentry *dentry) if (!fsd) return -ENOMEM; - fsd->real_fops = (void *)((unsigned long)d_fsd & - ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); + if ((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT) { + fsd->real_fops = NULL; + fsd->short_fops = (void *)((unsigned long)d_fsd & + ~(DEBUGFS_FSDATA_IS_REAL_FOPS_BIT | + DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT)); + } else { + fsd->real_fops = (void *)((unsigned long)d_fsd & + ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); + fsd->short_fops = NULL; + } refcount_set(&fsd->active_users, 1); init_completion(&fsd->active_users_drained); INIT_LIST_HEAD(&fsd->cancellations); @@ -241,9 +249,10 @@ static int debugfs_locked_down(struct inode *inode, { if ((inode->i_mode & 07777 & ~0444) == 0 && !(filp->f_mode & FMODE_WRITE) && - !real_fops->unlocked_ioctl && - !real_fops->compat_ioctl && - !real_fops->mmap) + (!real_fops || + (!real_fops->unlocked_ioctl && + !real_fops->compat_ioctl && + !real_fops->mmap))) return 0; if (security_locked_down(LOCKDOWN_DEBUGFS)) @@ -316,19 +325,38 @@ static ret_type full_proxy_ ## name(proto) \ return r; \ } -FULL_PROXY_FUNC(llseek, loff_t, filp, - PROTO(struct file *filp, loff_t offset, int whence), - ARGS(filp, offset, whence)); +#define FULL_PROXY_FUNC_BOTH(name, ret_type, filp, proto, args) \ +static ret_type full_proxy_ ## name(proto) \ +{ \ + struct dentry *dentry = F_DENTRY(filp); \ + struct debugfs_fsdata *fsd; \ + ret_type r; \ + \ + r = debugfs_file_get(dentry); \ + if (unlikely(r)) \ + return r; \ + fsd = dentry->d_fsdata; \ + if (fsd->real_fops) \ + r = fsd->real_fops->name(args); \ + else \ + r = fsd->short_fops->name(args); \ + debugfs_file_put(dentry); \ + return r; \ +} + +FULL_PROXY_FUNC_BOTH(llseek, loff_t, filp, + PROTO(struct file *filp, loff_t offset, int whence), + ARGS(filp, offset, whence)); -FULL_PROXY_FUNC(read, ssize_t, filp, - PROTO(struct file *filp, char __user *buf, size_t size, - loff_t *ppos), - ARGS(filp, buf, size, ppos)); +FULL_PROXY_FUNC_BOTH(read, ssize_t, filp, + PROTO(struct file *filp, char __user *buf, size_t size, + loff_t *ppos), + ARGS(filp, buf, size, ppos)); -FULL_PROXY_FUNC(write, ssize_t, filp, - PROTO(struct file *filp, const char __user *buf, size_t size, - loff_t *ppos), - ARGS(filp, buf, size, ppos)); +FULL_PROXY_FUNC_BOTH(write, ssize_t, filp, + PROTO(struct file *filp, const char __user *buf, + size_t size, loff_t *ppos), + ARGS(filp, buf, size, ppos)); FULL_PROXY_FUNC(unlocked_ioctl, long, filp, PROTO(struct file *filp, unsigned int cmd, unsigned long arg), @@ -363,7 +391,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp) * not to leak any resources. Releasers must not assume that * ->i_private is still being meaningful here. */ - if (real_fops->release) + if (real_fops && real_fops->release) r = real_fops->release(inode, filp); replace_fops(filp, d_inode(dentry)->i_fop); @@ -373,39 +401,48 @@ static int full_proxy_release(struct inode *inode, struct file *filp) } static void __full_proxy_fops_init(struct file_operations *proxy_fops, - const struct file_operations *real_fops) + struct debugfs_fsdata *fsd) { proxy_fops->release = full_proxy_release; - if (real_fops->llseek) + + if ((fsd->real_fops && fsd->real_fops->llseek) || + (fsd->short_fops && fsd->short_fops->llseek)) proxy_fops->llseek = full_proxy_llseek; - if (real_fops->read) + + if ((fsd->real_fops && fsd->real_fops->read) || + (fsd->short_fops && fsd->short_fops->read)) proxy_fops->read = full_proxy_read; - if (real_fops->write) + + if ((fsd->real_fops && fsd->real_fops->write) || + (fsd->short_fops && fsd->short_fops->write)) proxy_fops->write = full_proxy_write; - if (real_fops->poll) + + if (fsd->real_fops && fsd->real_fops->poll) proxy_fops->poll = full_proxy_poll; - if (real_fops->unlocked_ioctl) + + if (fsd->real_fops && fsd->real_fops->unlocked_ioctl) proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl; } static int full_proxy_open(struct inode *inode, struct file *filp) { struct dentry *dentry = F_DENTRY(filp); - const struct file_operations *real_fops = NULL; + const struct file_operations *real_fops; struct file_operations *proxy_fops = NULL; + struct debugfs_fsdata *fsd; int r; r = debugfs_file_get(dentry); if (r) return r == -EIO ? -ENOENT : r; - real_fops = debugfs_real_fops(filp); - + fsd = dentry->d_fsdata; + real_fops = fsd->real_fops; r = debugfs_locked_down(inode, filp, real_fops); if (r) goto out; - if (!fops_get(real_fops)) { + if (real_fops && !fops_get(real_fops)) { #ifdef CONFIG_MODULES if (real_fops->owner && real_fops->owner->state == MODULE_STATE_GOING) { @@ -426,11 +463,14 @@ static int full_proxy_open(struct inode *inode, struct file *filp) r = -ENOMEM; goto free_proxy; } - __full_proxy_fops_init(proxy_fops, real_fops); + __full_proxy_fops_init(proxy_fops, fsd); replace_fops(filp, proxy_fops); - if (real_fops->open) { - r = real_fops->open(inode, filp); + if (!real_fops || real_fops->open) { + if (real_fops) + r = real_fops->open(inode, filp); + else + r = simple_open(inode, filp); if (r) { replace_fops(filp, d_inode(dentry)->i_fop); goto free_proxy; diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 66d9b3b4c588..38a9c7eb97e6 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -412,7 +412,7 @@ static struct dentry *end_creating(struct dentry *dentry) static struct dentry *__debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *proxy_fops, - const struct file_operations *real_fops) + const void *real_fops) { struct dentry *dentry; struct inode *inode; @@ -450,49 +450,38 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, return end_creating(dentry); } -/** - * debugfs_create_file - 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. - * @fops: a pointer to a struct file_operations that should be used for - * this file. - * - * This is the basic "create a file" function for debugfs. It allows for a - * wide range of flexibility in creating a file, or a directory (if you want - * to create a directory, the debugfs_create_dir() function is - * recommended to be used instead.) - * - * This function will return a pointer to a dentry if it succeeds. This - * pointer must be passed to the debugfs_remove() function when the file is - * to be removed (no automatic cleanup happens if your module is unloaded, - * you are responsible here.) If an error occurs, ERR_PTR(-ERROR) will be - * returned. - * - * If debugfs is not enabled in the kernel, the value -%ENODEV will be - * returned. - * - * NOTE: it's expected that most callers should _ignore_ the errors returned - * by this function. Other debugfs functions handle the fact that the "dentry" - * passed to them could be an error and they don't crash in that case. - * Drivers should generally work fine even if debugfs fails to init anyway. - */ -struct dentry *debugfs_create_file(const char *name, umode_t mode, - struct dentry *parent, void *data, - const struct file_operations *fops) +struct dentry *debugfs_create_file_full(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct file_operations *fops) { + if (WARN_ON((unsigned long)fops & + (DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT | + DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))) + return ERR_PTR(-EINVAL); return __debugfs_create_file(name, mode, parent, data, fops ? &debugfs_full_proxy_file_operations : &debugfs_noop_file_operations, fops); } -EXPORT_SYMBOL_GPL(debugfs_create_file); +EXPORT_SYMBOL_GPL(debugfs_create_file_full); + +struct dentry *debugfs_create_file_short(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct debugfs_short_fops *fops) +{ + if (WARN_ON((unsigned long)fops & + (DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT | + DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))) + return ERR_PTR(-EINVAL); + + return __debugfs_create_file(name, mode, parent, data, + fops ? &debugfs_full_proxy_file_operations : + &debugfs_noop_file_operations, + (const void *)((unsigned long)fops | + DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT)); +} +EXPORT_SYMBOL_GPL(debugfs_create_file_short); /** * debugfs_create_file_unsafe - create a file in the debugfs filesystem diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index dae80c2a469e..a3edfa4f0d8e 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -18,6 +18,7 @@ extern const struct file_operations debugfs_full_proxy_file_operations; struct debugfs_fsdata { const struct file_operations *real_fops; + const struct debugfs_short_fops *short_fops; union { /* automount_fn is used when real_fops is NULL */ debugfs_automount_t automount; @@ -39,6 +40,11 @@ struct debugfs_fsdata { * pointer gets its lowest bit set. */ #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0) +/* + * A dentry's ->d_fsdata, when pointing to real fops, is with + * short fops instead of full fops. + */ +#define DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT BIT(1) /* Access BITS */ #define DEBUGFS_ALLOW_API BIT(0) diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 0928a6c8ae1e..59444b495d49 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -71,9 +71,63 @@ typedef struct vfsmount *(*debugfs_automount_t)(struct dentry *, void *); struct dentry *debugfs_lookup(const char *name, struct dentry *parent); -struct dentry *debugfs_create_file(const char *name, umode_t mode, - struct dentry *parent, void *data, - const struct file_operations *fops); +struct debugfs_short_fops { + ssize_t (*read)(struct file *, char __user *, size_t, loff_t *); + ssize_t (*write)(struct file *, const char __user *, size_t, loff_t *); + loff_t (*llseek) (struct file *, loff_t, int); +}; + +struct dentry *debugfs_create_file_full(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct file_operations *fops); +struct dentry *debugfs_create_file_short(const char *name, umode_t mode, + struct dentry *parent, void *data, + const struct debugfs_short_fops *fops); + +/** + * debugfs_create_file - 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. + * @fops: a pointer to a struct file_operations or struct debugfs_short_fops that + * should be used for this file. + * + * This is the basic "create a file" function for debugfs. It allows for a + * wide range of flexibility in creating a file, or a directory (if you want + * to create a directory, the debugfs_create_dir() function is + * recommended to be used instead.) + * + * This function will return a pointer to a dentry if it succeeds. This + * pointer must be passed to the debugfs_remove() function when the file is + * to be removed (no automatic cleanup happens if your module is unloaded, + * you are responsible here.) If an error occurs, ERR_PTR(-ERROR) will be + * returned. + * + * If debugfs is not enabled in the kernel, the value -%ENODEV will be + * returned. + * + * If fops points to a struct debugfs_short_fops, then simple_open() will be + * used for the open, and only read/write/llseek are supported and are proxied, + * so no module reference or release are needed. + * + * NOTE: it's expected that most callers should _ignore_ the errors returned + * by this function. Other debugfs functions handle the fact that the "dentry" + * passed to them could be an error and they don't crash in that case. + * Drivers should generally work fine even if debugfs fails to init anyway. + */ +#define debugfs_create_file(name, mode, parent, data, fops) \ + _Generic(fops, \ + const struct file_operations *: debugfs_create_file_full, \ + const struct debugfs_short_fops *: debugfs_create_file_short, \ + struct file_operations *: debugfs_create_file_full, \ + struct debugfs_short_fops *: debugfs_create_file_short) \ + (name, mode, parent, data, fops) + struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops); @@ -207,7 +261,7 @@ static inline struct dentry *debugfs_lookup(const char *name, static inline struct dentry *debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, - const struct file_operations *fops) + const void *fops) { return ERR_PTR(-ENODEV); } -- cgit v1.2.3 From 12c92098932b4bbf38396e9aed0a343d35437a21 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 12 Jan 2025 08:06:49 +0000 Subject: debugfs: allow to store an additional opaque pointer at file creation Set by debugfs_create_file_aux(name, mode, parent, data, aux, fops). Plain debugfs_create_file() has it set to NULL. Accessed by debugfs_get_aux(file). Convenience macros for numeric opaque data - debugfs_create_file_aux_num and debugfs_get_aux_num, resp. Signed-off-by: Al Viro Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20250112080705.141166-5-viro@zeniv.linux.org.uk Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 6 ++++++ fs/debugfs/inode.c | 14 +++++++++----- fs/debugfs/internal.h | 1 + include/linux/debugfs.h | 27 ++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 6 deletions(-) (limited to 'include/linux/debugfs.h') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index ae014bd36a6f..e33cc77699cd 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -47,6 +47,12 @@ const struct file_operations debugfs_noop_file_operations = { #define F_DENTRY(filp) ((filp)->f_path.dentry) +const void *debugfs_get_aux(const struct file *file) +{ + return DEBUGFS_I(file_inode(file))->aux; +} +EXPORT_SYMBOL_GPL(debugfs_get_aux); + const struct file_operations *debugfs_real_fops(const struct file *filp) { struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata; diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index c4e8b7f758e0..51d4c3e9d422 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -424,6 +424,7 @@ static struct dentry *end_creating(struct dentry *dentry) static struct dentry *__debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct file_operations *proxy_fops, const void *real_fops) { @@ -458,6 +459,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, proxy_fops = &debugfs_noop_file_operations; inode->i_fop = proxy_fops; DEBUGFS_I(inode)->raw = real_fops; + DEBUGFS_I(inode)->aux = aux; d_instantiate(dentry, inode); fsnotify_create(d_inode(dentry->d_parent), dentry); @@ -466,19 +468,21 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, struct dentry *debugfs_create_file_full(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct file_operations *fops) { - return __debugfs_create_file(name, mode, parent, data, + return __debugfs_create_file(name, mode, parent, data, aux, &debugfs_full_proxy_file_operations, fops); } EXPORT_SYMBOL_GPL(debugfs_create_file_full); struct dentry *debugfs_create_file_short(const char *name, umode_t mode, - struct dentry *parent, void *data, - const struct debugfs_short_fops *fops) + struct dentry *parent, void *data, + const void *aux, + const struct debugfs_short_fops *fops) { - return __debugfs_create_file(name, mode, parent, data, + return __debugfs_create_file(name, mode, parent, data, aux, &debugfs_full_short_proxy_file_operations, fops); } @@ -516,7 +520,7 @@ struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, const struct file_operations *fops) { - return __debugfs_create_file(name, mode, parent, data, + return __debugfs_create_file(name, mode, parent, data, NULL, &debugfs_open_proxy_file_operations, fops); } diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index 8d2de647b42c..93483fe84425 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -19,6 +19,7 @@ struct debugfs_inode_info { const struct debugfs_short_fops *short_fops; debugfs_automount_t automount; }; + const void *aux; }; static inline struct debugfs_inode_info *DEBUGFS_I(struct inode *inode) diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 59444b495d49..7c97417d73b5 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -79,9 +79,11 @@ struct debugfs_short_fops { struct dentry *debugfs_create_file_full(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct file_operations *fops); struct dentry *debugfs_create_file_short(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct debugfs_short_fops *fops); /** @@ -126,7 +128,15 @@ struct dentry *debugfs_create_file_short(const char *name, umode_t mode, const struct debugfs_short_fops *: debugfs_create_file_short, \ struct file_operations *: debugfs_create_file_full, \ struct debugfs_short_fops *: debugfs_create_file_short) \ - (name, mode, parent, data, fops) + (name, mode, parent, data, NULL, fops) + +#define debugfs_create_file_aux(name, mode, parent, data, aux, fops) \ + _Generic(fops, \ + const struct file_operations *: debugfs_create_file_full, \ + const struct debugfs_short_fops *: debugfs_create_file_short, \ + struct file_operations *: debugfs_create_file_full, \ + struct debugfs_short_fops *: debugfs_create_file_short) \ + (name, mode, parent, data, aux, fops) struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, struct dentry *parent, void *data, @@ -153,6 +163,7 @@ void debugfs_remove(struct dentry *dentry); void debugfs_lookup_and_remove(const char *name, struct dentry *parent); const struct file_operations *debugfs_real_fops(const struct file *filp); +const void *debugfs_get_aux(const struct file *file); int debugfs_file_get(struct dentry *dentry); void debugfs_file_put(struct dentry *dentry); @@ -259,6 +270,14 @@ static inline struct dentry *debugfs_lookup(const char *name, return ERR_PTR(-ENODEV); } +static inline struct dentry *debugfs_create_file_aux(const char *name, + umode_t mode, struct dentry *parent, + void *data, void *aux, + const void *fops) +{ + return ERR_PTR(-ENODEV); +} + static inline struct dentry *debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, const void *fops) @@ -312,6 +331,7 @@ static inline void debugfs_lookup_and_remove(const char *name, { } const struct file_operations *debugfs_real_fops(const struct file *filp); +void *debugfs_get_aux(const struct file *file); static inline int debugfs_file_get(struct dentry *dentry) { @@ -452,6 +472,11 @@ static inline ssize_t debugfs_read_file_str(struct file *file, #endif +#define debugfs_create_file_aux_num(name, mode, parent, data, n, fops) \ + debugfs_create_file_aux(name, mode, parent, data, \ + (void *)(unsigned long)n, fops) +#define debugfs_get_aux_num(f) (unsigned long)debugfs_get_aux(f) + /** * debugfs_create_xul - create a debugfs file that is used to read and write an * unsigned long value, formatted in hexadecimal -- cgit v1.2.3 From d1433c7ba289319983ec0086dd22524721a797ef Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 12 Jan 2025 08:06:50 +0000 Subject: debugfs: take debugfs_short_fops definition out of ifdef Signed-off-by: Al Viro Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20250112080705.141166-6-viro@zeniv.linux.org.uk Signed-off-by: Greg Kroah-Hartman --- include/linux/debugfs.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'include/linux/debugfs.h') diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 7c97417d73b5..68e9c6cbd835 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -67,16 +67,16 @@ static const struct file_operations __fops = { \ typedef struct vfsmount *(*debugfs_automount_t)(struct dentry *, void *); -#if defined(CONFIG_DEBUG_FS) - -struct dentry *debugfs_lookup(const char *name, struct dentry *parent); - struct debugfs_short_fops { ssize_t (*read)(struct file *, char __user *, size_t, loff_t *); ssize_t (*write)(struct file *, const char __user *, size_t, loff_t *); loff_t (*llseek) (struct file *, loff_t, int); }; +#if defined(CONFIG_DEBUG_FS) + +struct dentry *debugfs_lookup(const char *name, struct dentry *parent); + struct dentry *debugfs_create_file_full(const char *name, umode_t mode, struct dentry *parent, void *data, const void *aux, -- cgit v1.2.3 From f7862dfef6612b87b2ad8352c4d73886f09456d6 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 12 Jan 2025 08:07:05 +0000 Subject: saner replacement for debugfs_rename() Existing primitive has several problems: 1) calling conventions are clumsy - it returns a dentry reference that is either identical to its second argument or is an ERR_PTR(-E...); in both cases no refcount changes happen. Inconvenient for users and bug-prone; it would be better to have it return 0 on success and -E... on failure. 2) it allows cross-directory moves; however, no such caller have ever materialized and considering the way debugfs is used, it's unlikely to happen in the future. What's more, any such caller would have fun issues to deal with wrt interplay with recursive removal. It also makes the calling conventions clumsier... 3) tautological rename fails; the callers have no race-free way to deal with that. 4) new name must have been formed by the caller; quite a few callers have it done by sprintf/kasprintf/etc., ending up with considerable boilerplate. Proposed replacement: int debugfs_change_name(dentry, fmt, ...). All callers convert to that easily, and it's simpler internally. IMO debugfs_rename() should go; if we ever get a real-world use case for cross-directory moves in debugfs, we can always look into the right way to handle that. Signed-off-by: Al Viro Link: https://lore.kernel.org/r/20250112080705.141166-21-viro@zeniv.linux.org.uk Signed-off-by: Greg Kroah-Hartman --- Documentation/filesystems/debugfs.rst | 12 +-- drivers/net/bonding/bond_debugfs.c | 9 +- drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 19 +--- drivers/net/ethernet/marvell/skge.c | 5 +- drivers/net/ethernet/marvell/sky2.c | 5 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +- drivers/opp/debugfs.c | 10 +- fs/debugfs/inode.c | 106 ++++++++++------------ include/linux/debugfs.h | 9 +- mm/shrinker_debug.c | 16 +--- net/hsr/hsr_debugfs.c | 9 +- net/mac80211/debugfs_netdev.c | 11 +-- net/wireless/core.c | 5 +- 13 files changed, 77 insertions(+), 145 deletions(-) (limited to 'include/linux/debugfs.h') diff --git a/Documentation/filesystems/debugfs.rst b/Documentation/filesystems/debugfs.rst index dc35da8b8792..f7f977ffbf8d 100644 --- a/Documentation/filesystems/debugfs.rst +++ b/Documentation/filesystems/debugfs.rst @@ -211,18 +211,16 @@ seq_file content. There are a couple of other directory-oriented helper functions:: - struct dentry *debugfs_rename(struct dentry *old_dir, - struct dentry *old_dentry, - struct dentry *new_dir, - const char *new_name); + struct dentry *debugfs_change_name(struct dentry *dentry, + const char *fmt, ...); struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent, const char *target); -A call to debugfs_rename() will give a new name to an existing debugfs -file, possibly in a different directory. The new_name must not exist prior -to the call; the return value is old_dentry with updated information. +A call to debugfs_change_name() will give a new name to an existing debugfs +file, always in the same directory. The new_name must not exist prior +to the call; the return value is 0 on success and -E... on failuer. Symbolic links can be created with debugfs_create_symlink(). There is one important thing that all debugfs users must take into account: diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c index b19492a7f6ad..8adbec7c5084 100644 --- a/drivers/net/bonding/bond_debugfs.c +++ b/drivers/net/bonding/bond_debugfs.c @@ -63,13 +63,8 @@ void bond_debug_unregister(struct bonding *bond) void bond_debug_reregister(struct bonding *bond) { - struct dentry *d; - - d = debugfs_rename(bonding_debug_root, bond->debug_dir, - bonding_debug_root, bond->dev->name); - if (!IS_ERR(d)) { - bond->debug_dir = d; - } else { + int err = debugfs_change_name(bond->debug_dir, "%s", bond->dev->name); + if (err) { netdev_warn(bond->dev, "failed to reregister, so just unregister old one\n"); bond_debug_unregister(bond); } diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c index b0a6c96b6ef4..b35808d3d07f 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c @@ -505,21 +505,6 @@ void xgbe_debugfs_exit(struct xgbe_prv_data *pdata) void xgbe_debugfs_rename(struct xgbe_prv_data *pdata) { - char *buf; - - if (!pdata->xgbe_debugfs) - return; - - buf = kasprintf(GFP_KERNEL, "amd-xgbe-%s", pdata->netdev->name); - if (!buf) - return; - - if (!strcmp(pdata->xgbe_debugfs->d_name.name, buf)) - goto out; - - debugfs_rename(pdata->xgbe_debugfs->d_parent, pdata->xgbe_debugfs, - pdata->xgbe_debugfs->d_parent, buf); - -out: - kfree(buf); + debugfs_change_name(pdata->xgbe_debugfs, + "amd-xgbe-%s", pdata->netdev->name); } diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c index 25bf6ec44289..a1bada9eaaf6 100644 --- a/drivers/net/ethernet/marvell/skge.c +++ b/drivers/net/ethernet/marvell/skge.c @@ -3742,10 +3742,7 @@ static int skge_device_event(struct notifier_block *unused, skge = netdev_priv(dev); switch (event) { case NETDEV_CHANGENAME: - if (skge->debugfs) - skge->debugfs = debugfs_rename(skge_debug, - skge->debugfs, - skge_debug, dev->name); + debugfs_change_name(skge->debugfs, "%s", dev->name); break; case NETDEV_GOING_DOWN: diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c index 988fa28cfb5f..d7121c836508 100644 --- a/drivers/net/ethernet/marvell/sky2.c +++ b/drivers/net/ethernet/marvell/sky2.c @@ -4494,10 +4494,7 @@ static int sky2_device_event(struct notifier_block *unused, switch (event) { case NETDEV_CHANGENAME: - if (sky2->debugfs) { - sky2->debugfs = debugfs_rename(sky2_debug, sky2->debugfs, - sky2_debug, dev->name); - } + debugfs_change_name(sky2->debugfs, "%s", dev->name); break; case NETDEV_GOING_DOWN: diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index c81ea8cdfe6e..82e2908016bd 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -6489,11 +6489,7 @@ static int stmmac_device_event(struct notifier_block *unused, switch (event) { case NETDEV_CHANGENAME: - if (priv->dbgfs_dir) - priv->dbgfs_dir = debugfs_rename(stmmac_fs_dir, - priv->dbgfs_dir, - stmmac_fs_dir, - dev->name); + debugfs_change_name(priv->dbgfs_dir, "%s", dev->name); break; } done: diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c index 105de7c3274a..8fc6238b1728 100644 --- a/drivers/opp/debugfs.c +++ b/drivers/opp/debugfs.c @@ -217,7 +217,7 @@ static void opp_migrate_dentry(struct opp_device *opp_dev, { struct opp_device *new_dev = NULL, *iter; const struct device *dev; - struct dentry *dentry; + int err; /* Look for next opp-dev */ list_for_each_entry(iter, &opp_table->dev_list, node) @@ -234,16 +234,14 @@ static void opp_migrate_dentry(struct opp_device *opp_dev, opp_set_dev_name(dev, opp_table->dentry_name); - dentry = debugfs_rename(rootdir, opp_dev->dentry, rootdir, - opp_table->dentry_name); - if (IS_ERR(dentry)) { + err = debugfs_change_name(opp_dev->dentry, "%s", opp_table->dentry_name); + if (err) { dev_err(dev, "%s: Failed to rename link from: %s to %s\n", __func__, dev_name(opp_dev->dev), dev_name(dev)); return; } - new_dev->dentry = dentry; - opp_table->dentry = dentry; + new_dev->dentry = opp_table->dentry = opp_dev->dentry; } /** diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 51d4c3e9d422..75715d8877ee 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -830,76 +830,70 @@ void debugfs_lookup_and_remove(const char *name, struct dentry *parent) EXPORT_SYMBOL_GPL(debugfs_lookup_and_remove); /** - * debugfs_rename - rename a file/directory in the debugfs filesystem - * @old_dir: a pointer to the parent dentry for the renamed object. This - * should be a directory dentry. - * @old_dentry: dentry of an object to be renamed. - * @new_dir: a pointer to the parent dentry where the object should be - * moved. This should be a directory dentry. - * @new_name: a pointer to a string containing the target name. + * debugfs_change_name - rename a file/directory in the debugfs filesystem + * @dentry: dentry of an object to be renamed. + * @fmt: format for new name * * This function renames a file/directory in debugfs. The target must not * exist for rename to succeed. * - * This function will return a pointer to old_dentry (which is updated to - * reflect renaming) if it succeeds. If an error occurs, ERR_PTR(-ERROR) - * will be returned. + * This function will return 0 on success and -E... on failure. * * If debugfs is not enabled in the kernel, the value -%ENODEV will be * returned. */ -struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, - struct dentry *new_dir, const char *new_name) +int __printf(2, 3) debugfs_change_name(struct dentry *dentry, const char *fmt, ...) { - int error; - struct dentry *dentry = NULL, *trap; + int error = 0; + const char *new_name; struct name_snapshot old_name; + struct dentry *parent, *target; + struct inode *dir; + va_list ap; - if (IS_ERR(old_dir)) - return old_dir; - if (IS_ERR(new_dir)) - return new_dir; - if (IS_ERR_OR_NULL(old_dentry)) - return old_dentry; - - trap = lock_rename(new_dir, old_dir); - /* Source or destination directories don't exist? */ - if (d_really_is_negative(old_dir) || d_really_is_negative(new_dir)) - goto exit; - /* Source does not exist, cyclic rename, or mountpoint? */ - if (d_really_is_negative(old_dentry) || old_dentry == trap || - d_mountpoint(old_dentry)) - goto exit; - dentry = lookup_one_len(new_name, new_dir, strlen(new_name)); - /* Lookup failed, cyclic rename or target exists? */ - if (IS_ERR(dentry) || dentry == trap || d_really_is_positive(dentry)) - goto exit; - - take_dentry_name_snapshot(&old_name, old_dentry); - - error = simple_rename(&nop_mnt_idmap, d_inode(old_dir), old_dentry, - d_inode(new_dir), dentry, 0); - if (error) { - release_dentry_name_snapshot(&old_name); - goto exit; + if (IS_ERR_OR_NULL(dentry)) + return 0; + + va_start(ap, fmt); + new_name = kvasprintf_const(GFP_KERNEL, fmt, ap); + va_end(ap); + if (!new_name) + return -ENOMEM; + + parent = dget_parent(dentry); + dir = d_inode(parent); + inode_lock(dir); + + take_dentry_name_snapshot(&old_name, dentry); + + if (WARN_ON_ONCE(dentry->d_parent != parent)) { + error = -EINVAL; + goto out; + } + if (strcmp(old_name.name.name, new_name) == 0) + goto out; + target = lookup_one_len(new_name, parent, strlen(new_name)); + if (IS_ERR(target)) { + error = PTR_ERR(target); + goto out; } - d_move(old_dentry, dentry); - fsnotify_move(d_inode(old_dir), d_inode(new_dir), &old_name.name, - d_is_dir(old_dentry), - NULL, old_dentry); + if (d_really_is_positive(target)) { + dput(target); + error = -EINVAL; + goto out; + } + simple_rename_timestamp(dir, dentry, dir, target); + d_move(dentry, target); + dput(target); + fsnotify_move(dir, dir, &old_name.name, d_is_dir(dentry), NULL, dentry); +out: release_dentry_name_snapshot(&old_name); - unlock_rename(new_dir, old_dir); - dput(dentry); - return old_dentry; -exit: - if (dentry && !IS_ERR(dentry)) - dput(dentry); - unlock_rename(new_dir, old_dir); - if (IS_ERR(dentry)) - return dentry; - return ERR_PTR(-EINVAL); + inode_unlock(dir); + dput(parent); + kfree_const(new_name); + return error; } -EXPORT_SYMBOL_GPL(debugfs_rename); +EXPORT_SYMBOL_GPL(debugfs_change_name); /** * debugfs_initialized - Tells whether debugfs has been registered diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 68e9c6cbd835..fa2568b4380d 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -175,8 +175,7 @@ ssize_t debugfs_attr_write(struct file *file, const char __user *buf, ssize_t debugfs_attr_write_signed(struct file *file, const char __user *buf, size_t len, loff_t *ppos); -struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, - struct dentry *new_dir, const char *new_name); +int debugfs_change_name(struct dentry *dentry, const char *fmt, ...) __printf(2, 3); void debugfs_create_u8(const char *name, umode_t mode, struct dentry *parent, u8 *value); @@ -361,10 +360,10 @@ static inline ssize_t debugfs_attr_write_signed(struct file *file, return -ENODEV; } -static inline struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry, - struct dentry *new_dir, char *new_name) +static inline int __printf(2, 3) debugfs_change_name(struct dentry *dentry, + const char *fmt, ...) { - return ERR_PTR(-ENODEV); + return -ENODEV; } static inline void debugfs_create_u8(const char *name, umode_t mode, diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c index 4a85b94d12ce..794bd433cce0 100644 --- a/mm/shrinker_debug.c +++ b/mm/shrinker_debug.c @@ -195,8 +195,6 @@ int shrinker_debugfs_add(struct shrinker *shrinker) int shrinker_debugfs_rename(struct shrinker *shrinker, const char *fmt, ...) { - struct dentry *entry; - char buf[128]; const char *new, *old; va_list ap; int ret = 0; @@ -213,18 +211,8 @@ int shrinker_debugfs_rename(struct shrinker *shrinker, const char *fmt, ...) old = shrinker->name; shrinker->name = new; - if (shrinker->debugfs_entry) { - snprintf(buf, sizeof(buf), "%s-%d", shrinker->name, - shrinker->debugfs_id); - - entry = debugfs_rename(shrinker_debugfs_root, - shrinker->debugfs_entry, - shrinker_debugfs_root, buf); - if (IS_ERR(entry)) - ret = PTR_ERR(entry); - else - shrinker->debugfs_entry = entry; - } + ret = debugfs_change_name(shrinker->debugfs_entry, "%s-%d", + shrinker->name, shrinker->debugfs_id); mutex_unlock(&shrinker_mutex); diff --git a/net/hsr/hsr_debugfs.c b/net/hsr/hsr_debugfs.c index 1a195efc79cd..5b2cfac3b2ba 100644 --- a/net/hsr/hsr_debugfs.c +++ b/net/hsr/hsr_debugfs.c @@ -57,14 +57,11 @@ DEFINE_SHOW_ATTRIBUTE(hsr_node_table); void hsr_debugfs_rename(struct net_device *dev) { struct hsr_priv *priv = netdev_priv(dev); - struct dentry *d; + int err; - d = debugfs_rename(hsr_debugfs_root_dir, priv->node_tbl_root, - hsr_debugfs_root_dir, dev->name); - if (IS_ERR(d)) + err = debugfs_change_name(priv->node_tbl_root, "%s", dev->name); + if (err) netdev_warn(dev, "failed to rename\n"); - else - priv->node_tbl_root = d; } /* hsr_debugfs_init - create hsr node_table file for dumping diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c index a9bc2fd59f55..9fa38c489edc 100644 --- a/net/mac80211/debugfs_netdev.c +++ b/net/mac80211/debugfs_netdev.c @@ -1025,16 +1025,7 @@ void ieee80211_debugfs_remove_netdev(struct ieee80211_sub_if_data *sdata) void ieee80211_debugfs_rename_netdev(struct ieee80211_sub_if_data *sdata) { - struct dentry *dir; - char buf[10 + IFNAMSIZ]; - - dir = sdata->vif.debugfs_dir; - - if (IS_ERR_OR_NULL(dir)) - return; - - sprintf(buf, "netdev:%s", sdata->name); - debugfs_rename(dir->d_parent, dir, dir->d_parent, buf); + debugfs_change_name(sdata->vif.debugfs_dir, "netdev:%s", sdata->name); } void ieee80211_debugfs_recreate_netdev(struct ieee80211_sub_if_data *sdata, diff --git a/net/wireless/core.c b/net/wireless/core.c index afbdc549fb4a..9130cb872ed3 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -143,10 +143,7 @@ int cfg80211_dev_rename(struct cfg80211_registered_device *rdev, if (result) return result; - if (!IS_ERR_OR_NULL(rdev->wiphy.debugfsdir)) - debugfs_rename(rdev->wiphy.debugfsdir->d_parent, - rdev->wiphy.debugfsdir, - rdev->wiphy.debugfsdir->d_parent, newname); + debugfs_change_name(rdev->wiphy.debugfsdir, "%s", newname); nl80211_notify_wiphy(rdev, NL80211_CMD_NEW_WIPHY); -- cgit v1.2.3