> -----Original Message----- > From: Daniel Vetter <daniel@xxxxxxxx> > Sent: Monday, July 27, 2020 12:33 PM > To: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>; Dave Airlie <airlied@xxxxxxxxxx> > Cc: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; stable > <stable@xxxxxxxxxxxxxxx>; Gustavo Padovan > <gustavo.padovan@xxxxxxxxxxxxx>; Tang, CQ <cq.tang@xxxxxxxxx>; dri- > devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; Vetter, Daniel > <daniel.vetter@xxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxx> > 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@xxxxxxxx> > Date: Mon May 8 10:26:33 2017 +0200 > > drm: Nerf the preclose callback for modern drivers > > Also 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@xxxxxxxxxxxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > Cc: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxx> > > Cc: CQ Tang <cq.tang@xxxxxxxxx> > > Cc: <stable@xxxxxxxxxxxxxxx> # 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@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch