On 6/26/23 18:43, Boris Brezillon wrote: > On Mon, 26 Jun 2023 16:20:53 +0300 > Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx> 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
