Commit c74f7e82 authored by Alexander Shishkin's avatar Alexander Shishkin Committed by Greg Kroah-Hartman

stm class: Fix link list locking

Currently, the list of stm_sources linked to an stm device is protected by
a spinlock, which also means that sources' .unlink() method is called under
this spinlock. However, this method may (and does) sleep, which means
trouble.

This patch slightly reworks locking around stm::link_list so that bits that
might_sleep() are called with a mutex held instead. Modification of this
list requires both mutex and spinlock to be held, while looking at the list
can be done under either mutex or spinlock.
Signed-off-by: default avatarAlexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 4c127fd1
...@@ -641,6 +641,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, ...@@ -641,6 +641,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
if (err) if (err)
goto err_device; goto err_device;
mutex_init(&stm->link_mutex);
spin_lock_init(&stm->link_lock); spin_lock_init(&stm->link_lock);
INIT_LIST_HEAD(&stm->link_list); INIT_LIST_HEAD(&stm->link_list);
...@@ -671,11 +672,11 @@ void stm_unregister_device(struct stm_data *stm_data) ...@@ -671,11 +672,11 @@ void stm_unregister_device(struct stm_data *stm_data)
struct stm_source_device *src, *iter; struct stm_source_device *src, *iter;
int i; int i;
spin_lock(&stm->link_lock); mutex_lock(&stm->link_mutex);
list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) { list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
__stm_source_link_drop(src, stm); __stm_source_link_drop(src, stm);
} }
spin_unlock(&stm->link_lock); mutex_unlock(&stm->link_mutex);
synchronize_srcu(&stm_source_srcu); synchronize_srcu(&stm_source_srcu);
...@@ -694,6 +695,17 @@ void stm_unregister_device(struct stm_data *stm_data) ...@@ -694,6 +695,17 @@ void stm_unregister_device(struct stm_data *stm_data)
} }
EXPORT_SYMBOL_GPL(stm_unregister_device); EXPORT_SYMBOL_GPL(stm_unregister_device);
/*
* stm::link_list access serialization uses a spinlock and a mutex; holding
* either of them guarantees that the list is stable; modification requires
* holding both of them.
*
* Lock ordering is as follows:
* stm::link_mutex
* stm::link_lock
* src::link_lock
*/
/** /**
* stm_source_link_add() - connect an stm_source device to an stm device * stm_source_link_add() - connect an stm_source device to an stm device
* @src: stm_source device * @src: stm_source device
...@@ -710,6 +722,7 @@ static int stm_source_link_add(struct stm_source_device *src, ...@@ -710,6 +722,7 @@ static int stm_source_link_add(struct stm_source_device *src,
char *id; char *id;
int err; int err;
mutex_lock(&stm->link_mutex);
spin_lock(&stm->link_lock); spin_lock(&stm->link_lock);
spin_lock(&src->link_lock); spin_lock(&src->link_lock);
...@@ -719,6 +732,7 @@ static int stm_source_link_add(struct stm_source_device *src, ...@@ -719,6 +732,7 @@ static int stm_source_link_add(struct stm_source_device *src,
spin_unlock(&src->link_lock); spin_unlock(&src->link_lock);
spin_unlock(&stm->link_lock); spin_unlock(&stm->link_lock);
mutex_unlock(&stm->link_mutex);
id = kstrdup(src->data->name, GFP_KERNEL); id = kstrdup(src->data->name, GFP_KERNEL);
if (id) { if (id) {
...@@ -756,6 +770,7 @@ static int stm_source_link_add(struct stm_source_device *src, ...@@ -756,6 +770,7 @@ static int stm_source_link_add(struct stm_source_device *src,
stm_put_device(stm); stm_put_device(stm);
fail_detach: fail_detach:
mutex_lock(&stm->link_mutex);
spin_lock(&stm->link_lock); spin_lock(&stm->link_lock);
spin_lock(&src->link_lock); spin_lock(&src->link_lock);
...@@ -764,6 +779,7 @@ static int stm_source_link_add(struct stm_source_device *src, ...@@ -764,6 +779,7 @@ static int stm_source_link_add(struct stm_source_device *src,
spin_unlock(&src->link_lock); spin_unlock(&src->link_lock);
spin_unlock(&stm->link_lock); spin_unlock(&stm->link_lock);
mutex_unlock(&stm->link_mutex);
return err; return err;
} }
...@@ -776,13 +792,20 @@ static int stm_source_link_add(struct stm_source_device *src, ...@@ -776,13 +792,20 @@ static int stm_source_link_add(struct stm_source_device *src,
* If @stm is @src::link, disconnect them from one another and put the * If @stm is @src::link, disconnect them from one another and put the
* reference on the @stm device. * reference on the @stm device.
* *
* Caller must hold stm::link_lock. * Caller must hold stm::link_mutex.
*/ */
static void __stm_source_link_drop(struct stm_source_device *src, static void __stm_source_link_drop(struct stm_source_device *src,
struct stm_device *stm) struct stm_device *stm)
{ {
struct stm_device *link; struct stm_device *link;
lockdep_assert_held(&stm->link_mutex);
if (src->data->unlink)
src->data->unlink(src->data);
/* for stm::link_list modification, we hold both mutex and spinlock */
spin_lock(&stm->link_lock);
spin_lock(&src->link_lock); spin_lock(&src->link_lock);
link = srcu_dereference_check(src->link, &stm_source_srcu, 1); link = srcu_dereference_check(src->link, &stm_source_srcu, 1);
if (WARN_ON_ONCE(link != stm)) { if (WARN_ON_ONCE(link != stm)) {
...@@ -791,13 +814,13 @@ static void __stm_source_link_drop(struct stm_source_device *src, ...@@ -791,13 +814,13 @@ static void __stm_source_link_drop(struct stm_source_device *src,
} }
stm_output_free(link, &src->output); stm_output_free(link, &src->output);
/* caller must hold stm::link_lock */
list_del_init(&src->link_entry); list_del_init(&src->link_entry);
/* matches stm_find_device() from stm_source_link_store() */ /* matches stm_find_device() from stm_source_link_store() */
stm_put_device(link); stm_put_device(link);
rcu_assign_pointer(src->link, NULL); rcu_assign_pointer(src->link, NULL);
spin_unlock(&src->link_lock); spin_unlock(&src->link_lock);
spin_unlock(&stm->link_lock);
} }
/** /**
...@@ -819,12 +842,9 @@ static void stm_source_link_drop(struct stm_source_device *src) ...@@ -819,12 +842,9 @@ static void stm_source_link_drop(struct stm_source_device *src)
stm = srcu_dereference(src->link, &stm_source_srcu); stm = srcu_dereference(src->link, &stm_source_srcu);
if (stm) { if (stm) {
if (src->data->unlink) mutex_lock(&stm->link_mutex);
src->data->unlink(src->data);
spin_lock(&stm->link_lock);
__stm_source_link_drop(src, stm); __stm_source_link_drop(src, stm);
spin_unlock(&stm->link_lock); mutex_unlock(&stm->link_mutex);
} }
srcu_read_unlock(&stm_source_srcu, idx); srcu_read_unlock(&stm_source_srcu, idx);
......
...@@ -45,6 +45,7 @@ struct stm_device { ...@@ -45,6 +45,7 @@ struct stm_device {
int major; int major;
unsigned int sw_nmasters; unsigned int sw_nmasters;
struct stm_data *data; struct stm_data *data;
struct mutex link_mutex;
spinlock_t link_lock; spinlock_t link_lock;
struct list_head link_list; struct list_head link_list;
/* master allocation */ /* master allocation */
......
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