On Fri, 5 Feb 2021 16:29:41 +0100 Greg KH <greg@xxxxxxxxx> wrote:
> On Thu, Feb 04, 2021 at 04:31:43PM +0100, SeongJae Park wrote:
> > From: SeongJae Park <sjpark@xxxxxxxxx>
> >
> > DAMON is designed to be used by kernel space code such as the memory
> > management subsystems, and therefore it provides only kernel space API.
> > That said, letting the user space control DAMON could provide some
> > benefits to them. For example, it will allow user space to analyze
> > their specific workloads and make their own special optimizations.
> >
> > For such cases, this commit implements a simple DAMON application kernel
> > module, namely 'damon-dbgfs', which merely wraps the DAMON api and
> > exports those to the user space via the debugfs.
> >
> > 'damon-dbgfs' exports three files, ``attrs``, ``target_ids``, and
> > ``monitor_on`` under its debugfs directory, ``<debugfs>/damon/``.
[...]
> > ---
> > include/linux/damon.h | 3 +
> > mm/damon/Kconfig | 9 +
> > mm/damon/Makefile | 1 +
> > mm/damon/core.c | 47 +++++
> > mm/damon/dbgfs.c | 387 ++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 447 insertions(+)
> > create mode 100644 mm/damon/dbgfs.c
[...]
> > diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
> > new file mode 100644
> > index 000000000000..db15380737d1
> > --- /dev/null
> > +++ b/mm/damon/dbgfs.c
[...]
> > +
> > +static int dbgfs_fill_ctx_dir(struct dentry *dir, struct damon_ctx *ctx)
> > +{
> > + const char * const file_names[] = {"attrs", "target_ids"};
> > + const struct file_operations *fops[] = {&attrs_fops, &target_ids_fops};
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(file_names); i++) {
> > + if (!debugfs_create_file(file_names[i], 0600, dir,
> > + ctx, fops[i])) {
> > + pr_err("failed to create %s file\n", file_names[i]);
> > + return -ENOMEM;
>
> No need to check the return value of this function, just keep going and
> ignore it as there's nothing to do and kernel code should not do
> different things based on the output of any debugfs calls.
>
> Also, this check is totally wrong and doesn't do what you think it is
> doing...
Ok, I will drop the check.
>
> > +static int __init __damon_dbgfs_init(void)
> > +{
> > + struct dentry *dbgfs_root;
> > + const char * const file_names[] = {"monitor_on"};
> > + const struct file_operations *fops[] = {&monitor_on_fops};
> > + int i;
> > +
> > + dbgfs_root = debugfs_create_dir("damon", NULL);
> > + if (IS_ERR(dbgfs_root)) {
> > + pr_err("failed to create the dbgfs dir\n");
> > + return PTR_ERR(dbgfs_root);
>
> Again, no need to check anything, just pass the result of a debugfs call
> back into another one just fine.
Ok.
>
> > + }
> > +
> > + for (i = 0; i < ARRAY_SIZE(file_names); i++) {
> > + if (!debugfs_create_file(file_names[i], 0600, dbgfs_root,
> > + NULL, fops[i])) {
>
> Again, this isn't checking what you think it is, so please don't do it.
Got it.
I will fix those as you suggested in the next version.
Thanks,
SeongJae Park
>
> thanks,
>
> greg k-h
