drm: More debug info for fb leaks in mode_config_cleanup
diff mbox

Message ID 20171207144925.14191-1-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Dec. 7, 2017, 2:49 p.m. UTC
We're spotting this very rarely in CI, but have no idea. Let's add
more debug info about what's going on here.

References: https://bugs.freedesktop.org/show_bug.cgi?id=102707
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_mode_config.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Noralf Trønnes Dec. 7, 2017, 3:52 p.m. UTC | #1
Den 07.12.2017 15.49, skrev Daniel Vetter:
> We're spotting this very rarely in CI, but have no idea. Let's add
> more debug info about what's going on here.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102707
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---

Acked-by: Noralf Trønnes <noralf@tronnes.org>

>   drivers/gpu/drm/drm_mode_config.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index cc78b3d9e5e4..6ffe952142e6 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -469,6 +469,9 @@ 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) {
> +		struct drm_printer p = drm_debug_printer("[leaked fb]");
> +		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
> +		drm_framebuffer_print_info(&p, 1, fb);
>   		drm_framebuffer_free(&fb->base.refcount);
>   	}
>
Daniel Vetter Dec. 7, 2017, 5:47 p.m. UTC | #2
On Thu, Dec 07, 2017 at 04:52:04PM +0100, Noralf Trønnes wrote:
> 
> Den 07.12.2017 15.49, skrev Daniel Vetter:
> > We're spotting this very rarely in CI, but have no idea. Let's add
> > more debug info about what's going on here.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=102707
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> 
> Acked-by: Noralf Trønnes <noralf@tronnes.org>

Applied, thanks for taking a look!
-Daniel

> 
> >   drivers/gpu/drm/drm_mode_config.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index cc78b3d9e5e4..6ffe952142e6 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -469,6 +469,9 @@ 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) {
> > +		struct drm_printer p = drm_debug_printer("[leaked fb]");
> > +		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
> > +		drm_framebuffer_print_info(&p, 1, fb);
> >   		drm_framebuffer_free(&fb->base.refcount);
> >   	}
>
Thomas Hellstrom April 25, 2018, 6:37 a.m. UTC | #3
Hi!

On 12/07/2017 03:49 PM, Daniel Vetter wrote:
> We're spotting this very rarely in CI, but have no idea. Let's add
> more debug info about what's going on here.
>
> References:https://bugs.freedesktop.org/show_bug.cgi?id=102707
> Signed-off-by: Daniel Vetter<daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/drm_mode_config.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index cc78b3d9e5e4..6ffe952142e6 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -469,6 +469,9 @@ 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) {
> +		struct drm_printer p = drm_debug_printer("[leaked fb]");
> +		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
> +		drm_framebuffer_print_info(&p, 1, fb);
>   		drm_framebuffer_free(&fb->base.refcount);
>   	}
>   

Did we ever get to the bottom of what's causing this? We've seen 
occasional framebuffer leaks when transitioning from one to two screens 
and back again with vmwgfx on older drms, but I can't seem to reproduce 
that with the latest upstream code?

Thanks,

Thomas
Thomas Hellstrom April 25, 2018, 6:42 a.m. UTC | #4
Hi,

On 12/07/2017 03:49 PM, Daniel Vetter wrote:
> We're spotting this very rarely in CI, but have no idea. Let's add
> more debug info about what's going on here.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102707
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/drm_mode_config.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index cc78b3d9e5e4..6ffe952142e6 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -469,6 +469,9 @@ 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) {
> +		struct drm_printer p = drm_debug_printer("[leaked fb]");
> +		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
> +		drm_framebuffer_print_info(&p, 1, fb);
>   		drm_framebuffer_free(&fb->base.refcount);
>   	}
>   

Did we ever get to the bottom of what's causing those framebuffer leaks? 
We've seen sporadic framebuffer leaks when transitioning between one and 
two screens, but can't seem to repro with the latest drm-fixes branch?

Thanks,

