drm/msm: Fix debugfs deadlock
In normal cases the gem obj lock is acquired first before mm_lock. The
exception is iterating the various object lists. In the shrinker path,
deadlock is avoided by using msm_gem_trylock() and skipping over objects
that cannot be locked. But for debugfs the straightforward thing is to
split things out into a separate list of all objects protected by it's
own lock.
Fixes: d984457b31
("drm/msm: Add priv->mm_lock to protect active/inactive lists")
Signed-off-by: Rob Clark <robdclark@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20210401012722.527712-4-robdclark@gmail.com
Signed-off-by: Rob Clark <robdclark@chromium.org>
This commit is contained in:
parent
cc8a4d5a1b
commit
6ed0897cd8
5 changed files with 35 additions and 15 deletions
|
@ -111,23 +111,15 @@ static const struct file_operations msm_gpu_fops = {
|
||||||
static int msm_gem_show(struct drm_device *dev, struct seq_file *m)
|
static int msm_gem_show(struct drm_device *dev, struct seq_file *m)
|
||||||
{
|
{
|
||||||
struct msm_drm_private *priv = dev->dev_private;
|
struct msm_drm_private *priv = dev->dev_private;
|
||||||
struct msm_gpu *gpu = priv->gpu;
|
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
ret = mutex_lock_interruptible(&priv->mm_lock);
|
ret = mutex_lock_interruptible(&priv->obj_lock);
|
||||||
if (ret)
|
if (ret)
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
if (gpu) {
|
msm_gem_describe_objects(&priv->objects, m);
|
||||||
seq_printf(m, "Active Objects (%s):\n", gpu->name);
|
|
||||||
msm_gem_describe_objects(&gpu->active_list, m);
|
|
||||||
}
|
|
||||||
|
|
||||||
seq_printf(m, "Inactive Objects:\n");
|
mutex_unlock(&priv->obj_lock);
|
||||||
msm_gem_describe_objects(&priv->inactive_dontneed, m);
|
|
||||||
msm_gem_describe_objects(&priv->inactive_willneed, m);
|
|
||||||
|
|
||||||
mutex_unlock(&priv->mm_lock);
|
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
|
@ -446,6 +446,9 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
|
||||||
|
|
||||||
priv->wq = alloc_ordered_workqueue("msm", 0);
|
priv->wq = alloc_ordered_workqueue("msm", 0);
|
||||||
|
|
||||||
|
INIT_LIST_HEAD(&priv->objects);
|
||||||
|
mutex_init(&priv->obj_lock);
|
||||||
|
|
||||||
INIT_LIST_HEAD(&priv->inactive_willneed);
|
INIT_LIST_HEAD(&priv->inactive_willneed);
|
||||||
INIT_LIST_HEAD(&priv->inactive_dontneed);
|
INIT_LIST_HEAD(&priv->inactive_dontneed);
|
||||||
INIT_LIST_HEAD(&priv->inactive_purged);
|
INIT_LIST_HEAD(&priv->inactive_purged);
|
||||||
|
|
|
@ -174,7 +174,14 @@ struct msm_drm_private {
|
||||||
struct msm_rd_state *hangrd; /* debugfs to dump hanging submits */
|
struct msm_rd_state *hangrd; /* debugfs to dump hanging submits */
|
||||||
struct msm_perf_state *perf;
|
struct msm_perf_state *perf;
|
||||||
|
|
||||||
/*
|
/**
|
||||||
|
* List of all GEM objects (mainly for debugfs, protected by obj_lock
|
||||||
|
* (acquire before per GEM object lock)
|
||||||
|
*/
|
||||||
|
struct list_head objects;
|
||||||
|
struct mutex obj_lock;
|
||||||
|
|
||||||
|
/**
|
||||||
* Lists of inactive GEM objects. Every bo is either in one of the
|
* Lists of inactive GEM objects. Every bo is either in one of the
|
||||||
* inactive lists (depending on whether or not it is shrinkable) or
|
* inactive lists (depending on whether or not it is shrinkable) or
|
||||||
* gpu->active_list (for the gpu it is active on[1])
|
* gpu->active_list (for the gpu it is active on[1])
|
||||||
|
|
|
@ -961,7 +961,7 @@ void msm_gem_describe_objects(struct list_head *list, struct seq_file *m)
|
||||||
size_t size = 0;
|
size_t size = 0;
|
||||||
|
|
||||||
seq_puts(m, " flags id ref offset kaddr size madv name\n");
|
seq_puts(m, " flags id ref offset kaddr size madv name\n");
|
||||||
list_for_each_entry(msm_obj, list, mm_list) {
|
list_for_each_entry(msm_obj, list, node) {
|
||||||
struct drm_gem_object *obj = &msm_obj->base;
|
struct drm_gem_object *obj = &msm_obj->base;
|
||||||
seq_puts(m, " ");
|
seq_puts(m, " ");
|
||||||
msm_gem_describe(obj, m);
|
msm_gem_describe(obj, m);
|
||||||
|
@ -980,6 +980,10 @@ void msm_gem_free_object(struct drm_gem_object *obj)
|
||||||
struct drm_device *dev = obj->dev;
|
struct drm_device *dev = obj->dev;
|
||||||
struct msm_drm_private *priv = dev->dev_private;
|
struct msm_drm_private *priv = dev->dev_private;
|
||||||
|
|
||||||
|
mutex_lock(&priv->obj_lock);
|
||||||
|
list_del(&msm_obj->node);
|
||||||
|
mutex_unlock(&priv->obj_lock);
|
||||||
|
|
||||||
mutex_lock(&priv->mm_lock);
|
mutex_lock(&priv->mm_lock);
|
||||||
if (msm_obj->dontneed)
|
if (msm_obj->dontneed)
|
||||||
mark_unpurgable(msm_obj);
|
mark_unpurgable(msm_obj);
|
||||||
|
@ -1169,6 +1173,10 @@ static struct drm_gem_object *_msm_gem_new(struct drm_device *dev,
|
||||||
list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed);
|
list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed);
|
||||||
mutex_unlock(&priv->mm_lock);
|
mutex_unlock(&priv->mm_lock);
|
||||||
|
|
||||||
|
mutex_lock(&priv->obj_lock);
|
||||||
|
list_add_tail(&msm_obj->node, &priv->objects);
|
||||||
|
mutex_unlock(&priv->obj_lock);
|
||||||
|
|
||||||
return obj;
|
return obj;
|
||||||
|
|
||||||
fail:
|
fail:
|
||||||
|
@ -1239,6 +1247,10 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
|
||||||
list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed);
|
list_add_tail(&msm_obj->mm_list, &priv->inactive_willneed);
|
||||||
mutex_unlock(&priv->mm_lock);
|
mutex_unlock(&priv->mm_lock);
|
||||||
|
|
||||||
|
mutex_lock(&priv->obj_lock);
|
||||||
|
list_add_tail(&msm_obj->node, &priv->objects);
|
||||||
|
mutex_unlock(&priv->obj_lock);
|
||||||
|
|
||||||
return obj;
|
return obj;
|
||||||
|
|
||||||
fail:
|
fail:
|
||||||
|
|
|
@ -60,10 +60,16 @@ struct msm_gem_object {
|
||||||
*/
|
*/
|
||||||
uint8_t vmap_count;
|
uint8_t vmap_count;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Node in list of all objects (mainly for debugfs, protected by
|
||||||
|
* priv->obj_lock
|
||||||
|
*/
|
||||||
|
struct list_head node;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* An object is either:
|
* An object is either:
|
||||||
* inactive - on priv->inactive_dontneed/willneed/purged depending
|
* inactive - on priv->inactive_dontneed or priv->inactive_willneed
|
||||||
* on status
|
* (depending on purgability status)
|
||||||
* active - on one one of the gpu's active_list.. well, at
|
* active - on one one of the gpu's active_list.. well, at
|
||||||
* least for now we don't have (I don't think) hw sync between
|
* least for now we don't have (I don't think) hw sync between
|
||||||
* 2d and 3d one devices which have both, meaning we need to
|
* 2d and 3d one devices which have both, meaning we need to
|
||||||
|
|
Loading…
Add table
Reference in a new issue