[PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2

[Thomas Hellström (Intel)]

Hi,

On 6/8/21 9:14 AM, Christian König wrote:
> Am 08.06.21 um 07:29 schrieb Thomas Hellström (Intel):
>> Hi,
>>
>> On 6/7/21 7:59 PM, Christian König wrote:
>>>
>>>
>>> Am 07.06.21 um 19:58 schrieb Thomas Hellström (Intel):
>>>>
>>>> On 6/7/21 7:54 PM, Christian König wrote:
>>>>>
>>>>>
>>>>> Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
>>>>>>
>>>>>> On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
>>>>>>>
>>>>>>> On 6/2/21 12:09 PM, Christian König wrote:
>>>>>>> ...
>>>>>>>> @@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct 
>>>>>>>> ttm_buffer_object *bo,
>>>>>>>>    */
>>>>>>>>   static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
>>>>>>>>                     const struct ttm_place *place,
>>>>>>>> -                  struct ttm_resource *mem,
>>>>>>>> +                  struct ttm_resource **mem,
>>>>>>>>                     struct ttm_operation_ctx *ctx)
>>>>>>>>   {
>>>>>>>>       struct ttm_device *bdev = bo->bdev;
>>>>>>>> -    struct ttm_resource_manager *man = ttm_manager_type(bdev, 
>>>>>>>> mem->mem_type);
>>>>>>>> +    struct ttm_resource_manager *man;
>>>>>>>>       struct ww_acquire_ctx *ticket;
>>>>>>>>       int ret;
>>>>>>>>   +    man = ttm_manager_type(bdev, (*mem)->mem_type);
>>>>>>>
>>>>>>> Isn't (*mem) uninitialized here? Should be place->mem_type? 
>>>>>>> Eviction is immediately sent to the bushes.
>>>>>>>
>>>>>>> Got at least one additional NULL pointer deref to track down in 
>>>>>>> the eviction code, but could be a merge error of mine as well.
>>>>>>
>>>>>> Actually this last one was probably due to a bad temporary fix of 
>>>>>> the above one.
>>>>>
>>>>> I've found one more warning during my testing now. But that is 
>>>>> just a false positive.
>>>>>
>>>>> Apart from that I haven't seen any other fallout, but fingers 
>>>>> crossed.
>>>>
>>>> vmwgfx doesn't seem to happy. It works AFAICT., but warns in 
>>>> vmw_move() about ttm_bo_assign_mem() replacing an existing resource.
>>>
>>> Yeah, that's the one I've just fixed. Patch is underway.
>>
>> If that's the move_to_ghost patch, I don't think that would fix the 
>> vmwgfx issue, as IIRC vmwgfx ever uses ghost objects.
>
> Mhm, could be that vmwgfx is hitting the same warning with a different 
> backtrace.
>
> Do you have the log to double check?

Unfortunately not, but IIRC it was directly from vmw_move().

/Thomas