On Fri, 4 Apr 2025 16:58:20 +0200 Thomas Zimmermann <tzimmerm...@suse.de> wrote:
> Hi > > Am 04.04.25 um 16:52 schrieb Boris Brezillon: > > On Fri, 4 Apr 2025 10:01:50 +0200 > > Thomas Zimmermann <tzimmerm...@suse.de> wrote: > > > >>>> In your case, vmap an pin both intent to hold the shmem pages in memory. > >>>> They might be build on top of the same implementation, but one should > >>>> not be implemented with the other because of their different meanings. > >>> But that's not what we do, is it? Sure, in drm_gem_shmem_vmap_locked(), > >>> we call drm_gem_shmem_pin_locked(), but that's an internal function to > >>> make sure the pages are allocated and stay around until > >>> drm_gem_shmem_vunmap_locked() is called. > >>> > >>> I guess we could rename pin_count into hard_refcount or > >>> page_residency_count or xxx_count, and change the pin/unpin_locked() > >>> function names accordingly, but that's just a naming detail, it doesn't > >>> force you to call drm_gem_pin() to vmap() your GEM, it's something we > >>> do internally. > >> Such a rename would be much appreciated. page_residency_count seems > >> appropriate. > > On a second thought, I think I prefer > > 'unevictable_count/inc_unevictable()/dec_unevictable()'. But looking at > > Ok > > > the gem-vram changes you just posted, it looks like gem-shmem is not the > > only one to use the term 'pin' for this page pinning thing, so if we go > > and plan for a rename, I'd rather make it DRM-wide than gem-shmem being > > the outlier yet again :-). > > Which one do you mean? > > - The code in gem-vram does pinning in the TTM sense of the term. Sorry, but I still don't see why pinning should be a TTM only thing. If I read the doc of drm_gem_vram_pin(): " Pinning a buffer object ensures that it is not evicted from a memory region. A pinned buffer object has to be unpinned before it can be pinned to another region. If the pl_flag argument is 0, the buffer is pinned at its current location (video RAM or system memory). " And this pinning is not so different from the pinning we have in gem-shmem: making buffer object memory unevictable/unmovable. > > - From the client code, the pinning got removed. > > - The GEM pin/unpin callbacks are still there, but the long-term plan is > to go to dma-buf pin callbacks AFAICT. > > - Renaming the dma-buf pin/unpin would be a kernel-wide change. Not > likely to happen. Again, I'm not sure why we'd want to do that anyway. Just because the TTM pinning semantics might be slightly different from the GEM/dma-buf ones doesn't mean the pinning term should be prohibited outside the TTM scope. The concept of pinning is pretty generic and applies to system memory too AFAICT.
