> -----Original Message-----
> From: Daniel Vetter <[email protected]>
> Sent: Monday, July 27, 2020 12:33 PM
> To: Chris Wilson <[email protected]>; Dave Airlie <[email protected]>
> Cc: intel-gfx <[email protected]>; stable
> <[email protected]>; Gustavo Padovan
> <[email protected]>; Tang, CQ <[email protected]>; dri-
> devel <[email protected]>; Vetter, Daniel
> <[email protected]>
> Subject: Re: [PATCH 1/3] drm: Restore driver.preclose() for all to use
>
> On Thu, Jul 23, 2020 at 7:21 PM Chris Wilson <[email protected]>
> 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 <[email protected]>
> 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 <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: Gustavo Padovan <[email protected]>
> > Cc: CQ Tang <[email protected]>
> > Cc: <[email protected]> # 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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
