[PATCH 28/37] drm/amdkfd: Add support for displaying VRAM usage

Oded Gabbay oded.gabbay at gmail.com
Tue Dec 19 09:42:51 UTC 2017


On Sat, Dec 16, 2017 at 10:36 PM, Felix Kühling
<felix.kuehling at gmail.com> wrote:
> Am 13.12.2017 um 08:23 schrieb Oded Gabbay:
>> On Tue, Dec 12, 2017 at 1:11 PM, Russell, Kent <Kent.Russell at amd.com> wrote:
>>> That's alright. I admit that it was a bit self-serving in that I was asked to get something somewhere to get the information out, and it was the simplest solution. I can see if I can come up with a more acceptable option in a future patch set, but for now I think Felix is right in that we can just drop this one for now, it's definitely not worth holding up the rest of the patches over.
>>>
>>>  Kent
>> Sure, np. I'll drop this for now.
>
> FWIW, there is precedent for this type of information in sysfs. See
> /sys/devices/virtual/drm/ttm/memory_accounting/kernel/used_memory.
>
> Regards,
>   Felix

I understand but I don't think that's applicable in our case. Who
works directly vs. ttm ? I'm not familiar with it enough but it seems
more like a debug feature.
I think it is better to follow amdgpu programming model.

Oded

