On Mon, 26 Jun 2023 19:06:55 +0300 Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx> wrote: > 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. Okay, we should be good then, sorry for the confusion.
