lkml.org 
[lkml]   [2023]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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

    \
     
     \ /
      Last update: 2023-05-22 15:04    [W:3.106 / U:0.900 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site