>
>>
>> Oded
>>
>>> -----Original Message-----
>>> From: Kuehling, Felix
>>> Sent: Monday, December 11, 2017 2:52 PM
>>> To: Koenig, Christian; Oded Gabbay
>>> Cc: Russell, Kent; amd-gfx list
>>> Subject: Re: [PATCH 28/37] drm/amdkfd: Add support for displaying VRAM usage
>>>
>>> On 2017-12-11 12:28 PM, Christian König wrote:
>>>> Am 11.12.2017 um 17:40 schrieb Oded Gabbay:
>>>>> On Mon, Dec 11, 2017 at 5:32 PM, Oded Gabbay <oded.gabbay at gmail.com>
>>>>> wrote:
>>>>>> On Sat, Dec 9, 2017 at 6:09 AM, Felix Kuehling
>>>>>> <Felix.Kuehling at amd.com> wrote:
>>>>>>> From: Kent Russell <kent.russell at amd.com>
>>>>>>>
>>>>>>> Add a sysfs file in topology (node/x/memory_banks/X/used_memory)
>>>>>>> that reports the current VRAM usage for that node. Only works for
>>>>>>> GPU nodes at this time.
>>>>>>>
>>>>>> As with patch 22 (perf counters), I would not expect this
>>>>>> information to be included in the topology. It doesn't describe the
>>>>>> properties of the device, but a current state.
>>>>>> Oded
>>>>> For example, in amdgpu, the VRAM usage is reported in the INFO IOCTL
>>>>> (AMDGPU_INFO_VRAM_USAGE). See function  amdgpu_info_ioctl()
>>>> Yep, completely agree.
>>>>
>>>> That stuff is runtime properties and not static attribute nor
>>>> configuration or setup.
>>>>
>>>> So either debugfs or IOCTL are the two best options as far as I can see.
>>> Right. I admit, this feature was a bit of a hack to quickly enable the HIP team without having to change a bunch of interfaces (ioctls, Thunk, and Runtime).
>>>
>>> This patch isn't critical for enabling dGPU support. I'll drop it for now and we can reimplement it properly later.
>>>
>>> Regards,
>>>   Felix
>>>
>>>> Christian.
>>>>
>>>>> Thanks,
>>>>>
>>>>> Oded
>>>>>
>>>>>
>>>>>>> Signed-off-by: Kent Russell <kent.russell at amd.com>
>>>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>>>>>> ---
>>>>>>>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 49
>>>>>>> +++++++++++++++++++++++++++----
>>>>>>>   drivers/gpu/drm/amd/amdkfd/kfd_topology.h |  4 ++-
>>>>>>>   2 files changed, 46 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>>>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>>>>>> index 7f0d41e..7f04038 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>>>>>> @@ -186,6 +186,8 @@ struct kfd_topology_device
>>>>>>> *kfd_create_topology_device(
>>>>>>>                  sysfs_show_gen_prop(buffer, "%s %llu\n", name,
>>>>>>> value)
>>>>>>>   #define sysfs_show_32bit_val(buffer, value) \
>>>>>>>                  sysfs_show_gen_prop(buffer, "%u\n", value)
>>>>>>> +#define sysfs_show_64bit_val(buffer, value) \
>>>>>>> +               sysfs_show_gen_prop(buffer, "%llu\n", value)
>>>>>>>   #define sysfs_show_str_val(buffer, value) \
>>>>>>>                  sysfs_show_gen_prop(buffer, "%s\n", value)
>>>>>>>
>>>>>>> @@ -268,11 +270,23 @@ static ssize_t mem_show(struct kobject *kobj,
>>>>>>> struct attribute *attr,
>>>>>>>   {
>>>>>>>          ssize_t ret;
>>>>>>>          struct kfd_mem_properties *mem;
>>>>>>> +       uint64_t used_mem;
>>>>>>>
>>>>>>>          /* Making sure that the buffer is an empty string */
>>>>>>>          buffer[0] = 0;
>>>>>>>
>>>>>>> -       mem = container_of(attr, struct kfd_mem_properties, attr);
>>>>>>> +       if (strcmp(attr->name, "used_memory") == 0) {
>>>>>>> +               mem = container_of(attr, struct kfd_mem_properties,
>>>>>>> attr_used);
>>>>>>> +               if (mem->gpu) {
>>>>>>> +                       used_mem =
>>>>>>> +mem->gpu->kfd2kgd->get_vram_usage(
>>>>>>> +
>>>>>>> mem->gpu->kgd);
>>>>>>> +                       return sysfs_show_64bit_val(buffer,
>>>>>>> +used_mem);
>>>>>>> +               }
>>>>>>> +               /* TODO: Report APU/CPU-allocated memory; For now
>>>>>>> return 0 */
>>>>>>> +               return 0;
>>>>>>> +       }
>>>>>>> +
>>>>>>> +       mem = container_of(attr, struct kfd_mem_properties,
>>>>>>> attr_props);
>>>>>>>          sysfs_show_32bit_prop(buffer, "heap_type",
>>>>>>> mem->heap_type);
>>>>>>>          sysfs_show_64bit_prop(buffer, "size_in_bytes",
>>>>>>> mem->size_in_bytes);
>>>>>>>          sysfs_show_32bit_prop(buffer, "flags", mem->flags); @@
>>>>>>> -527,7 +541,12 @@ static void kfd_remove_sysfs_node_entry(struct
>>>>>>> kfd_topology_device *dev)
>>>>>>>          if (dev->kobj_mem) {
>>>>>>>                  list_for_each_entry(mem, &dev->mem_props, list)
>>>>>>>                          if (mem->kobj) {
>>>>>>> -                               kfd_remove_sysfs_file(mem->kobj,
>>>>>>> &mem->attr);
>>>>>>> +                               /* TODO: Remove when CPU/APU
>>>>>>> supported */
>>>>>>> +                               if (dev->node_props.cpu_cores_count
>>>>>>> == 0)
>>>>>>> +
>>>>>>> +sysfs_remove_file(mem->kobj,
>>>>>>> +
>>>>>>> &mem->attr_used);
>>>>>>> +                               kfd_remove_sysfs_file(mem->kobj,
>>>>>>> +                                               &mem->attr_props);
>>>>>>>                                  mem->kobj = NULL;
>>>>>>>                          }
>>>>>>>                  kobject_del(dev->kobj_mem); @@ -629,12 +648,23 @@
>>>>>>> static int kfd_build_sysfs_node_entry(struct kfd_topology_device
>>>>>>> *dev,
>>>>>>>                  if (ret < 0)
>>>>>>>                          return ret;
>>>>>>>
>>>>>>> -               mem->attr.name = "properties";
>>>>>>> -               mem->attr.mode = KFD_SYSFS_FILE_MODE;
>>>>>>> -               sysfs_attr_init(&mem->attr);
>>>>>>> -               ret = sysfs_create_file(mem->kobj, &mem->attr);
>>>>>>> +               mem->attr_props.name = "properties";
>>>>>>> +               mem->attr_props.mode = KFD_SYSFS_FILE_MODE;
>>>>>>> +               sysfs_attr_init(&mem->attr_props);
>>>>>>> +               ret = sysfs_create_file(mem->kobj,
>>>>>>> +&mem->attr_props);
>>>>>>>                  if (ret < 0)
>>>>>>>                          return ret;
>>>>>>> +
>>>>>>> +               /* TODO: Support APU/CPU memory usage */
>>>>>>> +               if (dev->node_props.cpu_cores_count == 0) {
>>>>>>> +                       mem->attr_used.name = "used_memory";
>>>>>>> +                       mem->attr_used.mode = KFD_SYSFS_FILE_MODE;
>>>>>>> +                       sysfs_attr_init(&mem->attr_used);
>>>>>>> +                       ret = sysfs_create_file(mem->kobj,
>>>>>>> &mem->attr_used);
>>>>>>> +                       if (ret < 0)
>>>>>>> +                               return ret;
>>>>>>> +               }
>>>>>>> +
>>>>>>>                  i++;
>>>>>>>          }
>>>>>>>
>>>>>>> @@ -1075,15 +1105,22 @@ static struct kfd_topology_device
>>>>>>> *kfd_assign_gpu(struct kfd_dev *gpu)
>>>>>>>   {
>>>>>>>          struct kfd_topology_device *dev;
>>>>>>>          struct kfd_topology_device *out_dev = NULL;
>>>>>>> +       struct kfd_mem_properties *mem;
>>>>>>>
>>>>>>>          down_write(&topology_lock);
>>>>>>>          list_for_each_entry(dev, &topology_device_list, list)
>>>>>>>                  if (!dev->gpu && (dev->node_props.simd_count > 0))
>>>>>>> {
>>>>>>>                          dev->gpu = gpu;
>>>>>>>                          out_dev = dev;
>>>>>>> +
>>>>>>> +                       /* Assign mem->gpu */
>>>>>>> +                       list_for_each_entry(mem, &dev->mem_props,
>>>>>>> list)
>>>>>>> +                               mem->gpu = dev->gpu;
>>>>>>> +
>>>>>>>                          break;
>>>>>>>                  }
>>>>>>>          up_write(&topology_lock);
>>>>>>> +
>>>>>>>          return out_dev;
>>>>>>>   }
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>>>>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>>>>>>> index 53fca1f..0f698d8 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>>>>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>>>>>>> @@ -93,7 +93,9 @@ struct kfd_mem_properties {
>>>>>>>          uint32_t                width;
>>>>>>>          uint32_t                mem_clk_max;
>>>>>>>          struct kobject          *kobj;
>>>>>>> -       struct attribute        attr;
>>>>>>> +       struct kfd_dev          *gpu;
>>>>>>> +       struct attribute        attr_props;
>>>>>>> +       struct attribute        attr_used;
>>>>>>>   };
>>>>>>>
>>>>>>>   #define HSA_CACHE_TYPE_DATA            0x00000001
>>>>>>> --
>>>>>>> 2.7.4
>>>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>


More information about the amd-gfx mailing list