Re: [PATCH 1/9] drm/i915: Expose list of clients in sysfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 01/09/2020 16:09, Tvrtko Ursulin wrote:

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?

Clearly you were after an update against drm-tip.. :) Problem here was no userspace but I can try to respin it.

Regards,

Tvrtko


Regards,

Tvrtko

Lucas De Marchi

On Wed, Apr 15, 2020 at 3:11 AM Tvrtko Ursulin
<tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:

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

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@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux