[PATCH] vc4_bo.c: always set bo->resv

[Hans Verkuil]

On 06/02/2017 11:48 PM, Eric Anholt wrote:
> Hans Verkuil <hverkuil at xs4all.nl> writes:
> 
>> The bo->resv pointer could be NULL, leading to kernel oopses like the one below.
>> It looks like .fb_probe -> drm_fbdev_cma_create() -> drm_gem_cma_create() would
>> end up not initializing resv, because we're doing that in vc4_bo_create(), not
>> vc4_create_object().
>>
>> This patch ensures that bo->resv is always set in vc4_create_object
>> ensuring that it is never NULL.
>>
>> Thanks to Eric Anholt for pointing to the correct solution.
> 
> Folks that know dma-buf better: Will having two reservations around for
> the lifetime of the BO in the case of CMA dma-buf imports be a bad
> thing?  I'm assuming reservations are cheap, given that we're
> unconditionally allocating them per BO for un-shared BOs already.
> 
>> Signed-off-by: Hans Verkuil <hans.verkuil at cisco.com>
>> ---
>>    drivers/gpu/drm/vc4/vc4_bo.c | 12 ++++--------
>>    1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
>> index 80b2f9e55c5c..2231ee76cd8d 100644
>> --- a/drivers/gpu/drm/vc4/vc4_bo.c
>> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
>> @@ -91,8 +91,7 @@ static void vc4_bo_destroy(struct vc4_bo *bo)
>>    	vc4->bo_stats.num_allocated--;
>>    	vc4->bo_stats.size_allocated -= obj->size;
>>
>> -	if (bo->resv == &bo->_resv)
>> -		reservation_object_fini(bo->resv);
>> +	reservation_object_fini(bo->resv);
> 
> This one would need to be "reservation_object_fini(&bo->_resv);" -- we
> need to free the reservation that we created (and left unused in the
> case of an import), not the one that was imported from someone else.

Of course, fixed. I'll wait a bit to see if anyone replies to your query
above, and will post a v2 some time next week.

Regards,

	Hans

> 
>>    	drm_gem_cma_free_object(obj);
>>    }
>> @@ -212,6 +211,8 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size)
>>    	vc4->bo_stats.num_allocated++;
>>    	vc4->bo_stats.size_allocated += size;
>>    	mutex_unlock(&vc4->bo_lock);
>> +	bo->resv = &bo->_resv;
>> +	reservation_object_init(bo->resv);
>>
>>    	return &bo->base.base;
>>    }
>> @@ -250,12 +251,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size,
>>    			return ERR_PTR(-ENOMEM);
>>    		}
>>    	}
>> -	bo = to_vc4_bo(&cma_obj->base);
>> -
>> -	bo->resv = &bo->_resv;
>> -	reservation_object_init(bo->resv);
>> -
>> -	return bo;
>> +	return to_vc4_bo(&cma_obj->base);
>>    }
>>
>>    int vc4_dumb_create(struct drm_file *file_priv,
>> -- 
>> 2.11.0
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel