Re: [PATCH 1/2] drm: debugfs: make drm_debugfs_remove_files() a void function

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Jun 18, 2019 at 02:17:11PM +0200, Daniel Vetter wrote:
> On Tue, Jun 18, 2019 at 02:01:35PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jun 14, 2019 at 05:37:58PM +0200, Daniel Vetter wrote:
> > > On Fri, Jun 14, 2019 at 5:20 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Jun 14, 2019 at 04:59:08PM +0200, Daniel Vetter wrote:
> > > > > On Fri, Jun 14, 2019 at 11:51:09AM +0200, Greg Kroah-Hartman wrote:
> > > > > > The function can not fail, and no one checks the current return value,
> > > > > > so just mark it as a void function so no one gets confused.
> > > > > >
> > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > > Cc: David Airlie <airlied@linux.ie>
> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > ---
> > > > > >  drivers/gpu/drm/drm_debugfs.c | 5 ++---
> > > > > >  include/drm/drm_debugfs.h     | 9 ++++-----
> > > > > >  2 files changed, 6 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> > > > > > index 6f2802e9bfb5..515569002c86 100644
> > > > > > --- a/drivers/gpu/drm/drm_debugfs.c
> > > > > > +++ b/drivers/gpu/drm/drm_debugfs.c
> > > > > > @@ -270,8 +270,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> > > > > >  }
> > > > > >
> > > > > >
> > > > > > -int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
> > > > > > -                        struct drm_minor *minor)
> > > > > > +void drm_debugfs_remove_files(const struct drm_info_list *files, int count,
> > > > > > +                         struct drm_minor *minor)
> > > > >
> > > > > We're trying to entirely nuke this function here, see the kerneldoc for
> > > > > drm_debugfs_create_files(). Only user left is tegra, and we call the
> > > > > "remove all debugfs files" and the ->early_unregister hooks all from the
> > > > > same place. So this can all be garbage collected. It's mildly annoying
> > > > > because we'd need to move the kfree from ->early_unregister into ->destroy
> > > > > callbacks, because connectors are unregister before we throw away all the
> > > > > debugfs files in drm_dev_unregister(). But imo that's cleaner anway.
> > > >
> > > > I would love to see this function gone, it can also make things a lot
> > > > simpler from the point of view of creating the debugfs files as well, as
> > > > no dentries will need to be saved.
> > > >
> > > > > Up for that?
> > > >
> > > > Sure, I can do that.  I have a much larger patch messing with
> > > > drm_debugfs_create_files() that I want you all to be in a good mood for
> > > > when I submit it (it touches all drivers at once), so I might as well
> > > > clean this up first :)
> > > 
> > > Oh don't worry, we've had a pile of cleanup todo tasks in this area
> > > since a long time. You doing them all is going to make me a happy
> > > camper :-)
> > > 
> > > Only thing to be aware of is that we have a bit a habit of dragging
> > > good contributors of refactoring/cleanup/fundamental work like this
> > > into the drm fold for good. You might get stuck ...
> > 
> > Hah...
> > 
> > Anyway, what tree/branch should I do this work on?  I see drm-next, is
> > that the one to use, but it doesn't seem to have the other patches you
> > all said you accepted from me for this debugfs cleanup already.
> 
> linux-next is usually a good starting point, drm stuff is a bit too much
> spread around multiple trees. The only caveat is that some trees (drm-misc
> and drm-intel) keep merging during the feature freeze (after -rc6) and
> merge window, to collect patches for the merge-window+1. In those cases it
> might be better to rebase on top of drm-tip:
> 
> https://cgit.freedesktop.org/drm-tip
> 
> It's kinda like linux-next, but for drm. Only downside is that not all drm
> trees participate in that integration tree.
> 
> Simpelst is probably to just base on linux-next and dump everything that
> hasn't landed after -rc2 onto dri-devel again.

Thanks, I'll work off of that...

And what a tangled web we weave of debugfs files and pointers, it's
worse than sysfs here.  I'll send another email with why this work is so
hard to do...

thanks,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel