Re: vmwgfx leaking bo pins?

From: "Christian König" <christian.koenig@amd.com>
To: "Zack Rusin" <zackr@vmware.com>,
	"Thomas Hellström (Intel)" <thomas_os@shipmail.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Linux-graphics-maintainer <Linux-graphics-maintainer@vmware.com>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: vmwgfx leaking bo pins?
Date: Fri, 12 Mar 2021 09:13:40 +0100	[thread overview]
Message-ID: <8809a7fa-115b-9987-71cd-0510b6d4f39f@amd.com> (raw)
In-Reply-To: <DCF8E67E-36D4-400A-88B7-63C6253179D6@vmware.com>

Am 12.03.21 um 00:02 schrieb Zack Rusin:
>
>> On Mar 11, 2021, at 17:35, Thomas Hellström (Intel) <thomas_os@shipmail.org> wrote:
>>
>> Hi, Zack
>>
>> On 3/11/21 10:07 PM, Zack Rusin wrote:
>>>> On Mar 11, 2021, at 05:46, Thomas Hellström (Intel) <thomas_os@shipmail.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I tried latest drm-fixes today and saw a lot of these: Fallout from ttm rework?
>>> Yes, I fixed this in d1a73c641afd2617bd80bce8b71a096fc5b74b7e it was in drm-misc-next in the drm-misc tree for a while but hasn’t been merged for 5.12.

Mhm, crap. I hoped that you would have the same issue as radeon somehow 
and could help with debugging.

Please make sure the patch is pushed to drm-misc-fixes so that it ends 
up fixed in 5.12.

>>>
>>> z
>>>
>> Hmm, yes but doesn't that fix trip the ttm_bo_unpin() dma_resv_assert_held(bo->base.resv)?
> No, doesn’t seem to. TBH I’m not sure why myself, but it seems to be working fine.
>
>> Taking the reservation to unpin at TTM bo free has always been awkward and that's why vmwgfx and I guess other TTM drivers have been sloppy doing that as TTM never cared. Perhaps TTM could change the pin_count to an atomic and allow unlocked unpinning? still requiring the reservation lock for pin_count transition 0->1, though.
> Yea, that’d probably make sense. I think in general just making sure the requirements are consistent and well documented would be great.
>
>> Also, pinning at bo creation in vmwgfx has been to do the equivalent of ttm_bo_init_reserved() (which api was added later). Creating pinned would make the object isolated and allowing the reserve trylock that followed to always succeed. With the introduction of the TTM pin_count, it seems ttm_bo_init_reserved() is used to enable pinned creation which is used to emulate ttm_bo_init_reserved() :)
> Yea, we should probably port the vmwgfx code to ttm_bo_init_reserved just to be match the newly established semantics.

Yeah, I stumbled over that at well during one of the cleanups and 
considered changing the implementation. But then it got lost in all the 
rework.

Christian.

> z

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel