Hi Thomas,
actually I was very happy to get rid of that stuff.
In the long run I indeed wanted to replace ctx->resv with the
ww_acquire_ctx to enable eviction of even more things, but that is a
different story.
Recursive locking is usually something we should try to avoid.
Regards,
Christian.
Am 15.12.2017 um 08:01 schrieb Thomas Hellstrom:
> Roger and Chrisitian,
>
> Correct me if I'm wrong, but It seems to me like a lot of the recent
> changes to ttm_bo.c are to allow recursive reservation object locking
> in the case of shared reservation objects, but only in certain
> functions and with special arguments so it doesn't look like recursive
> locking to the lockdep checker. Wouldn't it be a lot cleaner if we
> were to hide all this in a resurrected __ttm_bo_reserve something
> along the lines of
>
> int __ttm_bo_reserve(struct ttm_bo *bo, struct ttm_operation_ctx *ctx) {
> Â Â Â if (ctx && ctx->resv == bo->resv) {
> #ifdef CONFIG_LOCKDEP
> Â Â Â Â Â Â Â WARN_ON(bo->reserved);
> lockdep_assert_held(&ctx->resv);
> Â Â Â Â Â Â ctx->reserve_count++;
> bo->reserved = true;
> #endif
> Â Â Â Â Â Â return0;
> Â Â Â Â } else {
> Â Â Â Â Â Â Â int ret = reservation_object_lock(bo->resv, NULL) ? 0:-EBUSY;
>
> Â Â Â Â Â Â Â if (ret)
> Â Â Â Â Â Â Â Â Â return ret;
> #ifdef CONFIG_LOCKDEP
> Â Â Â Â Â Â WARN_ON(bo->reserved);
> Â Â Â Â Â Â bo->reserved = true;
> #endif
> Â Â Â Â Â Â return 0;
> }
>
> And similar for tryreserve and unreserve? Perhaps with a
> ww_acquire_ctx included somewhere as well...
>
> /Thomas
>
>
>
>
> On 12/14/2017 09:10 AM, Roger He wrote:
>> Change-Id: I0c6ece0decd18d30ccc94e5c7ca106d351941c62
>> Signed-off-by: Roger He <Hongbo.He at amd.com>
>> ---
>> Â drivers/gpu/drm/ttm/ttm_bo.c | 11 +++++------
>> Â 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 098b22e..ba5b486 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -707,7 +707,6 @@ bool ttm_bo_eviction_valuable(struct
>> ttm_buffer_object *bo,
>> Â EXPORT_SYMBOL(ttm_bo_eviction_valuable);
>> Â Â static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct reservation_object *resv,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â uint32_t mem_type,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â const struct ttm_place *place,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct ttm_operation_ctx *ctx)
>> @@ -722,8 +721,9 @@ static int ttm_mem_evict_first(struct
>> ttm_bo_device *bdev,
>> Â Â Â Â Â spin_lock(&glob->lru_lock);
>> Â Â Â Â Â for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> Â Â Â Â Â Â Â Â Â list_for_each_entry(bo, &man->lru[i], lru) {
>> -Â Â Â Â Â Â Â Â Â Â Â if (bo->resv == resv) {
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (list_empty(&bo->ddestroy))
>> +Â Â Â Â Â Â Â Â Â Â Â if (bo->resv == ctx->resv) {
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!ctx->allow_reserved_eviction &&
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â list_empty(&bo->ddestroy))
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue;
>> Â Â Â Â Â Â Â Â Â Â Â Â Â } else {
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â locked = reservation_object_trylock(bo->resv);
>> @@ -835,7 +835,7 @@ static int ttm_bo_mem_force_space(struct
>> ttm_buffer_object *bo,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>> Â Â Â Â Â Â Â Â Â if (mem->mm_node)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>> -Â Â Â Â Â Â Â ret = ttm_mem_evict_first(bdev, bo->resv, mem_type, place,
>> ctx);
>> +Â Â Â Â Â Â Â ret = ttm_mem_evict_first(bdev, mem_type, place, ctx);
>> Â Â Â Â Â Â Â Â Â if (unlikely(ret != 0))
>> Â Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>> Â Â Â Â Â } while (1);
>> @@ -1332,8 +1332,7 @@ static int ttm_bo_force_list_clean(struct
>> ttm_bo_device *bdev,
>> Â Â Â Â Â for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>> Â Â Â Â Â Â Â Â Â while (!list_empty(&man->lru[i])) {
>> Â Â Â Â Â Â Â Â Â Â Â Â Â spin_unlock(&glob->lru_lock);
>> -Â Â Â Â Â Â Â Â Â Â Â ret = ttm_mem_evict_first(bdev, NULL, mem_type,
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â NULL, &ctx);
>> +Â Â Â Â Â Â Â Â Â Â Â ret = ttm_mem_evict_first(bdev, mem_type, NULL, &ctx);
>> Â Â Â Â Â Â Â Â Â Â Â Â Â if (ret)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>> Â Â Â Â Â Â Â Â Â Â Â Â Â spin_lock(&glob->lru_lock);
>
>
