Re: [PATCH v7 1/6] drm/amdgpu: Fix init ordering in amdgpu_vram_mgr_init()

[Maarten Lankhorst]

Hey,

On 7/21/26 14:10, Thomas Hellström wrote:
> Hi, Maarten,
> 
> On Tue, 2026-07-21 at 13:46 +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> Can I merge this through drm-misc-next together with the rest of the
>> series?
>>
>> Kind regards
>> ~Maarten
> 
> For the xe patches, Ack from me.
> 
> I do have a v8 that fixes a sashiko comment on the last patch, though,
> but I was holding that awaiting a review on the other patches.
Yeah seems about right. Patches themselves look good to me now, so would be nice if amd can ack here or in v8.

Series themselves look good, so feel free to add my r-b to entire series: (and Thadeu's t-b)

Reviewed-By: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>

> 
> Thanks,
> Thomas
> 
> 
> 
>>
>> On 7/7/26 20:05, Arunpravin Paneer Selvam wrote:
>>>
>>>
>>> On 7/3/2026 6:38 PM, Christian König wrote:
>>>> Arun please take a look at this.
>>> Sure Christian. This fix looks correct to me.
>>> Reviewed-by: Arunpravin Paneer Selvam
>>> <Arunpravin.PaneerSelvam@xxxxxxx>
>>>
>>> Thanks,
>>> Arun.
>>>>
>>>> Thanks,
>>>> Christian.
>>>>
>>>> On 7/3/26 15:05, Thomas Hellström wrote:
>>>>> drmm_cgroup_register_region() is called before INIT_LIST_HEAD()
>>>>> and
>>>>> gpu_buddy_init() in amdgpu_vram_mgr_init(). If it fails, the
>>>>> function
>>>>> returns early and bypasses those initializations.
>>>>>
>>>>> Since adev->mman.initialized is set to true before
>>>>> amdgpu_vram_mgr_init()
>>>>> is called, a failure triggers amdgpu_ttm_fini(), which calls
>>>>> amdgpu_vram_mgr_fini(), which then:
>>>>>
>>>>>   - Calls list_for_each_entry_safe() on reservations_pending
>>>>> and
>>>>>     reserved_pages, whose list_head::next pointers are zero-
>>>>> initialized
>>>>>     (NULL). The loop does not recognize them as empty and
>>>>> dereferences NULL.
>>>>>
>>>>>   - Calls gpu_buddy_fini(), which iterates free_trees[]
>>>>> unconditionally
>>>>>     via for_each_free_tree(). Since mm->free_trees is NULL
>>>>>     (never allocated), this dereferences NULL.
>>>>>
>>>>> Both result in a kernel panic on the module load error path.
>>>>>
>>>>> Fix by moving drmm_cgroup_register_region() to after the list
>>>>> and buddy
>>>>> allocator are fully initialized, so the teardown path is safe
>>>>> to run.
>>>>>
>>>>> Reported-by: Sashiko-bot <sashiko-bot@xxxxxxxxxx>
>>>>> Closes:
>>>>> https://sashiko.dev/#/patchset/20260428073116.15687-1-thomas.hellstrom@xxxxxxxxxxxxxxx?part=4
>>>>> Fixes: 2b624a2c1865 ("drm/ttm: Handle cgroup based eviction in
>>>>> TTM")
>>>>> Cc: Friedrich Vock <friedrich.vock@xxxxxx>
>>>>> Cc: Maarten Lankhorst <dev@xxxxxxxxxxxx>
>>>>> Cc: Tejun Heo <tj@xxxxxxxxxx>
>>>>> Cc: Maxime Ripard <mripard@xxxxxxxxxx>
>>>>> Cc: Christian König <christian.koenig@xxxxxxx>
>>>>> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
>>>>> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>>>>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
>>>>> Cc: <stable@xxxxxxxxxxxxxxx> # v6.14+
>>>>> Assisted-by: GitHub_Copilot:claude-sonnet-4.6
>>>>> Signed-off-by: Thomas Hellström
>>>>> <thomas.hellstrom@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 7 ++++---
>>>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>>> index 2a241a5b12c4..ac3f71d77140 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>>> @@ -918,9 +918,6 @@ int amdgpu_vram_mgr_init(struct
>>>>> amdgpu_device *adev)
>>>>>       struct ttm_resource_manager *man = &mgr->manager;
>>>>>       int err;
>>>>>   -    man->cg = drmm_cgroup_register_region(adev_to_drm(adev),
>>>>> "vram", adev->gmc.real_vram_size);
>>>>> -    if (IS_ERR(man->cg))
>>>>> -        return PTR_ERR(man->cg);
>>>>>       ttm_resource_manager_init(man, &adev->mman.bdev,
>>>>>                     adev->gmc.real_vram_size);
>>>>>   @@ -935,6 +932,10 @@ int amdgpu_vram_mgr_init(struct
>>>>> amdgpu_device *adev)
>>>>>       if (err)
>>>>>           return err;
>>>>>   +    man->cg = drmm_cgroup_register_region(adev_to_drm(adev),
>>>>> "vram", adev->gmc.real_vram_size);
>>>>> +    if (IS_ERR(man->cg))
>>>>> +        return PTR_ERR(man->cg);
>>>>> +
>>>>>       ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_VRAM,
>>>>> &mgr->manager);
>>>>>       ttm_resource_manager_set_used(man, true);
>>>>>       return 0;
>>>