Re: [PATCH v20 09/10] drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin

[Boris Brezillon]

On Wed, 2 Apr 2025 15:58:55 +0300
Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx> wrote:

> On 4/2/25 15:47, Thomas Zimmermann wrote:
> > Hi
> > 
> > Am 22.03.25 um 22:26 schrieb Dmitry Osipenko:  
> >> The vmapped pages shall be pinned in memory and previously get/
> >> put_pages()
> >> were implicitly hard-pinning/unpinning the pages. This will no longer be
> >> the case with addition of memory shrinker because pages_use_count > 0
> >> won't
> >> determine anymore whether pages are hard-pinned (they will be soft-
> >> pinned),
> >> while the new pages_pin_count will do the hard-pinning. Switch the
> >> vmap/vunmap() to use pin/unpin() functions in a preparation of addition
> >> of the memory shrinker support to drm-shmem.  
> > 
> > I've meanwhile rediscovered this patch and I'm sure this is not correct.
> > Vmap should not pin AFAIK. It is possible to vmap if the buffer has been
> > pinned, but that's not automatic.  For other vmaps it is necessary to
> > hold the reservation lock to prevent the buffer from moving.

Hm, is this problematic though? If you want to vmap() inside a section
that's protected by the resv lock, you can

- drm_gem_shmem_vmap_locked()
- do whatever you need to do with the vaddr,
- drm_gem_shmem_vunmap_locked()

and the {pin,page_use}_count will be back to their original values.
Those are just ref counters, and I doubt the overhead of
incrementing/decrementing them makes a difference compared to the heavy
page-allocation/vmap operations...