From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup Date: Mon, 30 Jan 2017 10:10:15 +0100 Message-ID: <20170130091015.GS3585@ulmo.ba.sec> References: <20170126225621.12314-1-noralf@tronnes.org> <20170127074934.7fvmfkxa7wioxwt4@phenom.ffwll.local> <20170127093616.GA21758@ulmo.ba.sec> <20170127140546.7urigfzjcnz6qajc@phenom.ffwll.local> <20170130085848.GQ3585@ulmo.ba.sec> <20170130090344.ti2yx5eyyh5puy32@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0868154086==" Return-path: Received: from mail-wj0-x243.google.com (mail-wj0-x243.google.com [IPv6:2a00:1450:400c:c01::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 09E046E39B for ; Mon, 30 Jan 2017 09:10:19 +0000 (UTC) Received: by mail-wj0-x243.google.com with SMTP id i7so6880106wjf.2 for ; Mon, 30 Jan 2017 01:10:18 -0800 (PST) In-Reply-To: <20170130090344.ti2yx5eyyh5puy32@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter 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 List-Id: dri-devel@lists.freedesktop.org --===============0868154086== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="f0Ums9VvOMUG7syy" Content-Disposition: inline --f0Ums9VvOMUG7syy Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 30, 2017 at 10:03:44AM +0100, Daniel Vetter wrote: > On Mon, Jan 30, 2017 at 09:58:48AM +0100, Thierry Reding wrote: > > 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=C3=B8nnes wro= te: > > > > > > This patchset removes the need for drivers to clean up their de= bugfs > > > > > > 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. > > > > > >=20 > > > > > > Two drivers still use drm_debugfs_remove_files(): > > > > > > - tegra in it's connectors, not sure if I can remove it. > > > > >=20 > > > > > I read through them, and they're removed on the component device = nodes > > > > > stuff. That looks somewhat fishy from a lifetime point of view, a= nd I > > > > > think removing all that code would be better, too. > > > >=20 > > > > What makes you think that's problematic from a lifetime point of vi= ew? > > > > The component device is tied to the DRM device, so these callbacks = are > > > > called at the right time. > > >=20 > > > debugfs is a userspace interface, which should disappear when > > > drm_dev_unregister gets called. I'm not sure at all whether that line= s up > > > with the cleanup of all your component nodes, but otoh it's rather > > > academic since you can't hotplug a tegra. > > >=20 > > > > 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 involvi= ng > > > > kmemdup() drm_debugfs_create_files() and kfree() in order to support > > > > debugfs files for multiple instances of subdevices. > > >=20 > > > Hm, that entire "do debugfs on the minor" thing makes almost never se= nse. > > > 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? > >=20 > > 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. > >=20 > > Am I missing something? >=20 > Per-connector entries are fine, per-minor imo not. Most, if not all, debugfs files in Tegra a per-connector. We have a couple that are per-CRTC. And then we have two files that are on the minor, which is something I had copied from i915, if I remember correctly, though I can't seem to find the original anymore. Maybe that was moved somewhere else in the meantime? > This is a historical accident, but it also doesn't really hurt anyone. > I think it'd make much more sense to move everything into a > per-devices entry (with maybe backwards compat links from minor to > devices). With per-device entries you mean rooted at the device backing the CRTC, encoder, connector, ...? > But really, this is 100% orthogonal to the cleanup here. If we want to get rid of the remainder of the cleanup, then it's not entirely orthogonal anymore. =3D) Not to say that this cleanup isn't useful in its own right. Thierry --f0Ums9VvOMUG7syy Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAliPAvcACgkQ3SOs138+ s6ENxxAAltyyO+t5pmZgaRnB4PbnAIIvrIvYJDeywiWSw4mFWqxGuGTkGBTL0e7s Q7d5oR63VpmTY2LhKBEOWtUDB5wGp0hpYcqMYueU5D6XrLDHja/7YWATlfR9BebM mYiZBDfYZXWvkTwrj5xZbNV5wKDL+U8Vs6jyJQupeRS/ysF0u4476g7leqofPrYA ztUqsckla1xWSEnXomdFy3sGFmxB2mYpeEOorYJ4w6TzpHOyMGM4uklVlTDL0Tgx bOlCarMExaxQ//9OS/sjV21gE7cggerL95eA02CucNA9ptAm5WGFGjoVrmGXS1GH Ca+bNnvPMfM08EdVT3p4E3moJsLShppcaqBWG5dGgfPS1Ifgt9KPZSZtblFMMt08 PxNcUGbuZGUJc8wr5VaGkLDA6Y1WeWRdTrvLr5XVFBKuGF3i/8hoGV6bABcr0DH+ TAin1zSiZ4RBgrbmfr8Q5+qMhOQslPZWcNv6SsuV3YAxsHjQwhfYzREFt2s2tUqP mAfsRZ6pL8cmNizhkRp5SNNPudxk++zOoowwEjetlQj2RUi1w8OWX/D/ptJ61yr7 2YEJIYYuTbFkMimRdEC2yiHA9FgNLFn6GQLXjEjEx81twHb9DvcL4/Swjv2Hdi3O Y3TyuUHZQ68LwyigPcoSnNtgDgU6aOxHHUgH+eABhJPXzr5/Y0U= =0jt6 -----END PGP SIGNATURE----- --f0Ums9VvOMUG7syy-- --===============0868154086== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0868154086==--