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.