[PATCH 02/15] drm/i915: Update client name on context create
Chris Wilson
chris at chris-wilson.co.uk
Mon Jan 18 23:01:20 UTC 2021
From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Some clients have the DRM fd passed to them over a socket by the X server.
Grab the real client and pid when they create their first context and
update the exposed data for more useful enumeration.
To enable lockless access to client name and pid data from the following
patches, we also make these fields rcu protected. In this way asynchronous
code paths where both contexts which remain after the client exit, and
access to client name and pid as they are getting updated due context
creation running in parallel with name/pid queries.
v2:
* Do not leak the pid reference and borrow context idr_lock. (Chris)
v3:
* More avoiding leaks. (Chris)
v4:
* Move update completely to drm client. (Chris)
* Do not lose previous client data on failure to re-register and simplify
update to only touch what it needs.
v5:
* Reuse ext_data local. (Chris)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 ++
drivers/gpu/drm/i915/i915_drm_client.c | 93 +++++++++++++++++----
drivers/gpu/drm/i915/i915_drm_client.h | 34 +++++++-
3 files changed, 115 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 4d2f40cf237b..b8070be10d06 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -76,6 +76,7 @@
#include "gt/intel_gpu_commands.h"
#include "gt/intel_ring.h"
+#include "i915_drm_client.h"
#include "i915_gem_context.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -2315,6 +2316,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
return -EIO;
}
+ ret = i915_drm_client_update(ext_data.fpriv->client, current);
+ if (ret)
+ return ret;
+
ext_data.ctx = i915_gem_create_context(i915, args->flags);
if (IS_ERR(ext_data.ctx))
return PTR_ERR(ext_data.ctx);
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 597829f2c657..135d83a787be 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -7,7 +7,10 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include <drm/drm_print.h>
+
#include "i915_drm_client.h"
+#include "i915_drv.h"
#include "i915_gem.h"
#include "i915_utils.h"
@@ -25,10 +28,15 @@ show_client_name(struct device *kdev, struct device_attribute *attr, char *buf)
{
struct i915_drm_client *client =
container_of(attr, typeof(*client), attr.name);
+ int ret;
- return snprintf(buf, PAGE_SIZE,
- READ_ONCE(client->closed) ? "<%s>" : "%s",
- client->name);
+ rcu_read_lock();
+ ret = snprintf(buf, PAGE_SIZE,
+ READ_ONCE(client->closed) ? "<%s>" : "%s",
+ i915_drm_client_name(client));
+ rcu_read_unlock();
+
+ return ret;
}
static ssize_t
@@ -36,10 +44,15 @@ show_client_pid(struct device *kdev, struct device_attribute *attr, char *buf)
{
struct i915_drm_client *client =
container_of(attr, typeof(*client), attr.pid);
+ int ret;
- return snprintf(buf, PAGE_SIZE,
- READ_ONCE(client->closed) ? "<%u>" : "%u",
- pid_nr(client->pid));
+ rcu_read_lock();
+ ret = snprintf(buf, PAGE_SIZE,
+ READ_ONCE(client->closed) ? "<%u>" : "%u",
+ pid_nr(i915_drm_client_pid(client)));
+ rcu_read_unlock();
+
+ return ret;
}
static int __client_register_sysfs(struct i915_drm_client *client)
@@ -91,20 +104,46 @@ static void __client_unregister_sysfs(struct i915_drm_client *client)
kobject_put(fetch_and_zero(&client->root));
}
+static struct i915_drm_client_name *get_name(struct i915_drm_client *client,
+ struct task_struct *task)
+{
+ struct i915_drm_client_name *name;
+ int len = strlen(task->comm);
+
+ name = kmalloc(struct_size(name, name, len + 1), GFP_KERNEL);
+ if (!name)
+ return NULL;
+
+ init_rcu_head(&name->rcu);
+ name->client = client;
+ name->pid = get_task_pid(task, PIDTYPE_PID);
+ memcpy(name->name, task->comm, len + 1);
+
+ return name;
+}
+
+static void free_name(struct rcu_head *rcu)
+{
+ struct i915_drm_client_name *name =
+ container_of(rcu, typeof(*name), rcu);
+
+ put_pid(name->pid);
+ kfree(name);
+}
+
static int
__i915_drm_client_register(struct i915_drm_client *client,
struct task_struct *task)
{
struct i915_drm_clients *clients = client->clients;
- char *name;
+ struct i915_drm_client_name *name;
int ret;
- name = kstrdup(task->comm, GFP_KERNEL);
+ name = get_name(client, task);
if (!name)
return -ENOMEM;
- client->pid = get_task_pid(task, PIDTYPE_PID);
- client->name = name;
+ RCU_INIT_POINTER(client->name, name);
if (!clients->root)
return 0; /* intel_fbdev_init registers a client before sysfs */
@@ -116,18 +155,22 @@ __i915_drm_client_register(struct i915_drm_client *client,
return 0;
err_sysfs:
- put_pid(client->pid);
- kfree(client->name);
-
+ RCU_INIT_POINTER(client->name, NULL);
+ call_rcu(&name->rcu, free_name);
return ret;
}
static void __i915_drm_client_unregister(struct i915_drm_client *client)
{
+ struct i915_drm_client_name *name;
+
__client_unregister_sysfs(client);
- put_pid(fetch_and_zero(&client->pid));
- kfree(fetch_and_zero(&client->name));
+ mutex_lock(&client->update_lock);
+ name = rcu_replace_pointer(client->name, NULL, true);
+ mutex_unlock(&client->update_lock);
+
+ call_rcu(&name->rcu, free_name);
}
static void __rcu_i915_drm_client_free(struct work_struct *wrk)
@@ -152,6 +195,7 @@ i915_drm_client_add(struct i915_drm_clients *clients, struct task_struct *task)
return ERR_PTR(-ENOMEM);
kref_init(&client->kref);
+ mutex_init(&client->update_lock);
client->clients = clients;
INIT_RCU_WORK(&client->rcu, __rcu_i915_drm_client_free);
@@ -188,3 +232,22 @@ void i915_drm_client_close(struct i915_drm_client *client)
WRITE_ONCE(client->closed, true);
i915_drm_client_put(client);
}
+
+int
+i915_drm_client_update(struct i915_drm_client *client,
+ struct task_struct *task)
+{
+ struct i915_drm_client_name *name;
+
+ name = get_name(client, task);
+ if (!name)
+ return -ENOMEM;
+
+ mutex_lock(&client->update_lock);
+ if (name->pid != rcu_dereference_protected(client->name, true)->pid)
+ name = rcu_replace_pointer(client->name, name, true);
+ mutex_unlock(&client->update_lock);
+
+ call_rcu(&name->rcu, free_name);
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index e222864748c7..45f754af3cfb 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -9,6 +9,7 @@
#include <linux/device.h>
#include <linux/kobject.h>
#include <linux/kref.h>
+#include <linux/mutex.h>
#include <linux/pid.h>
#include <linux/rcupdate.h>
#include <linux/sched.h>
@@ -25,14 +26,22 @@ struct i915_drm_clients {
struct kobject *root;
};
+struct i915_drm_client_name {
+ struct rcu_head rcu;
+ struct i915_drm_client *client;
+ struct pid *pid;
+ char name[];
+};
+
struct i915_drm_client {
struct kref kref;
struct rcu_work rcu;
+ struct mutex update_lock; /* Serializes name and pid updates. */
+
unsigned int id;
- struct pid *pid;
- char *name;
+ struct i915_drm_client_name __rcu *name;
bool closed;
struct i915_drm_clients *clients;
@@ -66,4 +75,25 @@ 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);
+int i915_drm_client_update(struct i915_drm_client *client,
+ struct task_struct *task);
+
+static inline const struct i915_drm_client_name *
+__i915_drm_client_name(const struct i915_drm_client *client)
+{
+ return rcu_dereference(client->name);
+}
+
+static inline const char *
+i915_drm_client_name(const struct i915_drm_client *client)
+{
+ return __i915_drm_client_name(client)->name;
+}
+
+static inline struct pid *
+i915_drm_client_pid(const struct i915_drm_client *client)
+{
+ return __i915_drm_client_name(client)->pid;
+}
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.20.1
More information about the Intel-gfx-trybot
mailing list