-----Original Message----- From: Daniel Vetter daniel@ffwll.ch Sent: Monday, July 27, 2020 12:33 PM To: Chris Wilson chris@chris-wilson.co.uk; Dave Airlie airlied@redhat.com Cc: intel-gfx intel-gfx@lists.freedesktop.org; stable stable@vger.kernel.org; Gustavo Padovan gustavo.padovan@collabora.com; Tang, CQ cq.tang@intel.com; dri- devel dri-devel@lists.freedesktop.org; Vetter, Daniel daniel.vetter@intel.com Subject: Re: [PATCH 1/3] drm: Restore driver.preclose() for all to use
On Thu, Jul 23, 2020 at 7:21 PM Chris Wilson chris@chris-wilson.co.uk wrote:
An unfortunate sequence of events, but it turns out there is a valid usecase for being able to free/decouple the driver objects before they are freed by the DRM core. In particular, if we have a pointer into a drm core object from inside a driver object, that pointer needs to be nerfed *before* it is freed so that concurrent access (e.g. debugfs) does not following the dangling pointer.
The legacy marker was adding in the code movement from drp_fops.c to drm_file.c
I might fumble a lot, but not this one:
commit 45c3d213a400c952ab7119f394c5293bb6877e6b Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Mon May 8 10:26:33 2017 +0200
drm: Nerf the preclose callback for modern driversAlso looking at the debugfs hook that has some rather adventurous stuff going on I think, feels a bit like a kitchensink with batteries included. If that's really all needed I'd say iterate the contexts by first going over files, then the ctx (which arent shared anyway) and the problem should also be gone.
Debugfs code can jump in after drm_gem_release() (where file->object_idr is destroyed), but before postclose(). At this window, everything is fine for debugfs context accessing except the file->object_idr.
--CQ
-Daniel
References: 9acdac68bcdc ("drm: rename drm_fops.c to drm_file.c") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@intel.com Cc: Gustavo Padovan gustavo.padovan@collabora.com Cc: CQ Tang cq.tang@intel.com Cc: stable@vger.kernel.org # v4.12+
drivers/gpu/drm/drm_file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 0ac4566ae3f4..7b4258d6f7cc 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -258,8 +258,7 @@ void drm_file_free(struct drm_file *file) (long)old_encode_dev(file->minor->kdev->devt), atomic_read(&dev->open_count));
if (drm_core_check_feature(dev, DRIVER_LEGACY) &&dev->driver->preclose)
if (dev->driver->preclose) dev->driver->preclose(dev, file); if (drm_core_check_feature(dev, DRIVER_LEGACY))-- 2.20.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch