lkml.org 
[lkml]   [2019]   [Dec]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] iio: core: use debugfs_remove_recursive() on IIO unload
On Mon, 16 Dec 2019 07:43:30 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Sun, 2019-12-15 at 16:25 +0000, Jonathan Cameron wrote:
> > [External]
> >
> > On Wed, 11 Dec 2019 16:59:03 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > From: Lars-Peter Clausen <lars@metafoo.de>
> > >
> > > The debugfs dir may have multiple files/dirs attached to it. Use
> > > debugfs_remove_recursive() to clean it up when the IIO module unloads.
> > >
> > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > >
> > > Note: note sure if this needs a Fixes tag. It's been in here for ages
> > > now,
> > > so it may not be a big issue.
> >
> > There is some argument in favour of doing this as a defensive measure,
> > but
> > in current IIO debugfs directories are only created by drivers that call
> > (indirectly) iio_device_register_debugfs and those must call
> > (again indirectly) iio_device_unregister_debugfs which has called
> > the recursive free on everything in their directories. So unless
> > something
> > very odd is going on this isn't a bug.
> >
> > Am I missing something?
>
> So, I admit my note is open for interpretation.
> [ Rergarding the "Fixes" tag, I'm still not super-clear on when/why/how
> it's needed. I'll get there soon though. ]
>
> This patch has been in our tree since 2012, and I'm not sure why it's using
> debugfs_remove_recursive() vs plain debugfs_remove().
>
> I sometimes take a look at these old patches, try to make some sense of
> them and upstream them, or override them with the version of upstream code.
> My intent is to sync ADI code [for IIO] closer to upstream. Which would
> make it easier to track any divergence.
>
> We're not seeing any bug with/without this.
> I guess my note about the "Fixes" tag may be a bit clumsy.
>
> From my side, both debugfs_remove_recursive() & plain debugfs_remove() are
> fine.
I'd rather leave it be as it kind of avoids people thinking they can get away
without cleaning up properly at the higher levels.

Thanks,

Jonathan

>
> Thanks
> Alex
>
> >
> >
> > > drivers/iio/industrialio-core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/industrialio-core.c
> > > b/drivers/iio/industrialio-core.c
> > > index 9a3579943574..dab67cb69fe6 100644
> > > --- a/drivers/iio/industrialio-core.c
> > > +++ b/drivers/iio/industrialio-core.c
> > > @@ -292,7 +292,7 @@ static void __exit iio_exit(void)
> > > if (iio_devt)
> > > unregister_chrdev_region(iio_devt, IIO_DEV_MAX);
> > > bus_unregister(&iio_bus_type);
> > > - debugfs_remove(iio_debugfs_dentry);
> > > + debugfs_remove_recursive(iio_debugfs_dentry);
> > > }
> > >
> > > #if defined(CONFIG_DEBUG_FS)

\
 
 \ /
  Last update: 2019-12-23 18:30    [W:0.040 / U:2.680 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site