[PATCH 2/2] drm/amdgpu: Enable clear page functionality

[Felix Kuehling]

On 2023-12-13 9:20, Christian König wrote:
> Am 12.12.23 um 00:32 schrieb Felix Kuehling:
>>
>> On 2023-12-11 04:50, Christian König wrote:
>>> Am 08.12.23 um 20:53 schrieb Alex Deucher:
>>>> [SNIP]
>>>>> You also need a functionality which resets all cleared blocks to
>>>>> uncleared after suspend/resume.
>>>>>
>>>>> No idea how to do this, maybe Alex knows of hand.
>>>> Since the buffers are cleared on creation, is there actually 
>>>> anything to do?
>>>
>>> Well exactly that's the problem, the buffers are no longer always 
>>> cleared on creation with this patch.
>>>
>>> Instead we clear on free, track which areas are cleared and clear 
>>> only the ones which aren't cleared yet on creation.
>>
>> The code I added for clearing-on-free a long time ago, does not clear 
>> to 0, but to a non-0 poison value. That was meant to make it easier 
>> to catch applications incorrectly relying on 0-initialized memory. Is 
>> that being changed? I didn't see it in this patch series.
>
> Yeah, Arun stumbled over that as well. Any objections that we fill 
> with zeros instead or is that poison value something necessary for 
> debugging?

I don't think it's strictly necessary. But it may encourage sloppy user 
mode programming to rely on 0-initialized memory that ends up breaking 
in corner cases or on older kernels.

That said, I see that this patch series adds clearing of memory in the 
VRAM manager, but it doesn't remove the clearing for 
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE in amdgpu_bo_release_notify and 
amdgpu_move_blit. This will lead to duplicate work.

I'm also not sure how the clearing added in this patch series will 
affect free-latency observed in user mode. Will this be synchronous and 
cause the user mode thread to stall while the memory is being cleared?

Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Regards,
>>   Felix
>>
>>
>>>
>>> So some cases need special handling. E.g. when the engine is not 
>>> initialized yet or suspend/resume.
>>>
>>> In theory after a suspend/resume cycle the VRAM is cleared to zeros, 
>>> but in practice that's not always true.
>>>
>>> Christian.
>>>
>>>> Alex
>