Commit 09b4ea47 authored by Ilija Hadzic's avatar Ilija Hadzic Committed by Dave Airlie

drm: make DRM_UNLOCKED ioctls with their own mutex

drm_getclient, drm_getstats and drm_getmap (with a few minor
adjustments) do not need global mutex, so fix that and
make the said ioctls DRM_UNLOCKED. Details:

  drm_getclient: the only thing that should be protected here
  is dev->filelist and that is already protected everywhere with
  dev->struct_mutex.

  drm_getstats: there is no need for any mutex here because the
  loop runs through quasi-static (set at load time only)
  data, and the actual count access is done with atomic_read()

  drm_getmap already uses dev->struct_mutex to protect
  dev->maplist, which also used to protect the same structure
  everywhere else except at three places:
  * drm_getsarea, which doesn't grab *any* mutex before
    touching dev->maplist (so no drm_global_mutex doesn't help
    here either; different issue for a different patch).
    However, drivers seem to call it only at
    initialization time so it probably doesn't matter
  * drm_master_destroy, which is called from drm_master_put,
    which in turn is protected with dev->struct_mutex
    everywhere else in drm module, so we are good here too.
  * drm_getsareactx, which releases the dev->struct_mutex
    too early, but this patch includes the fix for that.

v2: * incorporate comments received from Daniel Vetter
    * include the (long) explanation above to make it clear what
      we are doing (and why), also at Daniel Vetter's request
    * tighten up mutex grab/release locations to only
      encompass real critical sections, rather than some
      random code around them
Signed-off-by: default avatarIlija Hadzic <ihadzic@research.bell-labs.com>
Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
parent 53fead96
...@@ -154,8 +154,6 @@ int drm_getsareactx(struct drm_device *dev, void *data, ...@@ -154,8 +154,6 @@ int drm_getsareactx(struct drm_device *dev, void *data,
return -EINVAL; return -EINVAL;
} }
mutex_unlock(&dev->struct_mutex);
request->handle = NULL; request->handle = NULL;
list_for_each_entry(_entry, &dev->maplist, head) { list_for_each_entry(_entry, &dev->maplist, head) {
if (_entry->map == map) { if (_entry->map == map) {
...@@ -164,6 +162,9 @@ int drm_getsareactx(struct drm_device *dev, void *data, ...@@ -164,6 +162,9 @@ int drm_getsareactx(struct drm_device *dev, void *data,
break; break;
} }
} }
mutex_unlock(&dev->struct_mutex);
if (request->handle == NULL) if (request->handle == NULL)
return -EINVAL; return -EINVAL;
......
...@@ -65,9 +65,9 @@ static struct drm_ioctl_desc drm_ioctls[] = { ...@@ -65,9 +65,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0),
DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
......
...@@ -158,14 +158,11 @@ int drm_getmap(struct drm_device *dev, void *data, ...@@ -158,14 +158,11 @@ int drm_getmap(struct drm_device *dev, void *data,
int i; int i;
idx = map->offset; idx = map->offset;
if (idx < 0)
mutex_lock(&dev->struct_mutex);
if (idx < 0) {
mutex_unlock(&dev->struct_mutex);
return -EINVAL; return -EINVAL;
}
i = 0; i = 0;
mutex_lock(&dev->struct_mutex);
list_for_each(list, &dev->maplist) { list_for_each(list, &dev->maplist) {
if (i == idx) { if (i == idx) {
r_list = list_entry(list, struct drm_map_list, head); r_list = list_entry(list, struct drm_map_list, head);
...@@ -211,9 +208,9 @@ int drm_getclient(struct drm_device *dev, void *data, ...@@ -211,9 +208,9 @@ int drm_getclient(struct drm_device *dev, void *data,
int i; int i;
idx = client->idx; idx = client->idx;
mutex_lock(&dev->struct_mutex);
i = 0; i = 0;
mutex_lock(&dev->struct_mutex);
list_for_each_entry(pt, &dev->filelist, lhead) { list_for_each_entry(pt, &dev->filelist, lhead) {
if (i++ >= idx) { if (i++ >= idx) {
client->auth = pt->authenticated; client->auth = pt->authenticated;
...@@ -249,8 +246,6 @@ int drm_getstats(struct drm_device *dev, void *data, ...@@ -249,8 +246,6 @@ int drm_getstats(struct drm_device *dev, void *data,
memset(stats, 0, sizeof(*stats)); memset(stats, 0, sizeof(*stats));
mutex_lock(&dev->struct_mutex);
for (i = 0; i < dev->counters; i++) { for (i = 0; i < dev->counters; i++) {
if (dev->types[i] == _DRM_STAT_LOCK) if (dev->types[i] == _DRM_STAT_LOCK)
stats->data[i].value = stats->data[i].value =
...@@ -262,8 +257,6 @@ int drm_getstats(struct drm_device *dev, void *data, ...@@ -262,8 +257,6 @@ int drm_getstats(struct drm_device *dev, void *data,
stats->count = dev->counters; stats->count = dev->counters;
mutex_unlock(&dev->struct_mutex);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment