[PATCH] drm/ttm: make ttm_bo_unpin more defensive

[Thomas Zimmermann]

Hi

Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
> Hi, Christian
> 
> On 3/12/21 10:38 AM, Christian König wrote:
>> We seem to have some more driver bugs than thought.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>>   include/drm/ttm/ttm_bo_api.h | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
>> index 4fb523dfab32..df9fe596e7c5 100644
>> --- a/include/drm/ttm/ttm_bo_api.h
>> +++ b/include/drm/ttm/ttm_bo_api.h
>> @@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct 
>> ttm_buffer_object *bo)
>>   static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
>>   {
>>       dma_resv_assert_held(bo->base.resv);
>> -    WARN_ON_ONCE(!bo->pin_count);
>>       WARN_ON_ONCE(!kref_read(&bo->kref));
>> -    --bo->pin_count;
>> +    if (bo->pin_count)
>> +        --bo->pin_count;
>> +    else
>> +        WARN_ON_ONCE(true);
>>   }
>>   int ttm_mem_evict_first(struct ttm_device *bdev,
> 
> Since I now have been staring for half a year at the code of the driver 
> that made pinning an art, I have a couple of suggestions here, Could we 
> use an atomic for pin_count, allowing unlocked unpinning but require the 
> lock only for pin_count transition 0->1, (but unlocked for 
> pin_if_already_pinned). In particular I think vmwgfx would benefit from 
> unlocked unpins. Also if the atomic were a refcount_t, that would 
> probably give you the above behaviour?

What's the benefit?

I'm asking because, there's been talk about streamlining all the GEM 
locking and actually allowing dma-buf resv locking in pin and vmap 
operations. Atomic ops might not contradict this, but also might not be 
useful in the long term.

Best regards
Thomas

> 
> /Thomas
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210315/bc9180bc/attachment-0001.sig>