[PATCH] drm/core: Do not call drm_framebuffer_remove internally during teardown.

[PATCH] drm/core: Do not call drm_framebuffer_remove internally during teardown.

[Maarten Lankhorst]

This may cause issues because encoders are already destroyed so removing
active primaries may use freed memory. Instead free the fb directly,
ignoring refcount.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 474f328535a1..9b9c4b41422a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5742,7 +5742,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	 */
 	WARN_ON(!list_empty(&dev->mode_config.fb_list));
 	list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
-		drm_framebuffer_remove(fb);
+		drm_framebuffer_free(&fb->refcount);
 	}
 
 	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/core: Do not call drm_framebuffer_remove internally during teardown.

[Ville Syrjälä]

On Wed, Sep 09, 2015 at 01:46:21PM +0200, Maarten Lankhorst wrote:
> This may cause issues because encoders are already destroyed so removing
> active primaries may use freed memory. Instead free the fb directly,
> ignoring refcount.

So what about fixing the cause, not the symptom? That is remove
framebuffers before nuking crtc/encoders/etc.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 474f328535a1..9b9c4b41422a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5742,7 +5742,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  	 */
>  	WARN_ON(!list_empty(&dev->mode_config.fb_list));
>  	list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
> -		drm_framebuffer_remove(fb);
> +		drm_framebuffer_free(&fb->refcount);
>  	}
>  
>  	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/core: Do not call drm_framebuffer_remove internally during teardown.

[Daniel Vetter]

On Wed, Sep 9, 2015 at 1:51 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Sep 09, 2015 at 01:46:21PM +0200, Maarten Lankhorst wrote:
>> This may cause issues because encoders are already destroyed so removing
>> active primaries may use freed memory. Instead free the fb directly,
>> ignoring refcount.
>
> So what about fixing the cause, not the symptom? That is remove
> framebuffers before nuking crtc/encoders/etc.

Also by that point we shouldn't have any framebuffers left (the
WARN_ON is for that), so not sure what's the point of this patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/core: Do not call drm_framebuffer_remove internally during teardown.

[Maarten Lankhorst]

Op 09-09-15 om 13:59 schreef Daniel Vetter:
> On Wed, Sep 9, 2015 at 1:51 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> On Wed, Sep 09, 2015 at 01:46:21PM +0200, Maarten Lankhorst wrote:
>>> This may cause issues because encoders are already destroyed so removing
>>> active primaries may use freed memory. Instead free the fb directly,
>>> ignoring refcount.
>> So what about fixing the cause, not the symptom? That is remove
>> framebuffers before nuking crtc/encoders/etc.
> Also by that point we shouldn't have any framebuffers left (the
> WARN_ON is for that), so not sure what's the point of this patch.
> -Daniel
Yes, but the current way would crash on atomic because encoders are already done. This removes a caller of drm_framebuffer_remove and uses the correct destroy function regardless even when refcounts are leaked. :)

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/core: Do not call drm_framebuffer_remove internally during teardown.

[Daniel Vetter]

On Wed, Sep 09, 2015 at 04:33:33PM +0200, Maarten Lankhorst wrote:
> Op 09-09-15 om 13:59 schreef Daniel Vetter:
> > On Wed, Sep 9, 2015 at 1:51 PM, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> >> On Wed, Sep 09, 2015 at 01:46:21PM +0200, Maarten Lankhorst wrote:
> >>> This may cause issues because encoders are already destroyed so removing
> >>> active primaries may use freed memory. Instead free the fb directly,
> >>> ignoring refcount.
> >> So what about fixing the cause, not the symptom? That is remove
> >> framebuffers before nuking crtc/encoders/etc.
> > Also by that point we shouldn't have any framebuffers left (the
> > WARN_ON is for that), so not sure what's the point of this patch.
> > -Daniel
> Yes, but the current way would crash on atomic because encoders are
> already done. This removes a caller of drm_framebuffer_remove and uses
> the correct destroy function regardless even when refcounts are leaked.
> :)

Makes some sense I guess ... applied to drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel