Skip to content

Commit

Permalink
drm/i915: Update client name on context create
Browse files Browse the repository at this point in the history
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@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20210123153733.18139-3-chris@chris-wilson.co.uk
Link: https://patchwork.freedesktop.org/patch/msgid/20210124153136.19124-3-chris@chris-wilson.co.uk
  • Loading branch information
Tvrtko Ursulin authored and Chris Wilson committed Jan 24, 2021
1 parent ed89070 commit 12d52c7
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 17 deletions.
5 changes: 5 additions & 0 deletions drivers/gpu/drm/i915/gem/i915_gem_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down
93 changes: 78 additions & 15 deletions drivers/gpu/drm/i915/i915_drm_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -25,21 +28,31 @@ 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 sysfs_emit(buf,
READ_ONCE(client->closed) ? "<%s>\n" : "%s\n",
client->name);
rcu_read_lock();
ret = sysfs_emit(buf,
READ_ONCE(client->closed) ? "<%s>\n" : "%s\n",
i915_drm_client_name(client));
rcu_read_unlock();

return ret;
}

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);
int ret;

rcu_read_lock();
ret = sysfs_emit(buf,
READ_ONCE(client->closed) ? "<%u>\n" : "%u\n",
pid_nr(i915_drm_client_pid(client)));
rcu_read_unlock();

return sysfs_emit(buf,
READ_ONCE(client->closed) ? "<%u>\n" : "%u\n",
pid_nr(client->pid));
return ret;
}

static int __client_register_sysfs(struct i915_drm_client *client)
Expand Down Expand Up @@ -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 */
Expand All @@ -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)
Expand All @@ -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);

Expand Down Expand Up @@ -189,6 +233,25 @@ void i915_drm_client_close(struct i915_drm_client *client)
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;
}

void i915_drm_clients_fini(struct i915_drm_clients *clients)
{
while (!xa_empty(&clients->xarray)) {
Expand Down
34 changes: 32 additions & 2 deletions drivers/gpu/drm/i915/i915_drm_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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;
Expand Down Expand Up @@ -66,6 +75,27 @@ 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;
}

void i915_drm_clients_fini(struct i915_drm_clients *clients);

#endif /* !__I915_DRM_CLIENT_H__ */

0 comments on commit 12d52c7

Please sign in to comment.