* Re: [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create
@ 2020-04-16 6:12 kbuild test robot
0 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2020-04-16 6:12 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 11776 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20200415101138.26126-3-tvrtko.ursulin@linux.intel.com>
References: <20200415101138.26126-3-tvrtko.ursulin@linux.intel.com>
TO: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
TO: Intel-gfx(a)lists.freedesktop.org
CC: Chris Wilson <chris@chris-wilson.co.uk>
Hi Tvrtko,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v5.7-rc1 next-20200415]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Tvrtko-Ursulin/Per-client-engine-busyness/20200416-032109
base: git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce:
# apt-get install sparse
# sparse version:
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
:::::: branch date: 11 hours ago
:::::: commit date: 11 hours ago
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/gpu/drm/i915/i915_drm_client.c:130:23: sparse: warning: incorrect type in argument 1 (different address spaces)
>> drivers/gpu/drm/i915/i915_drm_client.c:130:23: sparse: expected struct pid *pid
>> drivers/gpu/drm/i915/i915_drm_client.c:130:23: sparse: got struct pid [noderef] <asn:4> *pid
drivers/gpu/drm/i915/i915_drm_client.c:131:21: sparse: warning: incorrect type in argument 1 (different address spaces)
>> drivers/gpu/drm/i915/i915_drm_client.c:131:21: sparse: expected void const *
>> drivers/gpu/drm/i915/i915_drm_client.c:131:21: sparse: got char [noderef] <asn:4> *name
drivers/gpu/drm/i915/i915_drm_client.c:232:17: sparse: error: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/i915_drm_client.c:232:17: sparse: struct pid *
>> drivers/gpu/drm/i915/i915_drm_client.c:232:17: sparse: struct pid [noderef] <asn:4> *
# https://github.com/0day-ci/linux/commit/68e51ee0010d0e158add87c4362668f009fc8130
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 68e51ee0010d0e158add87c4362668f009fc8130
vim +130 drivers/gpu/drm/i915/i915_drm_client.c
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 104
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 105 static int
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 106 __i915_drm_client_register(struct i915_drm_client *client,
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 107 struct task_struct *task)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 108 {
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 109 struct i915_drm_clients *clients = client->clients;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 110 char *name;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 111 int ret;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 112
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 113 name = kstrdup(task->comm, GFP_KERNEL);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 114 if (!name)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 115 return -ENOMEM;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 116
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 117 rcu_assign_pointer(client->pid, get_task_pid(task, PIDTYPE_PID));
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 118 rcu_assign_pointer(client->name, name);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 119
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 120 if (!clients->root)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 121 return 0; /* intel_fbdev_init registers a client before sysfs */
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 122
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 123 ret = __client_register_sysfs(client);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 124 if (ret)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 125 goto err_sysfs;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 126
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 127 return 0;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 128
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 129 err_sysfs:
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 @130 put_pid(client->pid);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 @131 kfree(client->name);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 132
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 133 return ret;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 134 }
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 135
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 136 static void
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 137 __i915_drm_client_unregister(struct i915_drm_client *client)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 138 {
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 139 __client_unregister_sysfs(client);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 140
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 141 put_pid(rcu_replace_pointer(client->pid, NULL, true));
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 142 kfree(rcu_replace_pointer(client->name, NULL, true));
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 143 }
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 144
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 145 struct i915_drm_client *
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 146 i915_drm_client_add(struct i915_drm_clients *clients, struct task_struct *task)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 147 {
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 148 struct i915_drm_client *client;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 149 int ret;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 150
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 151 client = kzalloc(sizeof(*client), GFP_KERNEL);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 152 if (!client)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 153 return ERR_PTR(-ENOMEM);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 154
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 155 kref_init(&client->kref);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 156 mutex_init(&client->update_lock);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 157 client->clients = clients;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 158
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 159 ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 160 xa_limit_32b, &clients->next_id, GFP_KERNEL);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 161 if (ret)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 162 goto err_id;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 163
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 164 ret = __i915_drm_client_register(client, task);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 165 if (ret)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 166 goto err_register;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 167
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 168 return client;
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 169
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 170 err_register:
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 171 xa_erase(&clients->xarray, client->id);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 172 err_id:
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 173 kfree(client);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 174
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 175 return ERR_PTR(ret);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 176 }
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 177
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 178 void __i915_drm_client_free(struct kref *kref)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 179 {
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 180 struct i915_drm_client *client =
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 181 container_of(kref, typeof(*client), kref);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 182
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 183 __i915_drm_client_unregister(client);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 184 xa_erase(&client->clients->xarray, client->id);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 185 kfree_rcu(client, rcu);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 186 }
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 187
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 188 void i915_drm_client_close(struct i915_drm_client *client)
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 189 {
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 190 GEM_BUG_ON(READ_ONCE(client->closed));
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 191 WRITE_ONCE(client->closed, true);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 192 i915_drm_client_put(client);
d76ac5524c4129 Tvrtko Ursulin 2020-04-15 193 }
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 194
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 195 struct client_update_free {
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 196 struct rcu_head rcu;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 197 struct pid *pid;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 198 char *name;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 199 };
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 200
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 201 static void __client_update_free(struct rcu_head *rcu)
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 202 {
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 203 struct client_update_free *old = container_of(rcu, typeof(*old), rcu);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 204
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 205 put_pid(old->pid);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 206 kfree(old->name);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 207 kfree(old);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 208 }
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 209
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 210 int
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 211 i915_drm_client_update(struct i915_drm_client *client,
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 212 struct task_struct *task)
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 213 {
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 214 struct drm_i915_private *i915 =
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 215 container_of(client->clients, typeof(*i915), clients);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 216 struct client_update_free *old;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 217 struct pid *pid;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 218 char *name;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 219 int ret;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 220
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 221 old = kmalloc(sizeof(*old), GFP_KERNEL);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 222 if (!old)
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 223 return -ENOMEM;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 224
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 225 ret = mutex_lock_interruptible(&client->update_lock);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 226 if (ret)
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 227 goto out_free;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 228
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 229 pid = get_task_pid(task, PIDTYPE_PID);
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 230 if (!pid)
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 231 goto out_pid;
68e51ee0010d0e Tvrtko Ursulin 2020-04-15 @232 if (pid == client->pid)
:::::: The code at line 130 was first introduced by commit
:::::: d76ac5524c4129b1718d52109a90ec752b1ba2fe drm/i915: Expose list of clients in sysfs
:::::: TO: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
:::::: CC: 0day robot <lkp@intel.com>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create
2020-09-14 13:12 [Intel-gfx] [PATCH 0/9] Per client engine busyness Tvrtko Ursulin
@ 2020-09-14 13:12 ` Tvrtko Ursulin
0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2020-09-14 13:12 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +
drivers/gpu/drm/i915/i915_drm_client.c | 110 +++++++++++++++++---
drivers/gpu/drm/i915/i915_drm_client.h | 10 +-
3 files changed, 111 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cf5ecbde9e06..5919cc5f8348 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -74,6 +74,7 @@
#include "gt/intel_engine_user.h"
#include "gt/intel_ring.h"
+#include "i915_drm_client.h"
#include "i915_gem_context.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -2388,6 +2389,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 137a5744f9c6..617509c2e887 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -7,6 +7,9 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include <drm/drm_print.h>
+
+#include "i915_drv.h"
#include "i915_drm_client.h"
#include "i915_gem.h"
#include "i915_utils.h"
@@ -22,10 +25,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;
+
+ rcu_read_lock();
+ ret = snprintf(buf, PAGE_SIZE,
+ READ_ONCE(client->closed) ? "<%s>" : "%s",
+ rcu_dereference(client->name));
+ rcu_read_unlock();
- return snprintf(buf, PAGE_SIZE,
- READ_ONCE(client->closed) ? "<%s>" : "%s",
- client->name);
+ return ret;
}
static ssize_t
@@ -33,10 +41,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(rcu_dereference(client->pid)));
+ rcu_read_unlock();
+
+ return ret;
}
static int
@@ -101,8 +114,8 @@ __i915_drm_client_register(struct i915_drm_client *client,
if (!name)
return -ENOMEM;
- client->pid = get_task_pid(task, PIDTYPE_PID);
- client->name = name;
+ rcu_assign_pointer(client->pid, get_task_pid(task, PIDTYPE_PID));
+ rcu_assign_pointer(client->name, name);
if (!clients->root)
return 0; /* intel_fbdev_init registers a client before sysfs */
@@ -114,8 +127,8 @@ __i915_drm_client_register(struct i915_drm_client *client,
return 0;
err_sysfs:
- put_pid(client->pid);
- kfree(client->name);
+ put_pid(rcu_replace_pointer(client->pid, NULL, true));
+ kfree(rcu_replace_pointer(client->name, NULL, true));
return ret;
}
@@ -125,8 +138,8 @@ __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));
+ put_pid(rcu_replace_pointer(client->pid, NULL, true));
+ kfree(rcu_replace_pointer(client->name, NULL, true));
}
struct i915_drm_client *
@@ -140,6 +153,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;
ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
@@ -177,3 +191,75 @@ void i915_drm_client_close(struct i915_drm_client *client)
WRITE_ONCE(client->closed, true);
i915_drm_client_put(client);
}
+
+struct client_update_free {
+ struct rcu_head rcu;
+ struct pid *pid;
+ char *name;
+};
+
+static void __client_update_free(struct rcu_head *rcu)
+{
+ struct client_update_free *old = container_of(rcu, typeof(*old), rcu);
+
+ put_pid(old->pid);
+ kfree(old->name);
+ kfree(old);
+}
+
+int
+i915_drm_client_update(struct i915_drm_client *client,
+ struct task_struct *task)
+{
+ struct drm_i915_private *i915 =
+ container_of(client->clients, typeof(*i915), clients);
+ struct client_update_free *old;
+ struct pid *pid;
+ char *name;
+ int ret;
+
+ old = kmalloc(sizeof(*old), GFP_KERNEL);
+ if (!old)
+ return -ENOMEM;
+
+ ret = mutex_lock_interruptible(&client->update_lock);
+ if (ret)
+ goto out_free;
+
+ pid = get_task_pid(task, PIDTYPE_PID);
+ if (!pid)
+ goto out_pid;
+ if (pid == rcu_access_pointer(client->pid))
+ goto out_name;
+
+ name = kstrdup(task->comm, GFP_KERNEL);
+ if (!name) {
+ drm_notice(&i915->drm,
+ "Failed to update client id=%u,name=%s,pid=%u! (%d)\n",
+ client->id,
+ client->name,
+ pid_nr(rcu_access_pointer(client->pid)),
+ ret);
+ goto out_name;
+ }
+
+ init_rcu_head(&old->rcu);
+
+ old->pid = rcu_replace_pointer(client->pid, pid, true);
+ old->name = rcu_replace_pointer(client->name, name, true);
+
+ mutex_unlock(&client->update_lock);
+
+ call_rcu(&old->rcu, __client_update_free);
+
+ return 0;
+
+out_name:
+ put_pid(pid);
+out_pid:
+ mutex_unlock(&client->update_lock);
+out_free:
+ kfree(old);
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index af6998c74d4c..11b48383881d 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>
@@ -26,9 +27,11 @@ struct i915_drm_client {
struct rcu_head rcu;
+ struct mutex update_lock; /* Serializes name and pid updates. */
+
unsigned int id;
- struct pid *pid;
- char *name;
+ struct pid __rcu *pid;
+ char __rcu *name;
bool closed;
struct i915_drm_clients *clients;
@@ -61,4 +64,7 @@ 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);
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create
2020-09-04 12:59 [Intel-gfx] [PATCH 0/9] Per client engine busyness Tvrtko Ursulin
@ 2020-09-04 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2020-09-04 12:59 UTC (permalink / raw)
To: Intel-gfx; +Cc: Chris Wilson
From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +
drivers/gpu/drm/i915/i915_drm_client.c | 107 +++++++++++++++++---
drivers/gpu/drm/i915/i915_drm_client.h | 10 +-
3 files changed, 108 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cf5ecbde9e06..5919cc5f8348 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -74,6 +74,7 @@
#include "gt/intel_engine_user.h"
#include "gt/intel_ring.h"
+#include "i915_drm_client.h"
#include "i915_gem_context.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -2388,6 +2389,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 33f695c4596c..b0048af68912 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -8,6 +8,9 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include <drm/drm_print.h>
+
+#include "i915_drv.h"
#include "i915_drm_client.h"
#include "i915_gem.h"
#include "i915_utils.h"
@@ -23,10 +26,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;
+
+ rcu_read_lock();
+ ret = snprintf(buf, PAGE_SIZE,
+ READ_ONCE(client->closed) ? "<%s>" : "%s",
+ rcu_dereference(client->name));
+ rcu_read_unlock();
- return snprintf(buf, PAGE_SIZE,
- READ_ONCE(client->closed) ? "<%s>" : "%s",
- client->name);
+ return ret;
}
static ssize_t
@@ -34,10 +42,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(rcu_dereference(client->pid)));
+ rcu_read_unlock();
+
+ return ret;
}
static int
@@ -102,8 +115,8 @@ __i915_drm_client_register(struct i915_drm_client *client,
if (!name)
return -ENOMEM;
- client->pid = get_task_pid(task, PIDTYPE_PID);
- client->name = name;
+ rcu_assign_pointer(client->pid, get_task_pid(task, PIDTYPE_PID));
+ rcu_assign_pointer(client->name, name);
if (!clients->root)
return 0; /* intel_fbdev_init registers a client before sysfs */
@@ -115,8 +128,8 @@ __i915_drm_client_register(struct i915_drm_client *client,
return 0;
err_sysfs:
- put_pid(client->pid);
- kfree(client->name);
+ put_pid(rcu_replace_pointer(client->pid, NULL, true));
+ kfree(rcu_replace_pointer(client->name, NULL, true));
return ret;
}
@@ -126,8 +139,8 @@ __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));
+ put_pid(rcu_replace_pointer(client->pid, NULL, true));
+ kfree(rcu_replace_pointer(client->name, NULL, true));
}
struct i915_drm_client *
@@ -141,6 +154,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;
ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
@@ -178,3 +192,72 @@ void i915_drm_client_close(struct i915_drm_client *client)
WRITE_ONCE(client->closed, true);
i915_drm_client_put(client);
}
+
+struct client_update_free {
+ struct rcu_head rcu;
+ struct pid *pid;
+ char *name;
+};
+
+static void __client_update_free(struct rcu_head *rcu)
+{
+ struct client_update_free *old = container_of(rcu, typeof(*old), rcu);
+
+ put_pid(old->pid);
+ kfree(old->name);
+ kfree(old);
+}
+
+int
+i915_drm_client_update(struct i915_drm_client *client,
+ struct task_struct *task)
+{
+ struct drm_i915_private *i915 =
+ container_of(client->clients, typeof(*i915), clients);
+ struct client_update_free *old;
+ struct pid *pid;
+ char *name;
+ int ret;
+
+ old = kmalloc(sizeof(*old), GFP_KERNEL);
+ if (!old)
+ return -ENOMEM;
+
+ ret = mutex_lock_interruptible(&client->update_lock);
+ if (ret)
+ goto out_free;
+
+ pid = get_task_pid(task, PIDTYPE_PID);
+ if (!pid)
+ goto out_pid;
+ if (pid == rcu_access_pointer(client->pid))
+ goto out_name;
+
+ name = kstrdup(task->comm, GFP_KERNEL);
+ if (!name) {
+ drm_notice(&i915->drm,
+ "Failed to update client id=%u,name=%s,pid=%u! (%d)\n",
+ client->id, client->name, pid_nr(client->pid), ret);
+ goto out_name;
+ }
+
+ init_rcu_head(&old->rcu);
+
+ old->pid = rcu_replace_pointer(client->pid, pid, true);
+ old->name = rcu_replace_pointer(client->name, name, true);
+
+ mutex_unlock(&client->update_lock);
+
+ call_rcu(&old->rcu, __client_update_free);
+
+ return 0;
+
+out_name:
+ put_pid(pid);
+out_pid:
+ mutex_unlock(&client->update_lock);
+out_free:
+ kfree(old);
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 5009a2ae4b65..9f39b7407a5d 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -10,6 +10,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>
@@ -27,9 +28,11 @@ struct i915_drm_client {
struct rcu_head rcu;
+ struct mutex update_lock; /* Serializes name and pid updates. */
+
unsigned int id;
- struct pid *pid;
- char *name;
+ struct pid __rcu *pid;
+ char __rcu *name;
bool closed;
struct i915_drm_clients *clients;
@@ -62,4 +65,7 @@ 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);
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create
2020-04-15 10:11 [Intel-gfx] [PATCH 0/9] Per client engine busyness Tvrtko Ursulin
@ 2020-04-15 10:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2020-04-15 10:11 UTC (permalink / raw)
To: Intel-gfx; +Cc: Chris Wilson
From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +
drivers/gpu/drm/i915/i915_drm_client.c | 103 ++++++++++++++++++--
drivers/gpu/drm/i915/i915_drm_client.h | 10 +-
3 files changed, 106 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 11d9135cf21a..0a1d1db98d64 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -74,6 +74,7 @@
#include "gt/intel_engine_user.h"
#include "gt/intel_ring.h"
+#include "i915_drm_client.h"
#include "i915_gem_context.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -2356,6 +2357,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 2067fbcdb795..342a11554573 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -7,6 +7,9 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include <drm/drm_print.h>
+
+#include "i915_drv.h"
#include "i915_drm_client.h"
#include "i915_gem.h"
#include "i915_utils.h"
@@ -22,10 +25,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;
+
+ rcu_read_lock();
+ ret = snprintf(buf, PAGE_SIZE,
+ READ_ONCE(client->closed) ? "<%s>" : "%s",
+ rcu_dereference(client->name));
+ rcu_read_unlock();
- return snprintf(buf, PAGE_SIZE,
- READ_ONCE(client->closed) ? "<%s>" : "%s",
- client->name);
+ return ret;
}
static ssize_t
@@ -33,10 +41,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(rcu_dereference(client->pid)));
+ rcu_read_unlock();
+
+ return ret;
}
static int
@@ -101,8 +114,8 @@ __i915_drm_client_register(struct i915_drm_client *client,
if (!name)
return -ENOMEM;
- client->pid = get_task_pid(task, PIDTYPE_PID);
- client->name = name;
+ rcu_assign_pointer(client->pid, get_task_pid(task, PIDTYPE_PID));
+ rcu_assign_pointer(client->name, name);
if (!clients->root)
return 0; /* intel_fbdev_init registers a client before sysfs */
@@ -125,8 +138,8 @@ __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));
+ put_pid(rcu_replace_pointer(client->pid, NULL, true));
+ kfree(rcu_replace_pointer(client->name, NULL, true));
}
struct i915_drm_client *
@@ -140,6 +153,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;
ret = xa_alloc_cyclic(&clients->xarray, &client->id, client,
@@ -177,3 +191,72 @@ void i915_drm_client_close(struct i915_drm_client *client)
WRITE_ONCE(client->closed, true);
i915_drm_client_put(client);
}
+
+struct client_update_free {
+ struct rcu_head rcu;
+ struct pid *pid;
+ char *name;
+};
+
+static void __client_update_free(struct rcu_head *rcu)
+{
+ struct client_update_free *old = container_of(rcu, typeof(*old), rcu);
+
+ put_pid(old->pid);
+ kfree(old->name);
+ kfree(old);
+}
+
+int
+i915_drm_client_update(struct i915_drm_client *client,
+ struct task_struct *task)
+{
+ struct drm_i915_private *i915 =
+ container_of(client->clients, typeof(*i915), clients);
+ struct client_update_free *old;
+ struct pid *pid;
+ char *name;
+ int ret;
+
+ old = kmalloc(sizeof(*old), GFP_KERNEL);
+ if (!old)
+ return -ENOMEM;
+
+ ret = mutex_lock_interruptible(&client->update_lock);
+ if (ret)
+ goto out_free;
+
+ pid = get_task_pid(task, PIDTYPE_PID);
+ if (!pid)
+ goto out_pid;
+ if (pid == client->pid)
+ goto out_name;
+
+ name = kstrdup(task->comm, GFP_KERNEL);
+ if (!name) {
+ drm_notice(&i915->drm,
+ "Failed to update client id=%u,name=%s,pid=%u! (%d)\n",
+ client->id, client->name, pid_nr(client->pid), ret);
+ goto out_name;
+ }
+
+ init_rcu_head(&old->rcu);
+
+ old->pid = rcu_replace_pointer(client->pid, pid, true);
+ old->name = rcu_replace_pointer(client->name, name, true);
+
+ mutex_unlock(&client->update_lock);
+
+ call_rcu(&old->rcu, __client_update_free);
+
+ return 0;
+
+out_name:
+ put_pid(pid);
+out_pid:
+ mutex_unlock(&client->update_lock);
+out_free:
+ kfree(old);
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index af6998c74d4c..11b48383881d 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>
@@ -26,9 +27,11 @@ struct i915_drm_client {
struct rcu_head rcu;
+ struct mutex update_lock; /* Serializes name and pid updates. */
+
unsigned int id;
- struct pid *pid;
- char *name;
+ struct pid __rcu *pid;
+ char __rcu *name;
bool closed;
struct i915_drm_clients *clients;
@@ -61,4 +64,7 @@ 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);
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-14 13:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 6:12 [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create kbuild test robot
-- strict thread matches above, loose matches on Subject: below --
2020-09-14 13:12 [Intel-gfx] [PATCH 0/9] Per client engine busyness Tvrtko Ursulin
2020-09-14 13:12 ` [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create Tvrtko Ursulin
2020-09-04 12:59 [Intel-gfx] [PATCH 0/9] Per client engine busyness Tvrtko Ursulin
2020-09-04 12:59 ` [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create Tvrtko Ursulin
2020-04-15 10:11 [Intel-gfx] [PATCH 0/9] Per client engine busyness Tvrtko Ursulin
2020-04-15 10:11 ` [Intel-gfx] [PATCH 2/9] drm/i915: Update client name on context create Tvrtko Ursulin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.