Thanks.
To add to what Kent said, this is for providing per-process information
in rocm-smi or other tools. It should allow us to do something like "ps"
or "top" for compute processes running on the GPU. Currently it's only
the PASID. We'll add per-process memory usage and probably user mode
queue usage as well (there has been a recent request to detect and
report queue oversubscription because it can have severe performance
impact).
The change is Reviewed-by: Felix Kuehling <felix.kuehl...@amd.com>
Regards,
Felix
On 2019-06-19 10:44, Koenig, Christian wrote:
> Ok, that's what I wanted to know. Feel free to add my Acked-by to the patch.
>
> Thanks,
> Christian.
>
> Am 19.06.19 um 16:42 schrieb Russell, Kent:
>> I'd rather it be in debugfs too, but the requirements are that it be exposed
>> through the SMI. And whenever we do anything that requires root for reading
>> in the SMI, people complain (they expect root for writing, but I had dozens
>> of complaints/bug reports when reporting voltage via debugfs required root).
>> That's why we did things like moving the voltage, memory usage, etc to sysfs.
>>
>> So unfortunately it can't go in debugfs, even though that's where I would
>> have preferred it. I know that it kind of locks us in interface-wise though.
>>
>> Kent
>>
>> -----Original Message-----
>> From: Koenig, Christian <christian.koe...@amd.com>
>> Sent: Wednesday, June 19, 2019 10:35 AM
>> To: Russell, Kent <kent.russ...@amd.com>; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdkfd: Add procfs-style information for KFD
>> processes
>>
>> Do we need a stable interface? Would debugfs do as well?
>>
>> I mean in general looks good for sysfs as well, just want to double check.
>>
>> Christian.
>>
>> Am 19.06.19 um 16:28 schrieb Russell, Kent:
>>> Right now the use case would be to list which processes were created in a
>>> KFD context, but it would allow for further expansion to include things
>>> like the GPU associated with the PID, memory usage, etc. For now, the use
>>> case is listing KFD-related PIDs, but will be expanded later to include
>>> memory usage for sure (plus other things that I expect will requested later
>>> on).
>>>
>>> Kent
>>>
>>> -----Original Message-----
>>> From: Christian König <ckoenig.leichtzumer...@gmail.com>
>>> Sent: Wednesday, June 19, 2019 10:04 AM
>>> To: Russell, Kent <kent.russ...@amd.com>;
>>> amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH] drm/amdkfd: Add procfs-style information for KFD
>>> processes
>>>
>>> Am 19.06.19 um 16:01 schrieb Russell, Kent:
>>>> Add a folder structure to /sys/class/kfd/kfd/ called proc which
>>>> contains subfolders, each representing an active KFD process' PID,
>>>> containing 1
>>>> file: pasid.
>>> What is the use case of that information? In other words would it be maybe
>>> better to create debugfs entries instead?
>>>
>>> Christian.
>>>
>>>> Change-Id: Id3dfab8a6250264434b34ccddbcdb459d1da7478
>>>> Signed-off-by: Kent Russell <kent.russ...@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdkfd/kfd_module.c | 6 ++
>>>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 ++
>>>> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 100 ++++++++++++++++++++++-
>>>> 3 files changed, 113 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
>>>> index 932007eb9168..986ff52d5750 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
>>>> @@ -56,6 +56,11 @@ static int kfd_init(void)
>>>> if (err < 0)
>>>> goto err_create_wq;
>>>>
>>>> + /* Ignore the return value, so that we can continue
>>>> + * to init the KFD, even if procfs isn't craated
>>>> + */
>>>> + kfd_procfs_init();
>>>> +
>>>> kfd_debugfs_init();
>>>>
>>>> return 0;
>>>> @@ -72,6 +77,7 @@ static void kfd_exit(void)
>>>> {
>>>> kfd_debugfs_fini();
>>>> kfd_process_destroy_wq();
>>>> + kfd_procfs_shutdown();
>>>> kfd_topology_shutdown();
>>>> kfd_chardev_exit();
>>>> }
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> index da589ee1366c..bd01396c8cea 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>>> @@ -35,6 +35,7 @@
>>>> #include <linux/kfifo.h>
>>>> #include <linux/seq_file.h>
>>>> #include <linux/kref.h>
>>>> +#include <linux/sysfs.h>
>>>> #include <kgd_kfd_interface.h>
>>>>
>>>> #include "amd_shared.h"
>>>> @@ -718,6 +719,10 @@ struct kfd_process {
>>>> * restored after an eviction
>>>> */
>>>> unsigned long last_restore_timestamp;
>>>> +
>>>> + /* Kobj for our procfs */
>>>> + struct kobject *kobj;
>>>> + struct attribute attr_pasid;
>>>> };
>>>>
>>>> #define KFD_PROCESS_TABLE_SIZE 5 /* bits: 32 entries */ @@ -820,6
>>>> +825,10 @@ int kfd_gtt_sa_free(struct kfd_dev *kfd, struct
>>>> +kfd_mem_obj
>>>> *mem_obj);
>>>>
>>>> extern struct device *kfd_device;
>>>>
>>>> +/* KFD's procfs */
>>>> +void kfd_procfs_init(void);
>>>> +void kfd_procfs_shutdown(void);
>>>> +
>>>> /* Topology */
>>>> int kfd_topology_init(void);
>>>> void kfd_topology_shutdown(void);
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>>>> index 4bdae78bab8e..ed2d83f93fd8 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>>>> @@ -68,6 +68,68 @@ static struct kfd_process *create_process(const struct
>>>> task_struct *thread,
>>>> static void evict_process_worker(struct work_struct *work);
>>>> static void restore_process_worker(struct work_struct *work);
>>>>
>>>> +struct kfd_procfs_tree {
>>>> + struct kobject *kobj;
>>>> +};
>>>> +
>>>> +static struct kfd_procfs_tree procfs;
>>>> +
>>>> +static ssize_t kfd_procfs_show(struct kobject *kobj, struct attribute
>>>> *attr,
>>>> + char *buffer)
>>>> +{
>>>> + int val = 0;
>>>> +
>>>> + if (strcmp(attr->name, "pasid") == 0) {
>>>> + struct kfd_process *p = container_of(attr, struct kfd_process,
>>>> + attr_pasid);
>>>> + val = p->pasid;
>>>> + } else {
>>>> + pr_err("Invalid attribute");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + return snprintf(buffer, PAGE_SIZE, "%d\n", val); }
>>>> +
>>>> +static void kfd_procfs_kobj_release(struct kobject *kobj) {
>>>> + kfree(kobj);
>>>> +}
>>>> +
>>>> +static const struct sysfs_ops kfd_procfs_ops = {
>>>> + .show = kfd_procfs_show,
>>>> +};
>>>> +
>>>> +static struct kobj_type procfs_type = {
>>>> + .release = kfd_procfs_kobj_release,
>>>> + .sysfs_ops = &kfd_procfs_ops,
>>>> +};
>>>> +
>>>> +void kfd_procfs_init(void)
>>>> +{
>>>> + int ret = 0;
>>>> +
>>>> + procfs.kobj = kfd_alloc_struct(procfs.kobj);
>>>> + if (!procfs.kobj)
>>>> + return;
>>>> +
>>>> + ret = kobject_init_and_add(procfs.kobj, &procfs_type,
>>>> + &kfd_device->kobj, "proc");
>>>> + if (ret) {
>>>> + pr_warn("Could not create procfs proc folder");
>>>> + /* If we fail to create the procfs, clean up */
>>>> + kfd_procfs_shutdown();
>>>> + }
>>>> +}
>>>> +
>>>> +void kfd_procfs_shutdown(void)
>>>> +{
>>>> + if (procfs.kobj) {
>>>> + kobject_del(procfs.kobj);
>>>> + kobject_put(procfs.kobj);
>>>> + procfs.kobj = NULL;
>>>> + }
>>>> +}
>>>>
>>>> int kfd_process_create_wq(void)
>>>> {
>>>> @@ -206,6 +268,7 @@ struct kfd_process *kfd_create_process(struct file
>>>> *filep)
>>>> {
>>>> struct kfd_process *process;
>>>> struct task_struct *thread = current;
>>>> + int ret;
>>>>
>>>> if (!thread->mm)
>>>> return ERR_PTR(-EINVAL);
>>>> @@ -223,11 +286,36 @@ struct kfd_process *kfd_create_process(struct
>>>> file *filep)
>>>>
>>>> /* A prior open of /dev/kfd could have already created the
>>>> process. */
>>>> process = find_process(thread);
>>>> - if (process)
>>>> + if (process) {
>>>> pr_debug("Process already found\n");
>>>> - else
>>>> + } else {
>>>> process = create_process(thread, filep);
>>>>
>>>> + if (!procfs.kobj)
>>>> + goto out;
>>>> +
>>>> + process->kobj = kfd_alloc_struct(process->kobj);
>>>> + if (!process->kobj) {
>>>> + pr_warn("Creating procfs kobject failed");
>>>> + goto out;
>>>> + }
>>>> + ret = kobject_init_and_add(process->kobj, &procfs_type,
>>>> + procfs.kobj, "%d",
>>>> + (int)process->lead_thread->pid);
>>>> + if (ret) {
>>>> + pr_warn("Creating procfs pid directory failed");
>>>> + goto out;
>>>> + }
>>>> +
>>>> + process->attr_pasid.name = "pasid";
>>>> + process->attr_pasid.mode = KFD_SYSFS_FILE_MODE;
>>>> + sysfs_attr_init(&process->attr_pasid);
>>>> + ret = sysfs_create_file(process->kobj, &process->attr_pasid);
>>>> + if (ret)
>>>> + pr_warn("Creating pasid for pid %d failed",
>>>> + (int)process->lead_thread->pid);
>>>> + }
>>>> +out:
>>>> mutex_unlock(&kfd_processes_mutex);
>>>>
>>>> return process;
>>>> @@ -355,6 +443,14 @@ static void kfd_process_wq_release(struct work_struct
>>>> *work)
>>>> struct kfd_process *p = container_of(work, struct kfd_process,
>>>> release_work);
>>>>
>>>> + /* Remove the procfs files */
>>>> + if (p->kobj) {
>>>> + sysfs_remove_file(p->kobj, &p->attr_pasid);
>>>> + kobject_del(p->kobj);
>>>> + kobject_put(p->kobj);
>>>> + p->kobj = NULL;
>>>> + }
>>>> +
>>>> kfd_iommu_unbind_process(p);
>>>>
>>>> kfd_process_free_outstanding_kfd_bos(p);
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
