Quoting Tvrtko Ursulin (2018-02-14 18:50:32)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Expose a list of clients with open file handles in sysfs.
>
> This will be a basis for a top-like utility showing per-client and per-
> engine GPU load.
>
> Currently we only expose each client's pid and name under opaque numbered
> directories in /sys/class/drm/card0/clients/.
>
> For instance:
>
> /sys/class/drm/card0/clients/3/name: Xorg
> /sys/class/drm/card0/clients/3/pid: 5664
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 13 +++++
> drivers/gpu/drm/i915/i915_gem.c | 112 +++++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/i915_sysfs.c | 8 +++
> 3 files changed, 126 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 70cf289855af..2133e67f5fbc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -345,6 +345,16 @@ struct drm_i915_file_private {
> */
> #define I915_MAX_CLIENT_CONTEXT_BANS 3
> atomic_t context_bans;
> +
struct {
> + unsigned int client_sysfs_id;
> + unsigned int client_pid;
> + char *client_name;
> + struct kobject *client_root;
} client;
> +
> + struct {
> + struct device_attribute pid;
> + struct device_attribute name;
> + } attr;
sysfs_attr?
> };
>
> /* Interface history:
> @@ -2365,6 +2375,9 @@ struct drm_i915_private {
>
> struct i915_pmu pmu;
>
> + struct kobject *clients_root;
> + atomic_t client_serial;
I'd start a new substruct { } clients;
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fc68b35854df..24607bce2efe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5613,6 +5613,89 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> +static ssize_t
> +show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> + struct drm_i915_file_private *file_priv =
> + container_of(attr, struct drm_i915_file_private, attr.name);
> +
> + return snprintf(buf, PAGE_SIZE, "%s", file_priv->client_name);
> +}
> +
> +static ssize_t
> +show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> + struct drm_i915_file_private *file_priv =
> + container_of(attr, struct drm_i915_file_private, attr.pid);
> +
> + return snprintf(buf, PAGE_SIZE, "%u", file_priv->client_pid);
> +}
> +
> +static int
> +i915_gem_add_client(struct drm_i915_private *i915,
> + struct drm_i915_file_private *file_priv,
> + struct task_struct *task,
> + unsigned int serial)
> +{
> + int ret = -ENOMEM;
> + struct device_attribute *attr;
> + char id[32];
> +
> + file_priv->client_name = kstrdup(task->comm, GFP_KERNEL);
> + if (!file_priv->client_name)
> + goto err_name;
> +
> + snprintf(id, sizeof(id), "%u", serial);
> + file_priv->client_root = kobject_create_and_add(id,
> + i915->clients_root);
Do we have to care about i915->clients_root being NULL here?
> + if (!file_priv->client_root)
> + goto err_client;
> +
> + attr = &file_priv->attr.name;
> + attr->attr.name = "name";
> + attr->attr.mode = 0444;
> + attr->show = show_client_name;
> +
> + ret = sysfs_create_file(file_priv->client_root,
> + (struct attribute *)attr);
> + if (ret)
> + goto err_attr_name;
> +
> + attr = &file_priv->attr.pid;
> + attr->attr.name = "pid";
> + attr->attr.mode = 0444;
> + attr->show = show_client_pid;
> +
> + ret = sysfs_create_file(file_priv->client_root,
> + (struct attribute *)attr);
> + if (ret)
> + goto err_attr_pid;
> +
> + file_priv->client_pid = pid_nr(get_task_pid(task, PIDTYPE_PID));
Are we before or after the "create_context get new pid"? Just wondering
aloud how that's going to tie in.
> +
> + return 0;
> +
> +err_attr_pid:
> + sysfs_remove_file(file_priv->client_root,
> + (struct attribute *)&file_priv->attr.name);
> +err_attr_name:
> + kobject_put(file_priv->client_root);
> +err_client:
> + kfree(file_priv->client_name);
> +err_name:
> + return ret;
> +}
> +
> +static void i915_gem_remove_client(struct drm_i915_file_private *file_priv)
> +{
> + sysfs_remove_file(file_priv->client_root,
> + (struct attribute *)&file_priv->attr.pid);
> + sysfs_remove_file(file_priv->client_root,
> + (struct attribute *)&file_priv->attr.name);
> + kobject_put(file_priv->client_root);
> + kfree(file_priv->client_name);
> +}
> +
> void i915_gem_release(struct drm_device *dev, struct drm_file *file)
> {
> struct drm_i915_file_private *file_priv = file->driver_priv;
> @@ -5626,32 +5709,47 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
> list_for_each_entry(request, &file_priv->mm.request_list, client_link)
> request->file_priv = NULL;
> spin_unlock(&file_priv->mm.lock);
> +
> + i915_gem_remove_client(file_priv);
> }
>
> int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
> {
> + int ret = -ENOMEM;
> struct drm_i915_file_private *file_priv;
> - int ret;
>
> DRM_DEBUG("\n");
>
> file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
> if (!file_priv)
> - return -ENOMEM;
> + goto err_alloc;
> +
> + file_priv->client_sysfs_id = atomic_inc_return(&i915->client_serial);
What's the use for client_sysfd_id? I don't see it used again.
> + ret = i915_gem_add_client(i915, file_priv, current,
> + file_priv->client_sysfs_id);
> + if (ret)
> + goto err_client;
>
> file->driver_priv = file_priv;
> + ret = i915_gem_context_open(i915, file);
> + if (ret)
> + goto err_context;
> +
> file_priv->dev_priv = i915;
> file_priv->file = file;
> + file_priv->bsd_engine = -1;
>
> spin_lock_init(&file_priv->mm.lock);
> INIT_LIST_HEAD(&file_priv->mm.request_list);
>
> - file_priv->bsd_engine = -1;
> -
> - ret = i915_gem_context_open(i915, file);
> - if (ret)
> - kfree(file_priv);
> + return 0;
>
> +err_context:
> + i915_gem_remove_client(file_priv);
> +err_client:
> + atomic_dec(&i915->client_serial);
> + kfree(file_priv);
> +err_alloc:
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index b33d2158c234..6cf032bc1573 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -575,6 +575,11 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
> struct device *kdev = dev_priv->drm.primary->kdev;
> int ret;
>
> + dev_priv->clients_root =
> + kobject_create_and_add("clients", &kdev->kobj);
> + if (!dev_priv->clients_root)
> + DRM_ERROR("Per-client sysfs setup failed\n");
> +
> #ifdef CONFIG_PM
> if (HAS_RC6(dev_priv)) {
> ret = sysfs_merge_group(&kdev->kobj,
> @@ -635,4 +640,7 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
> sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group);
> sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group);
> #endif
> +
> + if (dev_priv->clients_root)
> + kobject_put(dev_priv->clients_root);
> }
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
