Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

From: Thierry Reding <thierry.reding@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: liviu.dudau@arm.com, linux@armlinux.org.uk,
	dri-devel@lists.freedesktop.org, bskeggs@redhat.com,
	tomi.valkeinen@ti.com, jsarha@ti.com,
	linux+etnaviv@armlinux.org.uk, alexander.deucher@amd.com,
	daniel.vetter@intel.com, vincent.abriou@st.com,
	christian.koenig@amd.com, kraxel@redhat.com
Subject: Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup
Date: Mon, 30 Jan 2017 09:58:48 +0100	[thread overview]
Message-ID: <20170130085848.GQ3585@ulmo.ba.sec> (raw)
In-Reply-To: <20170127140546.7urigfzjcnz6qajc@phenom.ffwll.local>

[-- Attachment #1.1: Type: text/plain, Size: 2147 bytes --]

On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote:
> > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > > This patchset removes the need for drivers to clean up their debugfs
> > > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > > This funtion is also called should the driver error out in it's
> > > > drm_driver.debugfs_init callback.
> > > > 
> > > > Two drivers still use drm_debugfs_remove_files():
> > > > - tegra in it's connectors, not sure if I can remove it.
> > > 
> > > I read through them, and they're removed on the component device nodes
> > > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > > think removing all that code would be better, too.
> > 
> > What makes you think that's problematic from a lifetime point of view?
> > The component device is tied to the DRM device, so these callbacks are
> > called at the right time.
> 
> debugfs is a userspace interface, which should disappear when
> drm_dev_unregister gets called. I'm not sure at all whether that lines up
> with the cleanup of all your component nodes, but otoh it's rather
> academic since you can't hotplug a tegra.
> 
> > That said, I think it's safe to remove the other debugfs files from
> > Tegra. It might not be possible to remove the cleanup functions
> > altogether, though, because they have to do a special dance involving
> > kmemdup() drm_debugfs_create_files() and kfree() in order to support
> > debugfs files for multiple instances of subdevices.
> 
> Hm, that entire "do debugfs on the minor" thing makes almost never sense.
> All the things we have left in modern drivers are either per-fd, or
> per-device. Nothing of interest is per-minor. Or do you mean something
> else?

I'm not sure I understand what you're saying. We have plenty of code
that adds debugfs files to the connector's debugfs entry. And that's
within the minor's debugfs root.

Am I missing something?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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