Skip to content

L
linux
Commit 1d7cfea1 authored by Chris Wilson's avatar Chris Wilson
Browse files
Options

drm/i915: Do interrupible mutex lock first to avoid locking for unreference 

One of the primarily consumers of the i915 driver is X, a large signal
driven application. Frequently when writing into the buffers, there is a
pending signal which causes us not to take the interruptible lock but
then we need to take that same lock around the object unreference. By
rearranging the code to do the interruptible lock as the first check, we
can avoid the frequent additional locking around the unreference.
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
parent 139d363b
Hide whitespace changes
Inline Side-by-side
Showing with 93 additions and 113 deletions
drivers/gpu/drm/i915/i915_gem.c
View file @ 1d7cfea1
...@@ -547,16 +547,16 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, ...@@ -547,16 +547,16 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj_priv; struct drm_i915_gem_object *obj_priv;
int ret = 0; int ret = 0;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
return -ENOENT;
obj_priv = to_intel_bo(obj);
ret = i915_mutex_lock_interruptible(dev); ret = i915_mutex_lock_interruptible(dev);
if (ret) { if (ret)
drm_gem_object_unreference_unlocked(obj);
return ret; return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
ret = -ENOENT;
goto unlock;
} }
obj_priv = to_intel_bo(obj);
/* Bounds check source. */ /* Bounds check source. */
if (args->offset > obj->size || args->size > obj->size - args->offset) { if (args->offset > obj->size || args->size > obj->size - args->offset) {
...@@ -601,6 +601,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, ...@@ -601,6 +601,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
i915_gem_object_put_pages(obj); i915_gem_object_put_pages(obj);
out: out:
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret; return ret;
} }
...@@ -982,16 +983,17 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, ...@@ -982,16 +983,17 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj_priv; struct drm_i915_gem_object *obj_priv;
int ret = 0; int ret = 0;
obj = drm_gem_object_lookup(dev, file, args->handle);
if (obj == NULL)
return -ENOENT;
obj_priv = to_intel_bo(obj);
ret = i915_mutex_lock_interruptible(dev); ret = i915_mutex_lock_interruptible(dev);
if (ret) { if (ret)
drm_gem_object_unreference_unlocked(obj);
return ret; return ret;
obj = drm_gem_object_lookup(dev, file, args->handle);
if (obj == NULL) {
ret = -ENOENT;
goto unlock;
} }
obj_priv = to_intel_bo(obj);
/* Bounds check destination. */ /* Bounds check destination. */
if (args->offset > obj->size || args->size > obj->size - args->offset) { if (args->offset > obj->size || args->size > obj->size - args->offset) {
...@@ -1062,6 +1064,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, ...@@ -1062,6 +1064,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
out: out:
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret; return ret;
} }
...@@ -1098,16 +1101,16 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, ...@@ -1098,16 +1101,16 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
if (write_domain != 0 && read_domains != write_domain) if (write_domain != 0 && read_domains != write_domain)
return -EINVAL; return -EINVAL;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
return -ENOENT;
obj_priv = to_intel_bo(obj);
ret = i915_mutex_lock_interruptible(dev); ret = i915_mutex_lock_interruptible(dev);
if (ret) { if (ret)
drm_gem_object_unreference_unlocked(obj);
return ret; return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
ret = -ENOENT;
goto unlock;
} }
obj_priv = to_intel_bo(obj);
intel_mark_busy(dev, obj); intel_mark_busy(dev, obj);
...@@ -1139,6 +1142,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, ...@@ -1139,6 +1142,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list); list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret; return ret;
} }
...@@ -1157,14 +1161,14 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, ...@@ -1157,14 +1161,14 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
if (!(dev->driver->driver_features & DRIVER_GEM)) if (!(dev->driver->driver_features & DRIVER_GEM))
return -ENODEV; return -ENODEV;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
return -ENOENT;
ret = i915_mutex_lock_interruptible(dev); ret = i915_mutex_lock_interruptible(dev);
if (ret) { if (ret)
drm_gem_object_unreference_unlocked(obj);
return ret; return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
ret = -ENOENT;
goto unlock;
} }
/* Pinned buffers may be scanout, so flush the cache */ /* Pinned buffers may be scanout, so flush the cache */
...@@ -1172,6 +1176,7 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, ...@@ -1172,6 +1176,7 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
i915_gem_object_flush_cpu_write_domain(obj); i915_gem_object_flush_cpu_write_domain(obj);
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret; return ret;
} }
...@@ -1469,33 +1474,27 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, ...@@ -1469,33 +1474,27 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
if (!(dev->driver->driver_features & DRIVER_GEM)) if (!(dev->driver->driver_features & DRIVER_GEM))
return -ENODEV; return -ENODEV;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
return -ENOENT;
ret = i915_mutex_lock_interruptible(dev); ret = i915_mutex_lock_interruptible(dev);
if (ret) { if (ret)
drm_gem_object_unreference_unlocked(obj);
return ret; return ret;
}
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
obj_priv = to_intel_bo(obj); obj_priv = to_intel_bo(obj);
if (obj_priv->madv != I915_MADV_WILLNEED) { if (obj_priv->madv != I915_MADV_WILLNEED) {
DRM_ERROR("Attempting to mmap a purgeable buffer\n"); DRM_ERROR("Attempting to mmap a purgeable buffer\n");
drm_gem_object_unreference(obj); ret = -EINVAL;
mutex_unlock(&dev->struct_mutex); goto out;
return -EINVAL;
} }
if (!obj_priv->mmap_offset) { if (!obj_priv->mmap_offset) {
ret = i915_gem_create_mmap_offset(obj); ret = i915_gem_create_mmap_offset(obj);
if (ret) { if (ret)
drm_gem_object_unreference(obj); goto out;
mutex_unlock(&dev->struct_mutex);
return ret;
}
} }
args->offset = obj_priv->mmap_offset; args->offset = obj_priv->mmap_offset;
...@@ -1506,17 +1505,15 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, ...@@ -1506,17 +1505,15 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
*/ */
if (!obj_priv->agp_mem) { if (!obj_priv->agp_mem) {
ret = i915_gem_object_bind_to_gtt(obj, 0); ret = i915_gem_object_bind_to_gtt(obj, 0);
if (ret) { if (ret)
drm_gem_object_unreference(obj); goto out;
mutex_unlock(&dev->struct_mutex);
return ret;
}
} }
out:
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret;
return 0;
} }
static void static void
...@@ -4100,44 +4097,36 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, ...@@ -4100,44 +4097,36 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj_priv; struct drm_i915_gem_object *obj_priv;
int ret; int ret;
ret = i915_mutex_lock_interruptible(dev);
if (ret)
return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle); obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) { if (obj == NULL) {
DRM_ERROR("Bad handle in i915_gem_pin_ioctl(): %d\n", ret = -ENOENT;
args->handle); goto unlock;
return -ENOENT;
} }
obj_priv = to_intel_bo(obj); obj_priv = to_intel_bo(obj);
ret = i915_mutex_lock_interruptible(dev);
if (ret) {
drm_gem_object_unreference_unlocked(obj);
return ret;
}
if (obj_priv->madv != I915_MADV_WILLNEED) { if (obj_priv->madv != I915_MADV_WILLNEED) {
DRM_ERROR("Attempting to pin a purgeable buffer\n"); DRM_ERROR("Attempting to pin a purgeable buffer\n");
drm_gem_object_unreference(obj); ret = -EINVAL;
mutex_unlock(&dev->struct_mutex); goto out;
return -EINVAL;
} }
if (obj_priv->pin_filp != NULL && obj_priv->pin_filp != file_priv) { if (obj_priv->pin_filp != NULL && obj_priv->pin_filp != file_priv) {
DRM_ERROR("Already pinned in i915_gem_pin_ioctl(): %d\n", DRM_ERROR("Already pinned in i915_gem_pin_ioctl(): %d\n",
args->handle); args->handle);
drm_gem_object_unreference(obj); ret = -EINVAL;
mutex_unlock(&dev->struct_mutex); goto out;
return -EINVAL;
} }
obj_priv->user_pin_count++; obj_priv->user_pin_count++;
obj_priv->pin_filp = file_priv; obj_priv->pin_filp = file_priv;
if (obj_priv->user_pin_count == 1) { if (obj_priv->user_pin_count == 1) {
ret = i915_gem_object_pin(obj, args->alignment); ret = i915_gem_object_pin(obj, args->alignment);
if (ret != 0) { if (ret)
drm_gem_object_unreference(obj); goto out;
mutex_unlock(&dev->struct_mutex);
return ret;
}
} }
/* XXX - flush the CPU caches for pinned objects /* XXX - flush the CPU caches for pinned objects
...@@ -4145,10 +4134,11 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, ...@@ -4145,10 +4134,11 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
*/ */
i915_gem_object_flush_cpu_write_domain(obj); i915_gem_object_flush_cpu_write_domain(obj);
args->offset = obj_priv->gtt_offset; args->offset = obj_priv->gtt_offset;
out:
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret;
return 0;
} }
int int
...@@ -4160,27 +4150,22 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, ...@@ -4160,27 +4150,22 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj_priv; struct drm_i915_gem_object *obj_priv;
int ret; int ret;
ret = i915_mutex_lock_interruptible(dev);
if (ret)
return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle); obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) { if (obj == NULL) {
DRM_ERROR("Bad handle in i915_gem_unpin_ioctl(): %d\n", ret = -ENOENT;
args->handle); goto unlock;
return -ENOENT;
} }
obj_priv = to_intel_bo(obj); obj_priv = to_intel_bo(obj);
ret = i915_mutex_lock_interruptible(dev);
if (ret) {
drm_gem_object_unreference_unlocked(obj);
return ret;
}
if (obj_priv->pin_filp != file_priv) { if (obj_priv->pin_filp != file_priv) {
DRM_ERROR("Not pinned by caller in i915_gem_pin_ioctl(): %d\n", DRM_ERROR("Not pinned by caller in i915_gem_pin_ioctl(): %d\n",
args->handle); args->handle);
drm_gem_object_unreference(obj); ret = -EINVAL;
mutex_unlock(&dev->struct_mutex); goto out;
return -EINVAL;
} }
obj_priv->user_pin_count--; obj_priv->user_pin_count--;
if (obj_priv->user_pin_count == 0) { if (obj_priv->user_pin_count == 0) {
...@@ -4188,9 +4173,11 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, ...@@ -4188,9 +4173,11 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data,
i915_gem_object_unpin(obj); i915_gem_object_unpin(obj);
} }
out:
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return 0; return ret;
} }
int int
...@@ -4202,25 +4189,22 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, ...@@ -4202,25 +4189,22 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj_priv; struct drm_i915_gem_object *obj_priv;
int ret; int ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
DRM_ERROR("Bad handle in i915_gem_busy_ioctl(): %d\n",
args->handle);
return -ENOENT;
}
ret = i915_mutex_lock_interruptible(dev); ret = i915_mutex_lock_interruptible(dev);
if (ret) { if (ret)
drm_gem_object_unreference_unlocked(obj);
return ret; return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
ret = -ENOENT;
goto unlock;
} }
obj_priv = to_intel_bo(obj);
/* Count all active objects as busy, even if they are currently not used /* Count all active objects as busy, even if they are currently not used
* by the gpu. Users of this interface expect objects to eventually * by the gpu. Users of this interface expect objects to eventually
* become non-busy without any further actions, therefore emit any * become non-busy without any further actions, therefore emit any
* necessary flushes here. * necessary flushes here.
*/ */
obj_priv = to_intel_bo(obj);
args->busy = obj_priv->active; args->busy = obj_priv->active;
if (args->busy) { if (args->busy) {
/* Unconditionally flush objects, even when the gpu still uses this /* Unconditionally flush objects, even when the gpu still uses this
...@@ -4244,8 +4228,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, ...@@ -4244,8 +4228,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
} }
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return 0; return ret;
} }
int int
...@@ -4272,26 +4257,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, ...@@ -4272,26 +4257,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
return -EINVAL; return -EINVAL;
} }
ret = i915_mutex_lock_interruptible(dev);
if (ret)
return ret;
obj = drm_gem_object_lookup(dev, file_priv, args->handle); obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) { if (obj == NULL) {
DRM_ERROR("Bad handle in i915_gem_madvise_ioctl(): %d\n", ret = -ENOENT;
args->handle); goto unlock;
return -ENOENT;
} }
obj_priv = to_intel_bo(obj); obj_priv = to_intel_bo(obj);
ret = i915_mutex_lock_interruptible(dev);
if (ret) {
drm_gem_object_unreference_unlocked(obj);
return ret;
}
if (obj_priv->pin_count) { if (obj_priv->pin_count) {
drm_gem_object_unreference(obj); ret = -EINVAL;
mutex_unlock(&dev->struct_mutex); goto out;
DRM_ERROR("Attempted i915_gem_madvise_ioctl() on a pinned object\n");
return -EINVAL;
} }
if (obj_priv->madv != __I915_MADV_PURGED) if (obj_priv->madv != __I915_MADV_PURGED)
...@@ -4304,10 +4283,11 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, ...@@ -4304,10 +4283,11 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
args->retained = obj_priv->madv != __I915_MADV_PURGED; args->retained = obj_priv->madv != __I915_MADV_PURGED;
out:
drm_gem_object_unreference(obj); drm_gem_object_unreference(obj);
unlock:
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
return ret;
return 0;
} }
struct drm_gem_object * i915_gem_alloc_object(struct drm_device *dev, struct drm_gem_object * i915_gem_alloc_object(struct drm_device *dev,
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7