On Tue, Sep 1, 2020 at 8:25 AM Tvrtko Ursulin
<tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
>
>
> 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.
Yes, against drm-tip. I rebased it, but I think there is something
wrong with it.
If you can share your version I can do some tests.
thanks
Lucas De Marchi
>
> 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