Thomas
Daniel Vetter April 25, 2018, 6:49 a.m. UTC | #5
On Wed, Apr 25, 2018 at 8:42 AM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> Hi,
>
> On 12/07/2017 03:49 PM, Daniel Vetter wrote:
>>
>> We're spotting this very rarely in CI, but have no idea. Let's add
>> more debug info about what's going on here.
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=102707
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>   drivers/gpu/drm/drm_mode_config.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_mode_config.c
>> b/drivers/gpu/drm/drm_mode_config.c
>> index cc78b3d9e5e4..6ffe952142e6 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -469,6 +469,9 @@ 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)
>> {
>> +               struct drm_printer p = drm_debug_printer("[leaked fb]");
>> +               drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
>> +               drm_framebuffer_print_info(&p, 1, fb);
>>                 drm_framebuffer_free(&fb->base.refcount);
>>         }
>>
>
>
> Did we ever get to the bottom of what's causing those framebuffer leaks?
> We've seen sporadic framebuffer leaks when transitioning between one and two
> screens, but can't seem to repro with the latest drm-fixes branch?

https://bugs.freedesktop.org/show_bug.cgi?id=102707#c16

claims the fixes, but that was incomplete. Ville's recent work to
restructure the legacy fb refcounting is meant to plug this leak once
and for all. Given that vmwgfx also has some custom code (for the
fbdev stuff) could very well be that you have such issues too. See:

commit 5e9cfeba6abb7e1a3f240bd24eb29178f0b83716
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Thu Mar 22 17:22:51 2018 +0200

    drm/atomic-helper: Drop plane->fb references only for
drm_atomic_helper_shutdown()

Note that this stuff is only in -fixes, so probably not what you're looking for.
-Daniel
Thomas Hellstrom April 25, 2018, 8:15 a.m. UTC | #6
On 04/25/2018 08:49 AM, Daniel Vetter wrote:
> On Wed, Apr 25, 2018 at 8:42 AM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>> Hi,
>>
>> On 12/07/2017 03:49 PM, Daniel Vetter wrote:
>>> We're spotting this very rarely in CI, but have no idea. Let's add
>>> more debug info about what's going on here.
>>>
>>> References: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D102707&d=DwIFaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=HsUZYQaNMPVelJaUQrEYq1PTFDeRDixr0sJ9o5o5NNA&s=Fy9UT786mIt80SvkAKRdBU4Dy-_cr4ita_RmTA4Lqyk&e=
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_mode_config.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_mode_config.c
>>> b/drivers/gpu/drm/drm_mode_config.c
>>> index cc78b3d9e5e4..6ffe952142e6 100644
>>> --- a/drivers/gpu/drm/drm_mode_config.c
>>> +++ b/drivers/gpu/drm/drm_mode_config.c
>>> @@ -469,6 +469,9 @@ 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)
>>> {
>>> +               struct drm_printer p = drm_debug_printer("[leaked fb]");
>>> +               drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
>>> +               drm_framebuffer_print_info(&p, 1, fb);
>>>                  drm_framebuffer_free(&fb->base.refcount);
>>>          }
>>>
>>
>> Did we ever get to the bottom of what's causing those framebuffer leaks?
>> We've seen sporadic framebuffer leaks when transitioning between one and two
>> screens, but can't seem to repro with the latest drm-fixes branch?
>
>
> claims the fixes, but that was incomplete. Ville's recent work to
> restructure the legacy fb refcounting is meant to plug this leak once
> and for all. Given that vmwgfx also has some custom code (for the
> fbdev stuff) could very well be that you have such issues too. See:

Yes, hmm, the above doesn't fix our particular issue. I'll keep digging.

Thanks,
Thomas

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index cc78b3d9e5e4..6ffe952142e6 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -469,6 +469,9 @@  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) {
+		struct drm_printer p = drm_debug_printer("[leaked fb]");
+		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
+		drm_framebuffer_print_info(&p, 1, fb);
 		drm_framebuffer_free(&fb->base.refcount);
 	}