lkml.org 
[lkml]   [2016]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v3 3/7] debugfs: add support for self-protecting attribute file fops


    On Sun, 14 Feb 2016, Nicolai Stange wrote:

    > In order to protect them against file removal issues, debugfs_create_file()
    > creates a lifetime managing proxy around each struct file_operations
    > handed in.
    >
    > In cases where this struct file_operations is able to manage file lifetime
    > by itself already, the proxy created by debugfs is a waste of resources.
    >
    > The most common class of struct file_operations given to debugfs are those
    > defined by means of the DEFINE_SIMPLE_ATTRIBUTE() macro.
    >
    > Introduce a DEFINE_DEBUGFS_ATTRIBUTE() macro to allow any
    > struct file_operations of this class to be easily made file lifetime aware
    > and thus, to be operated unproxied.
    >
    > Specifically, introduce debugfs_attr_read() and debugfs_attr_write()
    > which wrap simple_attr_read() and simple_attr_write() under the protection
    > of a debugfs_use_file_start()/debugfs_use_file_finish() pair.
    >
    > Make DEFINE_DEBUGFS_ATTRIBUTE() set the defined struct file_operations'
    > ->read() and ->write() members to these wrappers.
    >
    > Export debugfs_create_file_unsafe() in order to allow debugfs users to
    > create their files in non-proxying operation mode.
    >
    > Finally, add a Coccinelle script chasing down possible candidates
    > for a DEFINE_SIMPLE_ATTRIBUTE()/debugfs_create_file() to
    > DEFINE_DEBUGFS_ATTRIBUTE()/debugfs_create_file_unsafe() migration.
    >
    > Signed-off-by: Nicolai Stange <nicstange@gmail.com>
    > ---
    > fs/debugfs/file.c | 28 +++++++++
    > fs/debugfs/inode.c | 28 +++++++++
    > include/linux/debugfs.h | 26 +++++++++
    > .../api/debugfs/debugfs_simple_attr.cocci | 68 ++++++++++++++++++++++

    Shouldn't the .cocci file be in a different patch, since it has a
    different maintainer?

    julia

    > 4 files changed, 150 insertions(+)
    > create mode 100644 scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
    >
    > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
    > index f638dbc..2da5fb0 100644
    > --- a/fs/debugfs/file.c
    > +++ b/fs/debugfs/file.c
    > @@ -285,6 +285,34 @@ const struct file_operations debugfs_full_proxy_file_operations = {
    > .open = full_proxy_open,
    > };
    >
    > +ssize_t debugfs_attr_read(struct file *file, char __user *buf,
    > + size_t len, loff_t *ppos)
    > +{
    > + ssize_t ret;
    > + int srcu_idx;
    > +
    > + ret = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
    > + if (likely(!ret))
    > + ret = simple_attr_read(file, buf, len, ppos);
    > + debugfs_use_file_finish(srcu_idx);
    > + return ret;
    > +}
    > +EXPORT_SYMBOL_GPL(debugfs_attr_read);
    > +
    > +ssize_t debugfs_attr_write(struct file *file, const char __user *buf,
    > + size_t len, loff_t *ppos)
    > +{
    > + ssize_t ret;
    > + int srcu_idx;
    > +
    > + ret = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
    > + if (likely(!ret))
    > + ret = simple_attr_write(file, buf, len, ppos);
    > + debugfs_use_file_finish(srcu_idx);
    > + return ret;
    > +}
    > +EXPORT_SYMBOL_GPL(debugfs_attr_write);
    > +
    > static struct dentry *debugfs_create_mode(const char *name, umode_t mode,
    > struct dentry *parent, void *value,
    > const struct file_operations *fops,
    > diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
    > index 42a9b34..f95e355 100644
    > --- a/fs/debugfs/inode.c
    > +++ b/fs/debugfs/inode.c
    > @@ -368,6 +368,33 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode,
    > }
    > EXPORT_SYMBOL_GPL(debugfs_create_file);
    >
    > +/**
    > + * debugfs_create_file_unsafe - 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.
    > + *
    > + * debugfs_create_file_unsafe() is completely analogous to
    > + * debugfs_create_file(), the only difference being that the fops
    > + * handed it will not get protected against file removals by the
    > + * debugfs core.
    > + *
    > + * It is your responsibility to protect your struct file_operation
    > + * methods against file removals by means of debugfs_use_file_start()
    > + * and debugfs_use_file_finish(). ->open() is still protected by
    > + * debugfs though.
    > + *
    > + * Any struct file_operations defined by means of
    > + * DEFINE_DEBUGFS_ATTRIBUTE() is protected against file removals and
    > + * thus, may be used here.
    > + */
    > struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode,
    > struct dentry *parent, void *data,
    > const struct file_operations *fops)
    > @@ -378,6 +405,7 @@ struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode,
    > &debugfs_noop_file_operations,
    > fops);
    > }
    > +EXPORT_SYMBOL_GPL(debugfs_create_file_unsafe);
    >
    > /**
    > * debugfs_create_file_size - create a file in the debugfs filesystem
    > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
    > index 6d45ae6..c880fe9 100644
    > --- a/include/linux/debugfs.h
    > +++ b/include/linux/debugfs.h
    > @@ -50,6 +50,9 @@ extern struct srcu_struct debugfs_srcu;
    > 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_unsafe(const char *name, umode_t mode,
    > + struct dentry *parent, void *data,
    > + const struct file_operations *fops);
    >
    > struct dentry *debugfs_create_file_size(const char *name, umode_t mode,
    > struct dentry *parent, void *data,
    > @@ -74,6 +77,26 @@ int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx)
    >
    > void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu);
    >
    > +ssize_t debugfs_attr_read(struct file *file, char __user *buf,
    > + size_t len, loff_t *ppos);
    > +ssize_t debugfs_attr_write(struct file *file, const char __user *buf,
    > + size_t len, loff_t *ppos);
    > +
    > +#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \
    > +static int __fops ## _open(struct inode *inode, struct file *file) \
    > +{ \
    > + __simple_attr_check_format(__fmt, 0ull); \
    > + return simple_attr_open(inode, file, __get, __set, __fmt); \
    > +} \
    > +static const struct file_operations __fops = { \
    > + .owner = THIS_MODULE, \
    > + .open = __fops ## _open, \
    > + .release = simple_attr_release, \
    > + .read = debugfs_attr_read, \
    > + .write = debugfs_attr_write, \
    > + .llseek = generic_file_llseek, \
    > +}
    > +
    > struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry,
    > struct dentry *new_dir, const char *new_name);
    >
    > @@ -183,6 +206,9 @@ static int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx)
    > static void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu)
    > { }
    >
    > +#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \
    > + static const struct file_operations __fops = { 0 }
    > +
    > static inline struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry,
    > struct dentry *new_dir, char *new_name)
    > {
    > diff --git a/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
    > new file mode 100644
    > index 0000000..bdc418d
    > --- /dev/null
    > +++ b/scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
    > @@ -0,0 +1,68 @@
    > +///
    > +/// Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
    > +/// for debugfs files.
    > +///
    > +/// Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
    > +/// imposes some significant overhead as compared to
    > +/// DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
    > +///
    > +// Copyright (C): 2016 Nicolai Stange
    > +// Options: --no-includes
    > +//
    > +
    > +virtual context
    > +virtual patch
    > +virtual org
    > +virtual report
    > +
    > +@dsa@
    > +declarer name DEFINE_SIMPLE_ATTRIBUTE;
    > +identifier dsa_fops;
    > +expression dsa_get, dsa_set, dsa_fmt;
    > +position p;
    > +@@
    > +DEFINE_SIMPLE_ATTRIBUTE@p(dsa_fops, dsa_get, dsa_set, dsa_fmt);
    > +
    > +@dcf@
    > +expression name, mode, parent, data;
    > +identifier dsa.dsa_fops;
    > +@@
    > +debugfs_create_file(name, mode, parent, data, &dsa_fops)
    > +
    > +
    > +@context_dsa depends on context && dcf@
    > +declarer name DEFINE_DEBUGFS_ATTRIBUTE;
    > +identifier dsa.dsa_fops;
    > +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt;
    > +@@
    > +* DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt);
    > +
    > +
    > +@patch_dcf depends on patch expression@
    > +expression name, mode, parent, data;
    > +identifier dsa.dsa_fops;
    > +@@
    > +- debugfs_create_file(name, mode, parent, data, &dsa_fops)
    > ++ debugfs_create_file_unsafe(name, mode, parent, data, &dsa_fops)
    > +
    > +@patch_dsa depends on patch_dcf && patch@
    > +identifier dsa.dsa_fops;
    > +expression dsa.dsa_get, dsa.dsa_set, dsa.dsa_fmt;
    > +@@
    > +- DEFINE_SIMPLE_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt);
    > ++ DEFINE_DEBUGFS_ATTRIBUTE(dsa_fops, dsa_get, dsa_set, dsa_fmt);
    > +
    > +
    > +@script:python depends on org && dcf@
    > +fops << dsa.dsa_fops;
    > +p << dsa.p;
    > +@@
    > +msg="%s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops)
    > +coccilib.org.print_todo(p[0], msg)
    > +
    > +@script:python depends on report && dcf@
    > +fops << dsa.dsa_fops;
    > +p << dsa.p;
    > +@@
    > +msg="WARNING: %s should be defined with DEFINE_DEBUGFS_ATTRIBUTE" % (fops)
    > +coccilib.report.print_report(p[0], msg)
    > --
    > 2.7.1
    >

    \
     
     \ /
      Last update: 2016-02-14 08:41    [W:4.549 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site