| Message ID | 20200415101138.26126-2-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Per client engine busyness | expand |
Hi, Any update on this? It now conflicts in a few places so it needs a rebase. Lucas De Marchi On Wed, Apr 15, 2020 at 3:11 AM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com> > Reviewed-by: Chris Wilson <chris@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@linux.intel.com> wrote: >> >> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com> >> Reviewed-by: Chris Wilson <chris@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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@linux.intel.com> wrote: >>> >>> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com> >>> Reviewed-by: Chris Wilson <chris@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@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Sep 1, 2020 at 8:25 AM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> 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@linux.intel.com> wrote: > >>> > >>> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com> > >>> Reviewed-by: Chris Wilson <chris@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@lists.freedesktop.org > >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 04/09/2020 07:26, Lucas De Marchi wrote: > On Tue, Sep 1, 2020 at 8:25 AM Tvrtko Ursulin > <tvrtko.ursulin@linux.intel.com> 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. I've sent a series out just now. Tested it lightly (proper IGTs are still in progress) and it seems to work. Regards, Tvrtko
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); }