[PATCH 1/5] drm/panfrost: Stop using drm_gem_shmem_put_pages()

[Dmitry Osipenko]

On 6/26/23 18:43, Boris Brezillon wrote:
> On Mon, 26 Jun 2023 16:20:53 +0300
> Dmitry Osipenko <dmitry.osipenko at collabora.com> wrote:
> 
>> On 6/26/23 15:02, Boris Brezillon wrote:
>>> -err_pages:
>>> -	drm_gem_shmem_put_pages(&bo->base);
>>>  err_unlock:
>>>  	dma_resv_unlock(obj->resv);
>>> +
>>> +	if (ret && pinned)
>>> +		drm_gem_shmem_unpin(&bo->base);  
>>
>> The drm_gem_shmem_unpin() was supposed to be used only in conjunction
>> with drm_gem_shmem_pin(). I've a pending patch to enable the pin/unpin
>> refcounting needed by drm-shmem shrinker, it will prohibit invocation of
>> unpin without a previous pin.
>>
>> I'm wondering whether it will be okay to simply remove
>> drm_gem_shmem_put_pages() from the Panfrost code, letting pages to be
>> kept allocated in a error case. They will be freed once BO is destroyed.
>>
> 
> Okay, so after looking at your shmem-shrinker series, I confirm we need
> to take a pin ref here (hard-pin), otherwise the buffer might be
> evicted before the GPU is done, especially after you drop gpu_usecount
> and use only pin_count to check whether a GEM object can be evicted or
> not.

See the drm_gem_evict() [1], it checks whether GEM is busy, preventing
BO eviction while it is in-use by GPU. Note that in case of Panfrost,
shrinker isn't enabled for growable BOs.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/drm_gem.c?h=next-20230626#n1495

-- 
Best regards,
Dmitry