17 Mar
2022
17 Mar
'22
5:45 p.m.
On 3/17/22 20:32, Daniel Vetter wrote:
+static void drm_gem_shmem_update_purgeable_status(struct drm_gem_shmem_object *shmem) +{
- struct drm_gem_object *obj = &shmem->base;
- struct drm_gem_shmem_shrinker *gem_shrinker = obj->dev->shmem_shrinker;
- size_t page_count = obj->size >> PAGE_SHIFT;
- if (!gem_shrinker || obj->import_attach || !obj->funcs->purge)
return;- mutex_lock(&shmem->vmap_lock);
- mutex_lock(&shmem->pages_lock);
- mutex_lock(&gem_shrinker->lock);
Uh this is just terrible I think.
Can't we move shmem helpers over to reasonable locking, i.e. per-object dma_resv_lock for everything? I know it's a pile of work, but I think we're way past the point with things like this popping up where we should just bite that bullet.
I discussed the full thing with Daniel Stone, but maybe a joint refresher on irc would be a good thing.
Aha! Perhaps I saw bits of that discussion, but it wasn't entirely clear to me what was discussed in fact. Sounds like a good idea to try to use the reservation lock everywhere, thank you for the suggestion.