Messages in this thread | | | From | Emil Velikov <> | Date | Mon, 22 May 2023 14:02:19 +0100 | Subject | Re: [PATCH v3 6/6] drm/shmem-helper: Switch to reservation lock |
| |
Hi Dmitry,
Saw v3 fly by, so I had a quick look. Original RB still stands, although I noticed a couple of non-blocking nitpicks.
On Sun, 21 May 2023 at 22:00, Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote:
> -static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem) > +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > {
Should this getter have a dma_resv_assert_held(shmem->base.resv); like it's put brethren?
> -void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem) > +static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem) > +{ > + int ret; > + > + dma_resv_assert_held(shmem->base.resv); > + > + ret = drm_gem_shmem_get_pages(shmem); > + > + return ret;
With the assert_held in the getter, it would be less confusing to inline this and the unpin_locked functions.
> +} > + > +static void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem) > { > - mutex_lock(&shmem->pages_lock); > - drm_gem_shmem_put_pages_locked(shmem); > - mutex_unlock(&shmem->pages_lock); > + dma_resv_assert_held(shmem->base.resv); > + > + drm_gem_shmem_put_pages(shmem);
Side note: the putter has an assert_held so the extra one here seems quite odd.
As said at the top - with or w/o these nitpicks, the original RB still stands.
HTH o/ -Emil
| |