On Tue, Mar 28, 2017 at 12:19:41PM +0300, Shamir Rabinovitch wrote:
> On Mon, Mar 27, 2017 at 11:17:14PM +0300, Shamir Rabinovitch wrote:
> >
> > > >
> > > > +#ifdef CONFIG_INFINIBAND_IPOIB_DEBUG
> > > > +static int ipoib_netdev_event(struct notifier_block *this,
> > > > + unsigned long event, void *ptr)
> > > > +{
> > > > + struct netdev_notifier_info *ni = ptr;
> > > > + struct net_device *dev = ni->dev;
> > > > +
> > > > + if (dev->netdev_ops->ndo_open != ipoib_open)
> > > > + return NOTIFY_DONE;
> > > > +
> > > > + switch (event) {
> > > > + case NETDEV_REGISTER:
> > > > + ipoib_create_debug_files(dev);
> > > > + break;
> > > > + case NETDEV_CHANGENAME:
> > > > + ipoib_debugfs_rename(dev);
> > >
> > > Why do we need explicit ipoib_debugfs_rename function?
> > > Will it work by simply calling ipoib_delete_debug_files
> > > and immediately after that ipoib_create_debug_files?
> > >
> > >
> > > > + break;
> > > > + case NETDEV_UNREGISTER:
> > > > + ipoib_delete_debug_files(dev);
> > > > + break;
> > > > + }
> > > > +
> > > > + return NOTIFY_DONE;
> > > > +}
> > > > +#endif
> > > > +
> >
> > Hi Leon,
> >
> > Good point.
> > I will have look on this idea and get back to you.
> >
> > BR, Shamir
>
> Hi Leon,
>
> It seems that the difference between using debugfs_rename and a
> combination of debugfs_create_file and debugfs_remove is mainly the
> atomicy of the rename operation with regard to the file system.
> debugfs_rename is atomic operation while the above combination is not.
> As result I see kernel panic when the rename operation interleave with
> the delete due to module unload. So after carefully considering what you
> suggest I think it might introduce unexpected issues.
Thank you for checking it.
There is one more thing which I noticed, you should check the return
values of debugfs_remove and destroy debugfs in case of failures.
>
> BR, Shamir
Attachment:
signature.asc
Description: PGP signature
