Re: [PATCH 2/2] drm/amdgpu: add AMDGPU_INFO_VM_STAT to return GPU VM

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]

Am 04.01.23 um 00:08 schrieb Marek Olšák:
> I see about the access now, but did you even look at the patch?

I did look at the patch, but I haven't fully understood yet what you are 
trying to do here.

> Because what the patch does isn't even exposed to common drm code, 
> such as the preferred domain and visible VRAM placement, so it can't 
> be in fdinfo right now.
>
> Or do you even know what fdinfo contains? Because it contains nothing 
> useful. It only has VRAM and GTT usage, which we already have in the 
> INFO ioctl, so it has nothing that we need. We mainly need the 
> eviction information and visible VRAM information now. Everything else 
> is a bonus.

Well the main question is what are you trying to get from that 
information? The eviction list for example is completely meaningless to 
userspace, that stuff is only temporary and will be cleared on the next 
CS again.

What we could expose is the VRAM over-commit value, e.g. how much BOs 
which where supposed to be in VRAM are in GTT now. I think that's what 
you are looking for here, right?

> Also, it's undesirable to open and parse a text file if we can just 
> call an ioctl.

Well I see the reasoning for that, but I also see why other drivers do a 
lot of the stuff we have as IOCTL as separate files in sysfs, fdinfo or 
debugfs.

Especially repeating all the static information which were already 
available under sysfs in the INFO IOCTL was a design mistake as far as I 
can see. Just compare what AMDGPU and the KFD code is doing to what for 
example i915 is doing.

Same for things like debug information about a process. The fdinfo stuff 
can be queried from external tools (gdb, gputop, umr etc...) as well 
which makes that interface more preferred.

>
> So do you want me to move it into amdgpu_vm.c? Because you could have 
> just said: Let's move it into amdgpu_vm.c. :)
>
> Thanks,
> Marek
>
> On Tue, Jan 3, 2023 at 3:33 AM Christian König 
> <ckoenig.leichtzumerken@gmail.com> wrote:
>
>     Take a look at /proc/self/fdinfo/$fd.
>
>     The Intel guys made that vendor agnostic and are using it within
>     their IGT gpu top tool.
>
>     Christian.
>
>     Am 02.01.23 um 18:57 schrieb Marek Olšák:
>>     What are you talking about? Is fdinfo in sysfs? Userspace drivers
>>     can't access sysfs.
>>
>>     Marek
>>
>>     On Mon, Jan 2, 2023, 10:56 Christian König
>>     <ckoenig.leichtzumerken@gmail.com> wrote:
>>
>>         Well first of all don't mess with the VM internals outside of
>>         the VM code.
>>
>>         Then why would we want to expose this through the IOCTL
>>         interface? We already have this in the fdinfo.
>>
>>         Christian.
>>
>>         Am 30.12.22 um 23:07 schrieb Marek Olšák:
>>>         To give userspace a detailed view about its GPU memory usage
>>>         and evictions.
>>>         This will help performance investigations.
>>>
>>>         Signed-off-by: Marek Olšák <marek.olsak@amd.com>
>>>
>>>         The patch is attached.
>>>
>>>         Marek
>>
>

[-- Attachment #2: Type: text/html, Size: 6718 bytes --]