Re: [PATCH 1/2] drm/ttm: fix busy memory to fail other user v6

From: "Koenig, Christian" <Christian.Koenig@amd.com>
To: Thomas Hellstrom <thomas@shipmail.org>,
	"Zhou, David(ChunMing)" <David1.Zhou@amd.com>,
	"Liang, Prike" <Prike.Liang@amd.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/ttm: fix busy memory to fail other user v6
Date: Wed, 8 May 2019 09:03:34 +0000	[thread overview]
Message-ID: <c100cb61-311f-aa07-7b6b-335b9049b0fc@amd.com> (raw)
In-Reply-To: <27bbe674-1095-1015-e4f3-1c336f71879f@shipmail.org>

Am 08.05.19 um 10:34 schrieb Thomas Hellstrom:
> [SNIP]
>>>> No, what I mean is to add the acquire_ctx as separate parameter to
>>>> ttm_mem_evict_first().
>>>>
>>>> E.g. we only need it in this function and it is actually not related
>>>> to the ttm operation context filled in by the driver.
>>>
>>> FWIW, I think it would be nice at some point to have a reservation
>>> context being part of the ttm operation context, so that validate and
>>> evict could do sleeping reservations, and have bos remain on the lru
>>> even when reserved...
>> Yeah, well that's exactly what the ctx->resv parameter is good for :)
>
> Hmm. I don't quite follow? It looks to me like ctx->resv is there to
> work around recursive reservations?

Well yes and no, this is to allow eviction of BOs which share the same 
reservation object.

>
>
> What I'm after is being able to do sleeping reservations within validate
> and evict and open up for returning -EDEADLK. One benefit would be to
> scan over the LRU lists, reserving exactly those bos we want to evict,
> and when all are reserved, we evict them. If we hit an -EDEADLK while
> evicting we need to restart. Then we need an acquire_ctx in the
> ttm_operation_ctx.

The acquire_ctx is available from the BO you try to find space for.

But we already tried this approach and it doesn't work. We have a lot of 
BOs which now share the same reservation object and so would cause an 
-EDEADLK.

>> And yes, we do keep the BOs on the LRU even when they are reserved.
>
> static inline int ttm_bo_reserve(struct ttm_buffer_object *bo,
>                  bool interruptible, bool no_wait,
>                  struct ww_acquire_ctx *ticket)

ttm_bo_reserve() is not always used any more outside of TTM. The for 
DMA-buf as well as amdgpu VMs code the reservation object is locked 
without calling ttm_bo_reserve now.

Regards,
Christian.

>
> /Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel