Re: [PATCH 12/24] drm/i915: Merge pre/postclose hooks

Daniel Vetter
2017-03-14T13:38:16Z

On Wed, Mar 08, 2017 at 04:45:13PM +0100, Daniel Vetter wrote:
> On Wed, Mar 08, 2017 at 03:07:48PM +0000, Chris Wilson wrote:
> > On Wed, Mar 08, 2017 at 03:12:45PM +0100, Daniel Vetter wrote:
> > > There's really not a reason afaics that we can't just clean up
> > > everything at the end, in the terminal postclose hook: Since this is
> > > closing a file descriptor we know no one else can have a reference or
> > > a thread doing something with that drm_file except the close code.
> > > Ordering shouldn't matter, as long as we don't kfree before we clean
> > > stuff up.
> >
> > My gut feeling was that preclose is the one we want to keep, as that is
> > closer to the onion unwind - during open, we do the core drm ops first
> > (e.g. drm_gem_open) before the backend, so during close we should do the
> > driver before the core.
> >
> > Maybe completely wrong ofc.
>
> I wasn't sure whether drivers might have some random pointers to fpriv
> that they might chase when we tear down gem objects and stuff like that.
> And if you look at what's before postclose, we still have onion
> unwrapping: All the things before it (fb cleanup, gem cleanup, master
> dropping) is stuff that userspace adds to the file _after_ it's fully
> opened.
>
> Well it's not entirely clear, because gem destroys the idr before
> postclose, which might or might not trip up someone.
>
> But afaik all the other destructive cleanup is done after postclose.
>
> So maybe that's the split we want in drm_close?
>
> - First release all runtime resources acquired after ->open. Not
> destructive cleanup, fpriv should keep working like right after ->open
> has returned. These functions would all have _release in their names.
> - call the driver's ->postclose.
> - All the destructive cleanup. Those functions would all have _destroy in
> their names.
>
> Since there's not really a real issue right now I'd prefer if we postpone
> this cleanup though, and first get all the driver patches merged. Just in
> case I missed something ...
>
> I kinda want to do an s/postclose/close/ using cocci anyway, so this isn't
> the last series on this topic.

And merged with Chris' irc r-b.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org