Re: [PATCH 1/2] drm/core: Preserve the framebuffer after removing it.

Daniel Vetter
2015-09-10T09:53:20Z

On Thu, Sep 10, 2015 at 10:07:41AM +0100, Tvrtko Ursulin wrote:
>
> On 09/09/2015 08:06 PM, Daniel Vetter wrote:
> >On Wed, Sep 9, 2015 at 6:36 PM, Tvrtko Ursulin
> ><tvrtko.ursulin@linux.intel.com> wrote:
> >>I am not even going that far, just talking about last frame stuck on screen.
> >>For me making that easier is a regression.
> >
> >So let's look at various systems:
> >- super-modern fbdev less system: logind keeps a dup of every
> >master-capabel drm fd. Compositor crashing won't ever result in
> >close() getting called since logind still has its copy. Cleanup needs
> >to be done manually anyway with the system compositor.
> >- Current systems: Compositor restarts and cleans up the mess we left behind.
>
> What if the compositor doesn't restart? Or logind crashes in the former
> case?
>
> Maybe I don't understand something, but I don't see how it is not quite bad
> to expect userspace to clean up the kernel structures after the previous
> userspace client.

That's not different from the compositor just freezing instead of
crashing: Screen contents stays on and nothing happens. Imo this really is
all just broken userspace, and the kernel can't make sure userspace
doesn't randomly fall over.

What we need to make sure is that assuming things work ok-ish there's no
observed regression. And I still think that's the case here.

> What happens if something keeps crashing leaving framebuffers around?

Only the active ones would be kept around, we still clean up everything
else. So the leak is very limited from a memory pov.

> If the only reason is to avoid modeset, why SETPLANE with NULL fb to disable
> planes associated with a framebuffers to be released wouldn't work?

Because in general drivers don't support that - primary plane helpers
cant' do that and for many drivers that's the only thing we have.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation