Skip to content

Commit

Permalink
fs: debugfs: differentiate short fops with proxy ops
Browse files Browse the repository at this point in the history 
Geert reported that my previous short fops debugfs changes
broke m68k, because it only has mandatory alignement of two,
so we can't stash the "is it short" information into the
pointer (as we already did with the "is it real" bit.)

Instead, exploit the fact that debugfs_file_get() called on
an already open file will already find that the fsdata is
no longer the real fops but rather the allocated data that
already distinguishes full/short ops, so only open() needs
to be able to distinguish. We can achieve that by using two
different open functions.

Unfortunately this requires another set of full file ops,
increasing the size by 536 bytes (x86-64), but that's still
a reasonable trade-off given that only converting some of
the wireless stack gained over 28k. This brings the total
cost of this to around 1k, for wins of 28k (all x86-64).

Reported-and-tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Link: https://lore.kernel.org/CAMuHMdWu_9-L2Te101w8hU7H_2yobJFPXSwwUmGHSJfaPWDKiQ@mail.gmail.com
Fixes: 8dc6d81 ("debugfs: add small file operations for most files")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Link: https://lore.kernel.org/r/20241129121536.30989-2-johannes@sipsolutions.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Johannes Berg authored and Greg Kroah-Hartman committed Jan 7, 2025
1 parent 78d4f34 commit f8f2589
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 34 deletions.
72 changes: 50 additions & 22 deletions fs/debugfs/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,13 @@ const struct file_operations *debugfs_real_fops(const struct file *filp)
}
EXPORT_SYMBOL_GPL(debugfs_real_fops);

/**
* debugfs_file_get - mark the beginning of file data access
* @dentry: the dentry object whose data is being accessed.
*
* Up to a matching call to debugfs_file_put(), any successive call
* into the file removing functions debugfs_remove() and
* debugfs_remove_recursive() will block. Since associated private
* file data may only get freed after a successful return of any of
* the removal functions, you may safely access it after a successful
* call to debugfs_file_get() without worrying about lifetime issues.
*
* If -%EIO is returned, the file has already been removed and thus,
* it is not safe to access any of its data. If, on the other hand,
* it is allowed to access the file data, zero is returned.
*/
int debugfs_file_get(struct dentry *dentry)
enum dbgfs_get_mode {
DBGFS_GET_ALREADY,
DBGFS_GET_REGULAR,
DBGFS_GET_SHORT,
};

static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode)
{
struct debugfs_fsdata *fsd;
void *d_fsd;
Expand All @@ -96,15 +87,17 @@ int debugfs_file_get(struct dentry *dentry)
if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) {
fsd = d_fsd;
} else {
if (WARN_ON(mode == DBGFS_GET_ALREADY))
return -EINVAL;

fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
if (!fsd)
return -ENOMEM;

if ((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT) {
if (mode == DBGFS_GET_SHORT) {
fsd->real_fops = NULL;
fsd->short_fops = (void *)((unsigned long)d_fsd &
~(DEBUGFS_FSDATA_IS_REAL_FOPS_BIT |
DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT));
~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
} else {
fsd->real_fops = (void *)((unsigned long)d_fsd &
~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
Expand Down Expand Up @@ -138,6 +131,26 @@ int debugfs_file_get(struct dentry *dentry)

return 0;
}

/**
* debugfs_file_get - mark the beginning of file data access
* @dentry: the dentry object whose data is being accessed.
*
* Up to a matching call to debugfs_file_put(), any successive call
* into the file removing functions debugfs_remove() and
* debugfs_remove_recursive() will block. Since associated private
* file data may only get freed after a successful return of any of
* the removal functions, you may safely access it after a successful
* call to debugfs_file_get() without worrying about lifetime issues.
*
* If -%EIO is returned, the file has already been removed and thus,
* it is not safe to access any of its data. If, on the other hand,
* it is allowed to access the file data, zero is returned.
*/
int debugfs_file_get(struct dentry *dentry)
{
return __debugfs_file_get(dentry, DBGFS_GET_ALREADY);
}
EXPORT_SYMBOL_GPL(debugfs_file_get);

/**
Expand Down Expand Up @@ -424,15 +437,16 @@ static void __full_proxy_fops_init(struct file_operations *proxy_fops,
proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl;
}

static int full_proxy_open(struct inode *inode, struct file *filp)
static int full_proxy_open(struct inode *inode, struct file *filp,
enum dbgfs_get_mode mode)
{
struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops;
struct file_operations *proxy_fops = NULL;
struct debugfs_fsdata *fsd;
int r;

r = debugfs_file_get(dentry);
r = __debugfs_file_get(dentry, mode);
if (r)
return r == -EIO ? -ENOENT : r;

Expand Down Expand Up @@ -491,8 +505,22 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
return r;
}

static int full_proxy_open_regular(struct inode *inode, struct file *filp)
{
return full_proxy_open(inode, filp, DBGFS_GET_REGULAR);
}

const struct file_operations debugfs_full_proxy_file_operations = {
.open = full_proxy_open,
.open = full_proxy_open_regular,
};

static int full_proxy_open_short(struct inode *inode, struct file *filp)
{
return full_proxy_open(inode, filp, DBGFS_GET_SHORT);
}

const struct file_operations debugfs_full_short_proxy_file_operations = {
.open = full_proxy_open_short,
};

ssize_t debugfs_attr_read(struct file *file, char __user *buf,
Expand Down
11 changes: 4 additions & 7 deletions fs/debugfs/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,7 @@ struct dentry *debugfs_create_file_full(const char *name, umode_t mode,
const struct file_operations *fops)
{
if (WARN_ON((unsigned long)fops &
(DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT |
DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)))
DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))
return ERR_PTR(-EINVAL);

return __debugfs_create_file(name, mode, parent, data,
Expand All @@ -471,15 +470,13 @@ struct dentry *debugfs_create_file_short(const char *name, umode_t mode,
const struct debugfs_short_fops *fops)
{
if (WARN_ON((unsigned long)fops &
(DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT |
DEBUGFS_FSDATA_IS_REAL_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 :
fops ? &debugfs_full_short_proxy_file_operations :
&debugfs_noop_file_operations,
(const void *)((unsigned long)fops |
DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT));
fops);
}
EXPORT_SYMBOL_GPL(debugfs_create_file_short);

Expand Down
6 changes: 1 addition & 5 deletions fs/debugfs/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ struct file_operations;
extern const struct file_operations debugfs_noop_file_operations;
extern const struct file_operations debugfs_open_proxy_file_operations;
extern const struct file_operations debugfs_full_proxy_file_operations;
extern const struct file_operations debugfs_full_short_proxy_file_operations;

struct debugfs_fsdata {
const struct file_operations *real_fops;
Expand All @@ -40,11 +41,6 @@ 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)
Expand Down

0 comments on commit f8f2589

Please sign in to comment.