On Fri, Oct 30, 2020 at 06:30:49PM +1100, Anand K. Mistry wrote:
> On Fri, 30 Oct 2020 at 18:20, Greg Kroah-Hartman
> <gre...@linuxfoundation.org> wrote:
> >
> > On Fri, Oct 30, 2020 at 06:04:42PM +1100, Anand K Mistry wrote:
> > > This mirrors support for exporting atomic_t values.
> > >
> > > Signed-off-by: Anand K Mistry <amis...@google.com>
> > >
> > > ---
> > >
> > > fs/debugfs/file.c | 37 +++++++++++++++++++++++++++++++++++++
> > > include/linux/debugfs.h | 6 ++++++
> > > 2 files changed, 43 insertions(+)
> > >
> > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > > index a768a09430c3..798bd3bdedec 100644
> > > --- a/fs/debugfs/file.c
> > > +++ b/fs/debugfs/file.c
> > > @@ -770,6 +770,43 @@ void debugfs_create_atomic_t(const char *name,
> > > umode_t mode,
> > > }
> > > EXPORT_SYMBOL_GPL(debugfs_create_atomic_t);
> > >
> > > +static int debugfs_atomic64_t_set(void *data, u64 val)
> > > +{
> > > + atomic64_set((atomic64_t *)data, val);
> > > + return 0;
> > > +}
> > > +static int debugfs_atomic64_t_get(void *data, u64 *val)
> > > +{
> > > + *val = atomic64_read((atomic64_t *)data);
> > > + return 0;
> > > +}
> > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic64_t, debugfs_atomic64_t_get,
> > > + debugfs_atomic64_t_set, "%lld\n");
> > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic64_t_ro, debugfs_atomic64_t_get,
> > > NULL,
> > > + "%lld\n");
> > > +DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic64_t_wo, NULL,
> > > debugfs_atomic64_t_set,
> > > + "%lld\n");
> > > +
> > > +/**
> > > + * debugfs_create_atomic64_t - create a debugfs file that is used to
> > > read and
> > > + * write an atomic64_t value
> > > + * @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.
> > > + * @value: a pointer to the variable that the file should read to and
> > > write
> > > + * from.
> > > + */
> > > +void debugfs_create_atomic64_t(const char *name, umode_t mode,
> > > + struct dentry *parent, atomic64_t *value)
> > > +{
> > > + debugfs_create_mode_unsafe(name, mode, parent, value,
> > > + &fops_atomic64_t, &fops_atomic64_t_ro,
> > > + &fops_atomic64_t_wo);
> > > +}
> > > +EXPORT_SYMBOL_GPL(debugfs_create_atomic64_t);
> > > +
> > > ssize_t debugfs_read_file_bool(struct file *file, char __user *user_buf,
> > > size_t count, loff_t *ppos)
> > > {
> > > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> > > index 851dd1f9a8a5..0fac84c53eab 100644
> > > --- a/include/linux/debugfs.h
> > > +++ b/include/linux/debugfs.h
> > > @@ -126,6 +126,8 @@ void debugfs_create_size_t(const char *name, umode_t
> > > mode,
> > > struct dentry *parent, size_t *value);
> > > void debugfs_create_atomic_t(const char *name, umode_t mode,
> > > struct dentry *parent, atomic_t *value);
> > > +void debugfs_create_atomic64_t(const char *name, umode_t mode,
> > > + struct dentry *parent, atomic64_t
> > > *value);
> > > struct dentry *debugfs_create_bool(const char *name, umode_t mode,
> > > struct dentry *parent, bool *value);
> > >
> > > @@ -291,6 +293,10 @@ static inline void debugfs_create_atomic_t(const
> > > char *name, umode_t mode,
> > > atomic_t *value)
> > > { }
> > >
> > > +static inline void debugfs_create_atomic64_t(const char *name, umode_t
> > > mode,
> > > + struct dentry *parent,
> > > atomic64_t *value)
> > > +{ }
> > > +
> > > static inline struct dentry *debugfs_create_bool(const char *name,
> > > umode_t mode,
> > > struct dentry *parent,
> > > bool *value)
> >
> > Looks good, but where is the user of this code? I can't add new apis
> > without a user.
>
> Fair enough. Right now, the user is just some local
> debugging/performance measuring which will never be upstreamed.
> Happy to let this drop.
Now dropped!
> > And are you _SURE_ you want to be using an atomic64_t in the first
> > place? We are starting to reduce the "raw" usage of atomic variables as
> > almost no one needs them, they should be using something else instead,
> > or just a u64 as atomics are not needed for simple statistics.
>
> I understand, and would generally never use atomics in real code. I
> used an atomic since I wanted accuracy (for some of the benchmarks I
> want to run) but can't use anything that blocks (spinlock/mutex) since
> the code is somewhere inside the scheduler.
You can't be "accurate" for a single value from a single file read by
userspace as you don't know if it changed right after you read the value :)
Again, people abuse atomic values thinking they are something they
really aren't...
thanks,
greg k-h
