[Intel-gfx] [PATCH 1/9] drm/i915: Expose list of clients in sysfs
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Tue Sep 1 15:09:37 UTC 2020
Hi,
On 26/08/2020 02:11, Lucas De Marchi wrote:
> Hi,
>
> Any update on this? It now conflicts in a few places so it needs a rebase.
I don't see any previous email on the topic - what kind of update, where
and how, are you looking for? Rebase against drm-tip so you pull it in?
Rebase against some internal in progress branch?
Regards,
Tvrtko
> Lucas De Marchi
>
> On Wed, Apr 15, 2020 at 3:11 AM Tvrtko Ursulin
> <tvrtko.ursulin at linux.intel.com> wrote:
>>
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> 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
>>
>> v2:
>> Chris Wilson:
>> * Enclose new members into dedicated structs.
>> * Protect against failed sysfs registration.
>>
>> v3:
>> * sysfs_attr_init.
>>
>> v4:
>> * Fix for internal clients.
>>
>> v5:
>> * Use cyclic ida for client id. (Chris)
>> * Do not leak pid reference. (Chris)
>> * Tidy code with some locals.
>>
>> v6:
>> * Use xa_alloc_cyclic to simplify locking. (Chris)
>> * No need to unregister individial sysfs files. (Chris)
>> * Rebase on top of fpriv kref.
>> * Track client closed status and reflect in sysfs.
>>
>> v7:
>> * Make drm_client more standalone concept.
>>
>> v8:
>> * Simplify sysfs show. (Chris)
>> * Always track name and pid.
>>
>> v9:
>> * Fix cyclic id assignment.
>>
>> v10:
>> * No need for a mutex around xa_alloc_cyclic.
>> * Refactor sysfs into own function.
>> * Unregister sysfs before freeing pid and name.
>> * Move clients setup into own function.
>>
>> v11:
>> * Call clients init directly from driver init. (Chris)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>> ---
>> drivers/gpu/drm/i915/Makefile | 3 +-
>> drivers/gpu/drm/i915/i915_drm_client.c | 179 +++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_drm_client.h | 64 +++++++++
>> drivers/gpu/drm/i915/i915_drv.c | 3 +
>> drivers/gpu/drm/i915/i915_drv.h | 5 +
>> drivers/gpu/drm/i915/i915_gem.c | 25 +++-
>> drivers/gpu/drm/i915/i915_sysfs.c | 8 ++
>> 7 files changed, 283 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/i915_drm_client.c
>> create mode 100644 drivers/gpu/drm/i915/i915_drm_client.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 44c506b7e117..b30f3d51c66a 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -33,7 +33,8 @@ subdir-ccflags-y += -I$(srctree)/$(src)
>> # Please keep these build lists sorted!
>>
>> # core driver code
>> -i915-y += i915_drv.o \
>> +i915-y += i915_drm_client.o \
>> + i915_drv.o \
>> i915_irq.o \
>> i915_getparam.o \
>> i915_params.o \
>> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
>> new file mode 100644
>> index 000000000000..2067fbcdb795
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
>> @@ -0,0 +1,179 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2020 Intel Corporation
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +
>> +#include "i915_drm_client.h"
>> +#include "i915_gem.h"
>> +#include "i915_utils.h"
>> +
>> +void i915_drm_clients_init(struct i915_drm_clients *clients)
>> +{
>> + clients->next_id = 0;
>> + xa_init_flags(&clients->xarray, XA_FLAGS_ALLOC);
>> +}
>> +
>> +static ssize_t
>> +show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
>> +{
>> + struct i915_drm_client *client =
>> + container_of(attr, typeof(*client), attr.name);
>> +
>> + return snprintf(buf, PAGE_SIZE,
>> + READ_ONCE(client->closed) ? "<%s>" : "%s",
>> + client->name);
>> +}
>> +
>> +static ssize_t
>> +show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
>> +{
>> + struct i915_drm_client *client =
>> + container_of(attr, typeof(*client), attr.pid);
>> +
>> + return snprintf(buf, PAGE_SIZE,
>> + READ_ONCE(client->closed) ? "<%u>" : "%u",
>> + pid_nr(client->pid));
>> +}
>> +
>> +static int
>> +__client_register_sysfs(struct i915_drm_client *client)
>> +{
>> + const struct {
>> + const char *name;
>> + struct device_attribute *attr;
>> + ssize_t (*show)(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf);
>> + } files[] = {
>> + { "name", &client->attr.name, show_client_name },
>> + { "pid", &client->attr.pid, show_client_pid },
>> + };
>> + unsigned int i;
>> + char buf[16];
>> + int ret;
>> +
>> + ret = scnprintf(buf, sizeof(buf), "%u", client->id);
>> + if (ret == sizeof(buf))
>> + return -EINVAL;
>> +
>> + client->root = kobject_create_and_add(buf, client->clients->root);
>> + if (!client->root)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < ARRAY_SIZE(files); i++) {
>> + struct device_attribute *attr = files[i].attr;
>> +
>> + sysfs_attr_init(&attr->attr);
>> +
>> + attr->attr.name = files[i].name;
>> + attr->attr.mode = 0444;
>> + attr->show = files[i].show;
>> +
>> + ret = sysfs_create_file(client->root, (struct attribute *)attr);
>> + if (ret)
>> + break;
>> + }
>> +
>> + if (ret)
>> + kobject_put(client->root);
>> +
>> + return ret;
>> +}
>> +
>> +static void __client_unregister_sysfs(struct i915_drm_client *client)
>> +{
>> + kobject_put(fetch_and_zero(&client->root));
>> +}
>> +
>> +static int
>> +__i915_drm_client_register(struct i915_drm_client *client,
>> + struct task_struct *task)
>> +{
>> + struct i915_drm_clients *clients = client->clients;
>> + char *name;
>> + int ret;
>> +
>> + name = kstrdup(task->comm, GFP_KERNEL);
>> + if (!name)
>> + return -ENOMEM;
>> +
>> + client->pid = get_task_pid(task, PIDTYPE_PID);
>> + client->name = name;
>> +
>> + if (!clients->root)
>> + return 0; /* intel_fbdev_init registers a client before sysfs */
>> +
>> + ret = __client_register_sysfs(client);
>> + if (ret)
>> + goto err_sysfs;
>> +
>> + return 0;
>> +
>> +err_sysfs:
>> + put_pid(client->pid);
>> + kfree(client->name);
>> +
>> + return ret;
>> +}
>> +
>> +static void
>> +__i915_drm_client_unregister(struct i915_drm_client *client)
>> +{
>> + __client_unregister_sysfs(client);
>> +
>> + put_pid(fetch_and_zero(&client->pid));
>> + kfree(fetch_and_zero(&client->name));
>> +}
>> +
>> +struct i915_drm_client *
>> +i915_drm_client_add(struct i915_drm_clients *clients, struct task_struct *task)
>> +{
>> + struct i915_drm_client *client;
>> + int ret;
>> +
>> + client = kzalloc(sizeof(*client), GFP_KERNEL);
>> + if (!client)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + kref_init(&client->kref);
>> + client->clients = clients;
>> +
>> + ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
>> + xa_limit_32b, &clients->next_id, GFP_KERNEL);
>> + if (ret)
>> + goto err_id;
>> +
>> + ret = __i915_drm_client_register(client, task);
>> + if (ret)
>> + goto err_register;
>> +
>> + return client;
>> +
>> +err_register:
>> + xa_erase(&clients->xarray, client->id);
>> +err_id:
>> + kfree(client);
>> +
>> + return ERR_PTR(ret);
>> +}
>> +
>> +void __i915_drm_client_free(struct kref *kref)
>> +{
>> + struct i915_drm_client *client =
>> + container_of(kref, typeof(*client), kref);
>> +
>> + __i915_drm_client_unregister(client);
>> + xa_erase(&client->clients->xarray, client->id);
>> + kfree_rcu(client, rcu);
>> +}
>> +
>> +void i915_drm_client_close(struct i915_drm_client *client)
>> +{
>> + GEM_BUG_ON(READ_ONCE(client->closed));
>> + WRITE_ONCE(client->closed, true);
>> + i915_drm_client_put(client);
>> +}
>> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
>> new file mode 100644
>> index 000000000000..af6998c74d4c
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/i915_drm_client.h
>> @@ -0,0 +1,64 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2020 Intel Corporation
>> + */
>> +
>> +#ifndef __I915_DRM_CLIENT_H__
>> +#define __I915_DRM_CLIENT_H__
>> +
>> +#include <linux/device.h>
>> +#include <linux/kobject.h>
>> +#include <linux/kref.h>
>> +#include <linux/pid.h>
>> +#include <linux/rcupdate.h>
>> +#include <linux/sched.h>
>> +#include <linux/xarray.h>
>> +
>> +struct i915_drm_clients {
>> + struct xarray xarray;
>> + u32 next_id;
>> +
>> + struct kobject *root;
>> +};
>> +
>> +struct i915_drm_client {
>> + struct kref kref;
>> +
>> + struct rcu_head rcu;
>> +
>> + unsigned int id;
>> + struct pid *pid;
>> + char *name;
>> + bool closed;
>> +
>> + struct i915_drm_clients *clients;
>> +
>> + struct kobject *root;
>> + struct {
>> + struct device_attribute pid;
>> + struct device_attribute name;
>> + } attr;
>> +};
>> +
>> +void i915_drm_clients_init(struct i915_drm_clients *clients);
>> +
>> +static inline struct i915_drm_client *
>> +i915_drm_client_get(struct i915_drm_client *client)
>> +{
>> + kref_get(&client->kref);
>> + return client;
>> +}
>> +
>> +void __i915_drm_client_free(struct kref *kref);
>> +
>> +static inline void i915_drm_client_put(struct i915_drm_client *client)
>> +{
>> + kref_put(&client->kref, __i915_drm_client_free);
>> +}
>> +
>> +void i915_drm_client_close(struct i915_drm_client *client);
>> +
>> +struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients,
>> + struct task_struct *task);
>> +
>> +#endif /* !__I915_DRM_CLIENT_H__ */
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 641f5e03b661..dac84b17d23d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -70,6 +70,7 @@
>> #include "gt/intel_rc6.h"
>>
>> #include "i915_debugfs.h"
>> +#include "i915_drm_client.h"
>> #include "i915_drv.h"
>> #include "i915_ioc32.h"
>> #include "i915_irq.h"
>> @@ -456,6 +457,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>>
>> i915_gem_init_early(dev_priv);
>>
>> + i915_drm_clients_init(&dev_priv->clients);
>> +
>> /* This must be called before any calls to HAS_PCH_* */
>> intel_detect_pch(dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index e9ee4daa9320..f9f0c3ba6e4a 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -91,6 +91,7 @@
>> #include "intel_wakeref.h"
>> #include "intel_wopcm.h"
>>
>> +#include "i915_drm_client.h"
>> #include "i915_gem.h"
>> #include "i915_gem_gtt.h"
>> #include "i915_gpu_error.h"
>> @@ -226,6 +227,8 @@ struct drm_i915_file_private {
>> /** ban_score: Accumulated score of all ctx bans and fast hangs. */
>> atomic_t ban_score;
>> unsigned long hang_timestamp;
>> +
>> + struct i915_drm_client *client;
>> };
>>
>> /* Interface history:
>> @@ -1201,6 +1204,8 @@ struct drm_i915_private {
>>
>> struct i915_pmu pmu;
>>
>> + struct i915_drm_clients clients;
>> +
>> struct i915_hdcp_comp_master *hdcp_master;
>> bool hdcp_comp_added;
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 0cbcb9f54e7d..5a0b5fae8b92 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1234,6 +1234,8 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
>> GEM_BUG_ON(!llist_empty(&dev_priv->mm.free_list));
>> GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
>> drm_WARN_ON(&dev_priv->drm, dev_priv->mm.shrink_count);
>> + drm_WARN_ON(&dev_priv->drm, !xa_empty(&dev_priv->clients.xarray));
>> + xa_destroy(&dev_priv->clients.xarray);
>> }
>>
>> int i915_gem_freeze(struct drm_i915_private *dev_priv)
>> @@ -1288,6 +1290,8 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
>> struct drm_i915_file_private *file_priv = file->driver_priv;
>> struct i915_request *request;
>>
>> + i915_drm_client_close(file_priv->client);
>> +
>> /* Clean up our request list when the client is going away, so that
>> * later retire_requests won't dereference our soon-to-be-gone
>> * file_priv.
>> @@ -1301,17 +1305,25 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
>> int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
>> {
>> struct drm_i915_file_private *file_priv;
>> - int ret;
>> + struct i915_drm_client *client;
>> + int ret = -ENOMEM;
>>
>> DRM_DEBUG("\n");
>>
>> file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
>> if (!file_priv)
>> - return -ENOMEM;
>> + goto err_alloc;
>> +
>> + client = i915_drm_client_add(&i915->clients, current);
>> + if (IS_ERR(client)) {
>> + ret = PTR_ERR(client);
>> + goto err_client;
>> + }
>>
>> file->driver_priv = file_priv;
>> file_priv->dev_priv = i915;
>> file_priv->file = file;
>> + file_priv->client = client;
>>
>> spin_lock_init(&file_priv->mm.lock);
>> INIT_LIST_HEAD(&file_priv->mm.request_list);
>> @@ -1321,8 +1333,15 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
>>
>> ret = i915_gem_context_open(i915, file);
>> if (ret)
>> - kfree(file_priv);
>> + goto err_context;
>> +
>> + return 0;
>>
>> +err_context:
>> + i915_drm_client_close(client);
>> +err_client:
>> + 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 45d32ef42787..b7d4a6d2dd5c 100644
>> --- a/drivers/gpu/drm/i915/i915_sysfs.c
>> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
>> @@ -560,6 +560,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,
>> @@ -627,4 +632,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.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list