[PATCH 4/4] drm/xe/bo: add some annotations in bo_put()

[Matthew Auld]

On 10/09/2024 15:49, Matthew Brost wrote:
> On Tue, Sep 10, 2024 at 02:11:49PM +0100, Matthew Auld wrote:
>> If the put() triggers bo destroy then there is at least one potential
>> sleeping lock. Also annotate bos_lock and ggtt lock.
>>
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
>> Cc: Tejas Upadhyay <tejas.upadhyay at intel.com>
>> Cc: "Thomas Hellström" <thomas.hellstrom at linux.intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_bo.c | 14 ++++++++++++++
>>   drivers/gpu/drm/xe/xe_bo.h |  6 +-----
>>   2 files changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index 53948a965404..4176b302c38d 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -2320,6 +2320,20 @@ void xe_bo_put_commit(struct llist_head *deferred)
>>   		drm_gem_object_free(&bo->ttm.base.refcount);
>>   }
>>   
>> +void xe_bo_put(struct xe_bo *bo)
>> +{
>> +	might_sleep();
>> +	if (bo) {
>> +#ifdef CONFIG_PROC_FS
>> +		if (bo->client)
>> +			might_lock(&bo->client->bos_lock);
>> +#endif
>> +		if (bo->ggtt_node && bo->ggtt_node->ggtt)
>> +			might_lock(&bo->ggtt_node->ggtt->lock);
> 
> Missed this. We can take dma-resv lock too if we get into the TTM code
> upon this put too, right? We should probably add that annotate too.

Yeah, I was also wondering the same thing, but it looks to only be 
trylock in ttm_bo_release(). AFAICT it will only grab the full dma-resv 
lock from a worker, which it does if the lock is contended, or if there 
are unsignalled fences and we need to wait.

> 
> Matt
> 
>> +		drm_gem_object_put(&bo->ttm.base);
>> +	}
>> +}
>> +
>>   /**
>>    * xe_bo_dumb_create - Create a dumb bo as backing for a fb
>>    * @file_priv: ...
>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
>> index dbfb3209615d..6e4be52306df 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.h
>> +++ b/drivers/gpu/drm/xe/xe_bo.h
>> @@ -126,11 +126,7 @@ static inline struct xe_bo *xe_bo_get(struct xe_bo *bo)
>>   	return bo;
>>   }
>>   
>> -static inline void xe_bo_put(struct xe_bo *bo)
>> -{
>> -	if (bo)
>> -		drm_gem_object_put(&bo->ttm.base);
>> -}
>> +void xe_bo_put(struct xe_bo *bo);
>>   
>>   static inline void __xe_bo_unset_bulk_move(struct xe_bo *bo)
>>   {
>> -- 
>> 2.46.0
>>