lkml.org 
[lkml]   [2018]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRE: [PATCH 3/4] drm/ttm: handle already locked BOs during eviction and swapout.
    Date
    I missed the Per-VM-BO share the reservation object with root bo. So context is not NULL here.
    So, this patch is:

    Reviewed-by: Roger He <Hongbo.He@amd.com>

    Thanks
    Roger(Hongbo.He)
    -----Original Message-----
    From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
    Sent: Friday, February 23, 2018 8:06 PM
    To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
    Subject: Re: [PATCH 3/4] drm/ttm: handle already locked BOs during eviction and swapout.

    Am 23.02.2018 um 10:46 schrieb He, Roger:
    >
    > -----Original Message-----
    > From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On
    > Behalf Of Christian K?nig
    > Sent: Tuesday, February 20, 2018 8:58 PM
    > To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
    > linux-kernel@vger.kernel.org
    > Subject: [PATCH 3/4] drm/ttm: handle already locked BOs during eviction and swapout.
    >
    > This solves the problem that when we swapout a BO from a domain we sometimes couldn't make room for it because holding the lock blocks all other BOs with this reservation object.
    >
    > Signed-off-by: Christian König <christian.koenig@amd.com>
    > ---
    > drivers/gpu/drm/ttm/ttm_bo.c | 33 ++++++++++++++++-----------------
    > 1 file changed, 16 insertions(+), 17 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
    > b/drivers/gpu/drm/ttm/ttm_bo.c index d90b1cf10b27..3a44c2ee4155 100644
    > --- a/drivers/gpu/drm/ttm/ttm_bo.c
    > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
    > @@ -713,31 +713,30 @@ bool ttm_bo_eviction_valuable(struct
    > ttm_buffer_object *bo, EXPORT_SYMBOL(ttm_bo_eviction_valuable);
    >
    > /**
    > - * Check the target bo is allowable to be evicted or swapout, including cases:
    > - *
    > - * a. if share same reservation object with ctx->resv, have
    > assumption
    > - * reservation objects should already be locked, so not lock again
    > and
    > - * return true directly when either the opreation
    > allow_reserved_eviction
    > - * or the target bo already is in delayed free list;
    > - *
    > - * b. Otherwise, trylock it.
    > + * Check if the target bo is allowed to be evicted or swapedout.
    > */
    > static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo,
    > - struct ttm_operation_ctx *ctx, bool *locked)
    > + struct ttm_operation_ctx *ctx,
    > + bool *locked)
    > {
    > - bool ret = false;
    > + /* First check if we can lock it */
    > + *locked = reservation_object_trylock(bo->resv);
    > + if (*locked)
    > + return true;
    >
    > - *locked = false;
    > + /* Check if it's locked because it is part of the current operation
    > +*/
    > if (bo->resv == ctx->resv) {
    > reservation_object_assert_held(bo->resv);
    > - if (ctx->allow_reserved_eviction || !list_empty(&bo->ddestroy))
    > - ret = true;
    > - } else {
    > - *locked = reservation_object_trylock(bo->resv);
    > - ret = *locked;
    > + return ctx->allow_reserved_eviction ||
    > + !list_empty(&bo->ddestroy);
    > }
    >
    > - return ret;
    > + /* Check if it's locked because it was already evicted */
    > + if (ww_mutex_is_owned_by(&bo->resv->lock, NULL))
    > + return true;
    >
    > For the special case: when command submission with Per-VM-BO enabled,
    > All BOs a/b/c are always valid BO. After the validation of BOs a and
    > b, when validation of BO c, is it possible to return true and then evict BO a and b by mistake ?
    > Because a/b/c share same task_struct.

    No, that's why I check the context as well. BOs explicitly reserved have a non NULL context while BOs trylocked for swapout have a NULL context.

    BOs have a non NULL context only when command submission and reserved by ttm_eu_re6serve_buffers .
    But for Per-VM-BO a/b/c they always are not in BO list, so they will be not reserved and have always NULL context.
    So above case also can happen. Anything missing here?

    >
    > + /* Some other thread is using it, don't touch it */
    > + return false;
    > }
    >
    > static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
    > --
    > 2.14.1
    >
    > _______________________________________________
    > dri-devel mailing list
    > dri-devel@lists.freedesktop.org
    > https://lists.freedesktop.org/mailman/listinfo/dri-devel

    _______________________________________________
    dri-devel mailing list
    dri-devel@lists.freedesktop.org
    https://lists.freedesktop.org/mailman/listinfo/dri-devel
    \
     
     \ /
      Last update: 2018-02-24 04:46    [W:4.418 / U:0.244 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site