* [PATCH v2] drm/amdkfd: Add queue information to sysfs @ 2020-01-31 13:45 Amber Lin 2020-01-31 17:06 ` Felix Kuehling 0 siblings, 1 reply; 4+ messages in thread From: Amber Lin @ 2020-01-31 13:45 UTC (permalink / raw) To: amd-gfx; +Cc: Amber Lin Provide compute queues information in sysfs under /sys/class/kfd/kfd/proc. The format is /sys/class/kfd/kfd/proc/<pid>/queues/<queue id>/XX where XX are size, type, and gpuid three files to represent queue size, queue type, and the GPU this queue uses. <queue id> folder and files underneath are generated when a queue is created. They are removed when the queue is destroyed. Signed-off-by: Amber Lin <Amber.Lin@amd.com> --- drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 ++ drivers/gpu/drm/amd/amdkfd/kfd_process.c | 96 ++++++++++++++++++++++ .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 + 3 files changed, 107 insertions(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index c0b0def..cb2d2d7 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -503,6 +503,12 @@ struct queue { struct kfd_process *process; struct kfd_dev *device; void *gws; + + /* procfs */ + struct kobject *kobj_qid; + struct attribute attr_size; + struct attribute attr_type; + struct attribute attr_gpuid; }; /* @@ -730,6 +736,7 @@ struct kfd_process { /* Kobj for our procfs */ struct kobject *kobj; + struct kobject *kobj_queues; struct attribute attr_pasid; }; @@ -836,6 +843,8 @@ extern struct device *kfd_device; /* KFD's procfs */ void kfd_procfs_init(void); void kfd_procfs_shutdown(void); +int kfd_procfs_add_queue(struct queue *q); +void kfd_procfs_del_queue(struct queue *q); /* Topology */ int kfd_topology_init(void); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c index 25b90f7..78ca037 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c @@ -132,6 +132,94 @@ void kfd_procfs_shutdown(void) } } +static int kfd_procfs_add_file(const char *name, struct kobject *kobj, + struct attribute *attr) +{ + int ret; + + attr->name = name; + attr->mode = KFD_SYSFS_FILE_MODE; + sysfs_attr_init(attr); + ret = sysfs_create_file(kobj, attr); + if (ret) + pr_warn("Creating %s file failed", name); + return ret; +} + +static ssize_t kfd_procfs_queue_show(struct kobject *kobj, + struct attribute *attr, char *buffer) +{ + if (!strcmp(attr->name, "size")) { + struct queue *q = container_of(attr, struct queue, attr_size); + return snprintf(buffer, PAGE_SIZE, "%llu", + q->properties.queue_size); + } else if (!strcmp(attr->name, "type")) { + struct queue *q = container_of(attr, struct queue, attr_type); + return snprintf(buffer, PAGE_SIZE, "%d", q->properties.type); + } else if (!strcmp(attr->name, "gpuid")) { + struct queue *q = container_of(attr, struct queue, attr_gpuid); + return snprintf(buffer, PAGE_SIZE, "%u", q->device->id); + } else + pr_err("Invalid attribute"); + + return 0; +} + +static const struct sysfs_ops procfs_queue_ops = { + .show = kfd_procfs_queue_show, +}; + +static struct kobj_type procfs_queue_type = { + .release = kfd_procfs_kobj_release, + .sysfs_ops = &procfs_queue_ops, +}; + +int kfd_procfs_add_queue(struct queue *q) +{ + struct kfd_process *proc; + int ret; + + if (!q || !q->process) + return -EINVAL; + proc = q->process; + + /* Create proc/<pid>/queues/<queue id> folder*/ + if (!proc->kobj_queues) + return -EFAULT; + if (q->kobj_qid) + return -EEXIST; + q->kobj_qid = kfd_alloc_struct(q->kobj_qid); + if (!q->kobj_qid) + return -ENOMEM; + ret = kobject_init_and_add(q->kobj_qid, &procfs_queue_type, + proc->kobj_queues, "%u", q->properties.queue_id); + if (ret < 0) { + pr_warn("Creating proc/<pid>/queues/%u failed", + q->properties.queue_id); + return ret; + } + + /* Create proc/<pid>/queues/<queue id>/XX files */ + kfd_procfs_add_file("size", q->kobj_qid, &q->attr_size); + kfd_procfs_add_file("type", q->kobj_qid, &q->attr_type); + kfd_procfs_add_file("gpuid", q->kobj_qid, &q->attr_gpuid); + + return 0; +} + +void kfd_procfs_del_queue(struct queue *q) +{ + if (!q || !q->process) + return; + + sysfs_remove_file(q->kobj_qid, &q->attr_size); + sysfs_remove_file(q->kobj_qid, &q->attr_type); + sysfs_remove_file(q->kobj_qid, &q->attr_gpuid); + kobject_del(q->kobj_qid); + kobject_put(q->kobj_qid); + q->kobj_qid = NULL; +} + int kfd_process_create_wq(void) { if (!kfd_process_wq) @@ -323,6 +411,11 @@ struct kfd_process *kfd_create_process(struct file *filep) if (ret) pr_warn("Creating pasid for pid %d failed", (int)process->lead_thread->pid); + + process->kobj_queues = kobject_create_and_add("queues", + process->kobj); + if (!process->kobj_queues) + pr_warn("Creating KFD proc/queues folder failed"); } out: if (!IS_ERR(process)) @@ -457,6 +550,9 @@ static void kfd_process_wq_release(struct work_struct *work) /* Remove the procfs files */ if (p->kobj) { sysfs_remove_file(p->kobj, &p->attr_pasid); + kobject_del(p->kobj_queues); + kobject_put(p->kobj_queues); + p->kobj_queues = NULL; kobject_del(p->kobj); kobject_put(p->kobj); p->kobj = NULL; diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c index 8fa856e..cb1ca11 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c @@ -322,6 +322,7 @@ int pqm_create_queue(struct process_queue_manager *pqm, if (q) { pr_debug("PQM done creating queue\n"); + kfd_procfs_add_queue(q); print_queue_properties(&q->properties); } @@ -378,6 +379,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid) } if (pqn->q) { + kfd_procfs_del_queue(pqn->q); dqm = pqn->q->device->dqm; retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q); if (retval) { -- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/amdkfd: Add queue information to sysfs 2020-01-31 13:45 [PATCH v2] drm/amdkfd: Add queue information to sysfs Amber Lin @ 2020-01-31 17:06 ` Felix Kuehling 2020-01-31 18:47 ` Lin, Amber 0 siblings, 1 reply; 4+ messages in thread From: Felix Kuehling @ 2020-01-31 17:06 UTC (permalink / raw) To: Amber Lin, amd-gfx You could save yourself the trouble of manually adding and removed individual sysfs files by setting the default_attrs in the kobj_type. See ttm_memory.c for an example how this is done. More comments inline. On 2020-01-31 8:45 a.m., Amber Lin wrote: > Provide compute queues information in sysfs under /sys/class/kfd/kfd/proc. > The format is /sys/class/kfd/kfd/proc/<pid>/queues/<queue id>/XX where > XX are size, type, and gpuid three files to represent queue size, queue > type, and the GPU this queue uses. <queue id> folder and files underneath > are generated when a queue is created. They are removed when the queue is > destroyed. > > Signed-off-by: Amber Lin <Amber.Lin@amd.com> > --- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 ++ > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 96 ++++++++++++++++++++++ > .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 + > 3 files changed, 107 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index c0b0def..cb2d2d7 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -503,6 +503,12 @@ struct queue { > struct kfd_process *process; > struct kfd_dev *device; > void *gws; > + > + /* procfs */ > + struct kobject *kobj_qid; > + struct attribute attr_size; > + struct attribute attr_type; > + struct attribute attr_gpuid; > }; > > /* > @@ -730,6 +736,7 @@ struct kfd_process { > > /* Kobj for our procfs */ > struct kobject *kobj; > + struct kobject *kobj_queues; > struct attribute attr_pasid; > }; > > @@ -836,6 +843,8 @@ extern struct device *kfd_device; > /* KFD's procfs */ > void kfd_procfs_init(void); > void kfd_procfs_shutdown(void); > +int kfd_procfs_add_queue(struct queue *q); > +void kfd_procfs_del_queue(struct queue *q); > > /* Topology */ > int kfd_topology_init(void); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c > index 25b90f7..78ca037 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c > @@ -132,6 +132,94 @@ void kfd_procfs_shutdown(void) > } > } > > +static int kfd_procfs_add_file(const char *name, struct kobject *kobj, > + struct attribute *attr) > +{ > + int ret; > + > + attr->name = name; > + attr->mode = KFD_SYSFS_FILE_MODE; > + sysfs_attr_init(attr); > + ret = sysfs_create_file(kobj, attr); > + if (ret) > + pr_warn("Creating %s file failed", name); > + return ret; > +} > + > +static ssize_t kfd_procfs_queue_show(struct kobject *kobj, > + struct attribute *attr, char *buffer) > +{ > + if (!strcmp(attr->name, "size")) { > + struct queue *q = container_of(attr, struct queue, attr_size); > + return snprintf(buffer, PAGE_SIZE, "%llu", > + q->properties.queue_size); > + } else if (!strcmp(attr->name, "type")) { > + struct queue *q = container_of(attr, struct queue, attr_type); > + return snprintf(buffer, PAGE_SIZE, "%d", q->properties.type); > + } else if (!strcmp(attr->name, "gpuid")) { > + struct queue *q = container_of(attr, struct queue, attr_gpuid); > + return snprintf(buffer, PAGE_SIZE, "%u", q->device->id); > + } else > + pr_err("Invalid attribute"); > + > + return 0; > +} > + > +static const struct sysfs_ops procfs_queue_ops = { > + .show = kfd_procfs_queue_show, > +}; > + > +static struct kobj_type procfs_queue_type = { > + .release = kfd_procfs_kobj_release, > + .sysfs_ops = &procfs_queue_ops, > +}; > + > +int kfd_procfs_add_queue(struct queue *q) > +{ > + struct kfd_process *proc; > + int ret; > + > + if (!q || !q->process) > + return -EINVAL; > + proc = q->process; > + > + /* Create proc/<pid>/queues/<queue id> folder*/ > + if (!proc->kobj_queues) > + return -EFAULT; > + if (q->kobj_qid) > + return -EEXIST; > + q->kobj_qid = kfd_alloc_struct(q->kobj_qid); > + if (!q->kobj_qid) > + return -ENOMEM; > + ret = kobject_init_and_add(q->kobj_qid, &procfs_queue_type, > + proc->kobj_queues, "%u", q->properties.queue_id); > + if (ret < 0) { > + pr_warn("Creating proc/<pid>/queues/%u failed", > + q->properties.queue_id); After kobject_init_and_add fails, you must call kobject_put to avoid memory leaks. > + return ret; > + } > + > + /* Create proc/<pid>/queues/<queue id>/XX files */ > + kfd_procfs_add_file("size", q->kobj_qid, &q->attr_size); > + kfd_procfs_add_file("type", q->kobj_qid, &q->attr_type); > + kfd_procfs_add_file("gpuid", q->kobj_qid, &q->attr_gpuid); > + > + return 0; > +} > + > +void kfd_procfs_del_queue(struct queue *q) > +{ > + if (!q || !q->process) > + return; You need to check q->obj_qid too, in case kfd_procfs_add_queue failed. Regards, Felix > + > + sysfs_remove_file(q->kobj_qid, &q->attr_size); > + sysfs_remove_file(q->kobj_qid, &q->attr_type); > + sysfs_remove_file(q->kobj_qid, &q->attr_gpuid); > + kobject_del(q->kobj_qid); > + kobject_put(q->kobj_qid); > + q->kobj_qid = NULL; > +} > + > int kfd_process_create_wq(void) > { > if (!kfd_process_wq) > @@ -323,6 +411,11 @@ struct kfd_process *kfd_create_process(struct file *filep) > if (ret) > pr_warn("Creating pasid for pid %d failed", > (int)process->lead_thread->pid); > + > + process->kobj_queues = kobject_create_and_add("queues", > + process->kobj); > + if (!process->kobj_queues) > + pr_warn("Creating KFD proc/queues folder failed"); > } > out: > if (!IS_ERR(process)) > @@ -457,6 +550,9 @@ static void kfd_process_wq_release(struct work_struct *work) > /* Remove the procfs files */ > if (p->kobj) { > sysfs_remove_file(p->kobj, &p->attr_pasid); > + kobject_del(p->kobj_queues); > + kobject_put(p->kobj_queues); > + p->kobj_queues = NULL; > kobject_del(p->kobj); > kobject_put(p->kobj); > p->kobj = NULL; > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c > index 8fa856e..cb1ca11 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c > @@ -322,6 +322,7 @@ int pqm_create_queue(struct process_queue_manager *pqm, > > if (q) { > pr_debug("PQM done creating queue\n"); > + kfd_procfs_add_queue(q); > print_queue_properties(&q->properties); > } > > @@ -378,6 +379,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid) > } > > if (pqn->q) { > + kfd_procfs_del_queue(pqn->q); > dqm = pqn->q->device->dqm; > retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q); > if (retval) { _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] drm/amdkfd: Add queue information to sysfs 2020-01-31 17:06 ` Felix Kuehling @ 2020-01-31 18:47 ` Lin, Amber 2020-01-31 19:22 ` Felix Kuehling 0 siblings, 1 reply; 4+ messages in thread From: Lin, Amber @ 2020-01-31 18:47 UTC (permalink / raw) To: Kuehling, Felix, amd-gfx@lists.freedesktop.org [AMD Official Use Only - Internal Distribution Only] It doesn't apply to this one because 1. It only has one set of attribute (dma32 or highmem) using the kobj_type, so it can set the default_attrs. In my case, I have multiple queues/QIDs that share the same kobj_type while each of them has their own attrs located in "struct queue". I can't assign default_attrs to a specific one like ttm_memory.c does in the global section. 2. I also looked into kobj_attribute see if I can simply use sysfs_create_group (instead of sysfs_create_file three times) like how KFD implements DF and topology perf. The challenge is it needs a pre-declared attrs set but in my case, queues are created dynamically so I can't pre-declare them unless I can predict the number of queues. Attr sets for DF and perf are both a fixed number. They both declare the attr sets in global data before the function calls sysfs_create_group while I can't do that in this case due to queues are dynamically generated. Thanks for the two inline comments. I'll fix them and submit again. Regards, Amber -----Original Message----- From: Kuehling, Felix <Felix.Kuehling@amd.com> Sent: Friday, January 31, 2020 12:06 PM To: Lin, Amber <Amber.Lin@amd.com>; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH v2] drm/amdkfd: Add queue information to sysfs You could save yourself the trouble of manually adding and removed individual sysfs files by setting the default_attrs in the kobj_type. See ttm_memory.c for an example how this is done. More comments inline. On 2020-01-31 8:45 a.m., Amber Lin wrote: > Provide compute queues information in sysfs under /sys/class/kfd/kfd/proc. > The format is /sys/class/kfd/kfd/proc/<pid>/queues/<queue id>/XX where > XX are size, type, and gpuid three files to represent queue size, > queue type, and the GPU this queue uses. <queue id> folder and files > underneath are generated when a queue is created. They are removed > when the queue is destroyed. > > Signed-off-by: Amber Lin <Amber.Lin@amd.com> > --- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 ++ > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 96 ++++++++++++++++++++++ > .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 + > 3 files changed, 107 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index c0b0def..cb2d2d7 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -503,6 +503,12 @@ struct queue { > struct kfd_process *process; > struct kfd_dev *device; > void *gws; > + > + /* procfs */ > + struct kobject *kobj_qid; > + struct attribute attr_size; > + struct attribute attr_type; > + struct attribute attr_gpuid; > }; > > /* > @@ -730,6 +736,7 @@ struct kfd_process { > > /* Kobj for our procfs */ > struct kobject *kobj; > + struct kobject *kobj_queues; > struct attribute attr_pasid; > }; > > @@ -836,6 +843,8 @@ extern struct device *kfd_device; > /* KFD's procfs */ > void kfd_procfs_init(void); > void kfd_procfs_shutdown(void); > +int kfd_procfs_add_queue(struct queue *q); void > +kfd_procfs_del_queue(struct queue *q); > > /* Topology */ > int kfd_topology_init(void); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c > b/drivers/gpu/drm/amd/amdkfd/kfd_process.c > index 25b90f7..78ca037 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c > @@ -132,6 +132,94 @@ void kfd_procfs_shutdown(void) > } > } > > +static int kfd_procfs_add_file(const char *name, struct kobject *kobj, > + struct attribute *attr) > +{ > + int ret; > + > + attr->name = name; > + attr->mode = KFD_SYSFS_FILE_MODE; > + sysfs_attr_init(attr); > + ret = sysfs_create_file(kobj, attr); > + if (ret) > + pr_warn("Creating %s file failed", name); > + return ret; > +} > + > +static ssize_t kfd_procfs_queue_show(struct kobject *kobj, > + struct attribute *attr, char *buffer) { > + if (!strcmp(attr->name, "size")) { > + struct queue *q = container_of(attr, struct queue, attr_size); > + return snprintf(buffer, PAGE_SIZE, "%llu", > + q->properties.queue_size); > + } else if (!strcmp(attr->name, "type")) { > + struct queue *q = container_of(attr, struct queue, attr_type); > + return snprintf(buffer, PAGE_SIZE, "%d", q->properties.type); > + } else if (!strcmp(attr->name, "gpuid")) { > + struct queue *q = container_of(attr, struct queue, attr_gpuid); > + return snprintf(buffer, PAGE_SIZE, "%u", q->device->id); > + } else > + pr_err("Invalid attribute"); > + > + return 0; > +} > + > +static const struct sysfs_ops procfs_queue_ops = { > + .show = kfd_procfs_queue_show, > +}; > + > +static struct kobj_type procfs_queue_type = { > + .release = kfd_procfs_kobj_release, > + .sysfs_ops = &procfs_queue_ops, > +}; > + > +int kfd_procfs_add_queue(struct queue *q) { > + struct kfd_process *proc; > + int ret; > + > + if (!q || !q->process) > + return -EINVAL; > + proc = q->process; > + > + /* Create proc/<pid>/queues/<queue id> folder*/ > + if (!proc->kobj_queues) > + return -EFAULT; > + if (q->kobj_qid) > + return -EEXIST; > + q->kobj_qid = kfd_alloc_struct(q->kobj_qid); > + if (!q->kobj_qid) > + return -ENOMEM; > + ret = kobject_init_and_add(q->kobj_qid, &procfs_queue_type, > + proc->kobj_queues, "%u", q->properties.queue_id); > + if (ret < 0) { > + pr_warn("Creating proc/<pid>/queues/%u failed", > + q->properties.queue_id); After kobject_init_and_add fails, you must call kobject_put to avoid memory leaks. > + return ret; > + } > + > + /* Create proc/<pid>/queues/<queue id>/XX files */ > + kfd_procfs_add_file("size", q->kobj_qid, &q->attr_size); > + kfd_procfs_add_file("type", q->kobj_qid, &q->attr_type); > + kfd_procfs_add_file("gpuid", q->kobj_qid, &q->attr_gpuid); > + > + return 0; > +} > + > +void kfd_procfs_del_queue(struct queue *q) > +{ > + if (!q || !q->process) > + return; You need to check q->obj_qid too, in case kfd_procfs_add_queue failed. Regards, Felix > + > + sysfs_remove_file(q->kobj_qid, &q->attr_size); > + sysfs_remove_file(q->kobj_qid, &q->attr_type); > + sysfs_remove_file(q->kobj_qid, &q->attr_gpuid); > + kobject_del(q->kobj_qid); > + kobject_put(q->kobj_qid); > + q->kobj_qid = NULL; > +} > + > int kfd_process_create_wq(void) > { > if (!kfd_process_wq) > @@ -323,6 +411,11 @@ struct kfd_process *kfd_create_process(struct file *filep) > if (ret) > pr_warn("Creating pasid for pid %d failed", > (int)process->lead_thread->pid); > + > + process->kobj_queues = kobject_create_and_add("queues", > + process->kobj); > + if (!process->kobj_queues) > + pr_warn("Creating KFD proc/queues folder failed"); > } > out: > if (!IS_ERR(process)) > @@ -457,6 +550,9 @@ static void kfd_process_wq_release(struct work_struct *work) > /* Remove the procfs files */ > if (p->kobj) { > sysfs_remove_file(p->kobj, &p->attr_pasid); > + kobject_del(p->kobj_queues); > + kobject_put(p->kobj_queues); > + p->kobj_queues = NULL; > kobject_del(p->kobj); > kobject_put(p->kobj); > p->kobj = NULL; > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c > index 8fa856e..cb1ca11 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c > @@ -322,6 +322,7 @@ int pqm_create_queue(struct process_queue_manager *pqm, > > if (q) { > pr_debug("PQM done creating queue\n"); > + kfd_procfs_add_queue(q); > print_queue_properties(&q->properties); > } > > @@ -378,6 +379,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid) > } > > if (pqn->q) { > + kfd_procfs_del_queue(pqn->q); > dqm = pqn->q->device->dqm; > retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q); > if (retval) { _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/amdkfd: Add queue information to sysfs 2020-01-31 18:47 ` Lin, Amber @ 2020-01-31 19:22 ` Felix Kuehling 0 siblings, 0 replies; 4+ messages in thread From: Felix Kuehling @ 2020-01-31 19:22 UTC (permalink / raw) To: Lin, Amber, amd-gfx@lists.freedesktop.org On 2020-01-31 1:47 p.m., Lin, Amber wrote: > [AMD Official Use Only - Internal Distribution Only] > > It doesn't apply to this one because > 1. It only has one set of attribute (dma32 or highmem) using the kobj_type, so it can set the default_attrs. In my case, I have multiple queues/QIDs that share the same kobj_type while each of them has their own attrs located in "struct queue". I can't assign default_attrs to a specific one like ttm_memory.c does in the global section. That's because you use container_of to find the queue that the attributes belong to. Instead you could use container_of to find the queue that the kobj belongs to. So instead of dynamically allocating the kobj, it would be a member of queue. Then you could use the default_attrs. Regards, Felix > 2. I also looked into kobj_attribute see if I can simply use sysfs_create_group (instead of sysfs_create_file three times) like how KFD implements DF and topology perf. The challenge is it needs a pre-declared attrs set but in my case, queues are created dynamically so I can't pre-declare them unless I can predict the number of queues. Attr sets for DF and perf are both a fixed number. They both declare the attr sets in global data before the function calls sysfs_create_group while I can't do that in this case due to queues are dynamically generated. > > Thanks for the two inline comments. I'll fix them and submit again. > > Regards, > Amber > > -----Original Message----- > From: Kuehling, Felix <Felix.Kuehling@amd.com> > Sent: Friday, January 31, 2020 12:06 PM > To: Lin, Amber <Amber.Lin@amd.com>; amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH v2] drm/amdkfd: Add queue information to sysfs > > You could save yourself the trouble of manually adding and removed individual sysfs files by setting the default_attrs in the kobj_type. > See ttm_memory.c for an example how this is done. > > More comments inline. > > On 2020-01-31 8:45 a.m., Amber Lin wrote: >> Provide compute queues information in sysfs under /sys/class/kfd/kfd/proc. >> The format is /sys/class/kfd/kfd/proc/<pid>/queues/<queue id>/XX where >> XX are size, type, and gpuid three files to represent queue size, >> queue type, and the GPU this queue uses. <queue id> folder and files >> underneath are generated when a queue is created. They are removed >> when the queue is destroyed. >> >> Signed-off-by: Amber Lin <Amber.Lin@amd.com> >> --- >> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 ++ >> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 96 ++++++++++++++++++++++ >> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 + >> 3 files changed, 107 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h >> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h >> index c0b0def..cb2d2d7 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h >> @@ -503,6 +503,12 @@ struct queue { >> struct kfd_process *process; >> struct kfd_dev *device; >> void *gws; >> + >> + /* procfs */ >> + struct kobject *kobj_qid; >> + struct attribute attr_size; >> + struct attribute attr_type; >> + struct attribute attr_gpuid; >> }; >> >> /* >> @@ -730,6 +736,7 @@ struct kfd_process { >> >> /* Kobj for our procfs */ >> struct kobject *kobj; >> + struct kobject *kobj_queues; >> struct attribute attr_pasid; >> }; >> >> @@ -836,6 +843,8 @@ extern struct device *kfd_device; >> /* KFD's procfs */ >> void kfd_procfs_init(void); >> void kfd_procfs_shutdown(void); >> +int kfd_procfs_add_queue(struct queue *q); void >> +kfd_procfs_del_queue(struct queue *q); >> >> /* Topology */ >> int kfd_topology_init(void); >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> index 25b90f7..78ca037 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> @@ -132,6 +132,94 @@ void kfd_procfs_shutdown(void) >> } >> } >> >> +static int kfd_procfs_add_file(const char *name, struct kobject *kobj, >> + struct attribute *attr) >> +{ >> + int ret; >> + >> + attr->name = name; >> + attr->mode = KFD_SYSFS_FILE_MODE; >> + sysfs_attr_init(attr); >> + ret = sysfs_create_file(kobj, attr); >> + if (ret) >> + pr_warn("Creating %s file failed", name); >> + return ret; >> +} >> + >> +static ssize_t kfd_procfs_queue_show(struct kobject *kobj, >> + struct attribute *attr, char *buffer) { >> + if (!strcmp(attr->name, "size")) { >> + struct queue *q = container_of(attr, struct queue, attr_size); >> + return snprintf(buffer, PAGE_SIZE, "%llu", >> + q->properties.queue_size); >> + } else if (!strcmp(attr->name, "type")) { >> + struct queue *q = container_of(attr, struct queue, attr_type); >> + return snprintf(buffer, PAGE_SIZE, "%d", q->properties.type); >> + } else if (!strcmp(attr->name, "gpuid")) { >> + struct queue *q = container_of(attr, struct queue, attr_gpuid); >> + return snprintf(buffer, PAGE_SIZE, "%u", q->device->id); >> + } else >> + pr_err("Invalid attribute"); >> + >> + return 0; >> +} >> + >> +static const struct sysfs_ops procfs_queue_ops = { >> + .show = kfd_procfs_queue_show, >> +}; >> + >> +static struct kobj_type procfs_queue_type = { >> + .release = kfd_procfs_kobj_release, >> + .sysfs_ops = &procfs_queue_ops, >> +}; >> + >> +int kfd_procfs_add_queue(struct queue *q) { >> + struct kfd_process *proc; >> + int ret; >> + >> + if (!q || !q->process) >> + return -EINVAL; >> + proc = q->process; >> + >> + /* Create proc/<pid>/queues/<queue id> folder*/ >> + if (!proc->kobj_queues) >> + return -EFAULT; >> + if (q->kobj_qid) >> + return -EEXIST; >> + q->kobj_qid = kfd_alloc_struct(q->kobj_qid); >> + if (!q->kobj_qid) >> + return -ENOMEM; >> + ret = kobject_init_and_add(q->kobj_qid, &procfs_queue_type, >> + proc->kobj_queues, "%u", q->properties.queue_id); >> + if (ret < 0) { >> + pr_warn("Creating proc/<pid>/queues/%u failed", >> + q->properties.queue_id); > After kobject_init_and_add fails, you must call kobject_put to avoid memory leaks. > > >> + return ret; >> + } >> + >> + /* Create proc/<pid>/queues/<queue id>/XX files */ >> + kfd_procfs_add_file("size", q->kobj_qid, &q->attr_size); >> + kfd_procfs_add_file("type", q->kobj_qid, &q->attr_type); >> + kfd_procfs_add_file("gpuid", q->kobj_qid, &q->attr_gpuid); >> + >> + return 0; >> +} >> + >> +void kfd_procfs_del_queue(struct queue *q) >> +{ >> + if (!q || !q->process) >> + return; > You need to check q->obj_qid too, in case kfd_procfs_add_queue failed. > > Regards, > Felix > >> + >> + sysfs_remove_file(q->kobj_qid, &q->attr_size); >> + sysfs_remove_file(q->kobj_qid, &q->attr_type); >> + sysfs_remove_file(q->kobj_qid, &q->attr_gpuid); >> + kobject_del(q->kobj_qid); >> + kobject_put(q->kobj_qid); >> + q->kobj_qid = NULL; >> +} >> + >> int kfd_process_create_wq(void) >> { >> if (!kfd_process_wq) >> @@ -323,6 +411,11 @@ struct kfd_process *kfd_create_process(struct file *filep) >> if (ret) >> pr_warn("Creating pasid for pid %d failed", >> (int)process->lead_thread->pid); >> + >> + process->kobj_queues = kobject_create_and_add("queues", >> + process->kobj); >> + if (!process->kobj_queues) >> + pr_warn("Creating KFD proc/queues folder failed"); >> } >> out: >> if (!IS_ERR(process)) >> @@ -457,6 +550,9 @@ static void kfd_process_wq_release(struct work_struct *work) >> /* Remove the procfs files */ >> if (p->kobj) { >> sysfs_remove_file(p->kobj, &p->attr_pasid); >> + kobject_del(p->kobj_queues); >> + kobject_put(p->kobj_queues); >> + p->kobj_queues = NULL; >> kobject_del(p->kobj); >> kobject_put(p->kobj); >> p->kobj = NULL; >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c >> index 8fa856e..cb1ca11 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c >> @@ -322,6 +322,7 @@ int pqm_create_queue(struct process_queue_manager *pqm, >> >> if (q) { >> pr_debug("PQM done creating queue\n"); >> + kfd_procfs_add_queue(q); >> print_queue_properties(&q->properties); >> } >> >> @@ -378,6 +379,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid) >> } >> >> if (pqn->q) { >> + kfd_procfs_del_queue(pqn->q); >> dqm = pqn->q->device->dqm; >> retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q); >> if (retval) { _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-31 19:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-31 13:45 [PATCH v2] drm/amdkfd: Add queue information to sysfs Amber Lin 2020-01-31 17:06 ` Felix Kuehling 2020-01-31 18:47 ` Lin, Amber 2020-01-31 19:22 ` Felix Kuehling
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.