[1/5,media] media-device: get rid of the spinlock
diff mbox

Message ID dba4d41bdfa6bb8dc51cb0f692102919b2b7c8b4.1458129823.git.mchehab@osg.samsung.com
State New
Headers show

Commit Message

Mauro Carvalho Chehab March 16, 2016, 12:04 p.m. UTC
Right now, the lock schema for media_device struct is messy,
since sometimes, it is protected via a spin lock, while, for
media graph traversal, it is protected by a mutex.

Solve this conflict by always using a mutex.

As a side effect, this prevents a bug where the media notifiers
were called at atomic context.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/media-device.c | 32 ++++++++++++++------------------
 drivers/media/media-entity.c | 16 ++++++++--------
 include/media/media-device.h |  6 +-----
 3 files changed, 23 insertions(+), 31 deletions(-)

Comments

Javier Martinez Canillas March 16, 2016, 12:53 p.m. UTC | #1
Hello Mauro,

On Wed, Mar 16, 2016 at 9:04 AM, Mauro Carvalho Chehab
<mchehab@osg.samsung.com> wrote:
> Right now, the lock schema for media_device struct is messy,
> since sometimes, it is protected via a spin lock, while, for
> media graph traversal, it is protected by a mutex.
>
> Solve this conflict by always using a mutex.
>
> As a side effect, this prevents a bug where the media notifiers
> were called at atomic context.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---

I agree with the patch.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab March 16, 2016, 1:10 p.m. UTC | #2
Em Wed, 16 Mar 2016 09:53:12 -0300
Javier Martinez Canillas <javier@dowhile0.org> escreveu:

> Hello Mauro,
> 
> On Wed, Mar 16, 2016 at 9:04 AM, Mauro Carvalho Chehab
> <mchehab@osg.samsung.com> wrote:
> > Right now, the lock schema for media_device struct is messy,
> > since sometimes, it is protected via a spin lock, while, for
> > media graph traversal, it is protected by a mutex.
> >
> > Solve this conflict by always using a mutex.
> >
> > As a side effect, this prevents a bug where the media notifiers
> > were called at atomic context.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Btw, I'm running a stress test here, doing bind/unbind on au0828,
while calling mc_nextgen_test:

Running one instance of this loop:
	$ i=0; while :; do i=$((i+1)); echo "loop $i"; sudo su -c "echo 1-3.1.2:1.0 > /sys/bus/usb/drivers/au0828/bind"; sudo su -c "echo 1-3.1.2:1.0 > /sys/bus/usb/drivers/au0828/unbind"; done


and 3 instances of this loop:
	$ while :; do clear; mc_nextgen_test; done

My test machine has 4 CPUs, so this should be enough to check
if the mutexes at ioctl and at the register/unregister functions
are ok.

Right now, the loop ran 160 times. Not a single trouble.

Ok, it is not doing any graph traversal ops, but the code seems to be
pretty much reliable with mutexes.

I'll keep it running for more time to be sure, but it seems that
the current media core works fine for dynamic
entity/interface/link addition/removal.

Regards,
Mauro


> > ---  
> 
> I agree with the patch.
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> Best regards,
> Javier
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus March 16, 2016, 2:10 p.m. UTC | #3
Hi Mauro,

On Wed, Mar 16, 2016 at 09:04:02AM -0300, Mauro Carvalho Chehab wrote:
> Right now, the lock schema for media_device struct is messy,
> since sometimes, it is protected via a spin lock, while, for
> media graph traversal, it is protected by a mutex.
> 
> Solve this conflict by always using a mutex.
> 
> As a side effect, this prevents a bug where the media notifiers
> were called at atomic context.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

The scope of mdev->lock isn't much, really. I think this is fine, no need to
create another mutex.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Shuah Khan March 22, 2016, 7:52 p.m. UTC | #4
On 03/16/2016 06:04 AM, Mauro Carvalho Chehab wrote:
> Right now, the lock schema for media_device struct is messy,
> since sometimes, it is protected via a spin lock, while, for
> media graph traversal, it is protected by a mutex.
> 
> Solve this conflict by always using a mutex.
> 
> As a side effect, this prevents a bug where the media notifiers
> were called at atomic context.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Tested bind_unbind au0828 loop 1000 times, followed by bind_unbind
snd_usb_audio loop 1000 times. Didn't see any lock warnings on a
KASAN enabled kernel (lock testing enabled). No use-after-free errors
during these runs.

Ran device removal test and rmmod and modprobe tests on both drivers.

Generated graph after the runs and the graph looks good.

Reviewed-by: Shuah Khan <shuahkh@osg.samsung.com>
Tested-by: Shuah Khan <shuahkh@osg.samsung.com>

thanks,
-- Shuah

> ---
>  drivers/media/media-device.c | 32 ++++++++++++++------------------
>  drivers/media/media-entity.c | 16 ++++++++--------
>  include/media/media-device.h |  6 +-----
>  3 files changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 6e43c95629ea..c32fa15cc76e 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -90,17 +90,17 @@ static struct media_entity *find_entity(struct media_device *mdev, u32 id)
>  
>  	id &= ~MEDIA_ENT_ID_FLAG_NEXT;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  
>  	media_device_for_each_entity(entity, mdev) {
>  		if (((media_entity_id(entity) == id) && !next) ||
>  		    ((media_entity_id(entity) > id) && next)) {
> -			spin_unlock(&mdev->lock);
> +			mutex_unlock(&mdev->graph_mutex);
>  			return entity;
>  		}
>  	}
>  
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  
>  	return NULL;
>  }
> @@ -590,12 +590,12 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  	if (!ida_pre_get(&mdev->entity_internal_idx, GFP_KERNEL))
>  		return -ENOMEM;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  
>  	ret = ida_get_new_above(&mdev->entity_internal_idx, 1,
>  				&entity->internal_idx);
>  	if (ret < 0) {
> -		spin_unlock(&mdev->lock);
> +		mutex_unlock(&mdev->graph_mutex);
>  		return ret;
>  	}
>  
> @@ -615,9 +615,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  		(notify)->notify(entity, notify->notify_data);
>  	}
>  
> -	spin_unlock(&mdev->lock);
> -
> -	mutex_lock(&mdev->graph_mutex);
>  	if (mdev->entity_internal_idx_max
>  	    >= mdev->pm_count_walk.ent_enum.idx_max) {
>  		struct media_entity_graph new = { .top = 0 };
> @@ -680,9 +677,9 @@ void media_device_unregister_entity(struct media_entity *entity)
>  	if (mdev == NULL)
>  		return;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_device_unregister_entity(entity);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_device_unregister_entity);
>  
> @@ -703,7 +700,6 @@ void media_device_init(struct media_device *mdev)
>  	INIT_LIST_HEAD(&mdev->pads);
>  	INIT_LIST_HEAD(&mdev->links);
>  	INIT_LIST_HEAD(&mdev->entity_notify);
> -	spin_lock_init(&mdev->lock);
>  	mutex_init(&mdev->graph_mutex);
>  	ida_init(&mdev->entity_internal_idx);
>  
> @@ -752,9 +748,9 @@ EXPORT_SYMBOL_GPL(__media_device_register);
>  int __must_check media_device_register_entity_notify(struct media_device *mdev,
>  					struct media_entity_notify *nptr)
>  {
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	list_add_tail(&nptr->list, &mdev->entity_notify);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
> @@ -771,9 +767,9 @@ static void __media_device_unregister_entity_notify(struct media_device *mdev,
>  void media_device_unregister_entity_notify(struct media_device *mdev,
>  					struct media_entity_notify *nptr)
>  {
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_device_unregister_entity_notify(mdev, nptr);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
>  
> @@ -787,11 +783,11 @@ void media_device_unregister(struct media_device *mdev)
>  	if (mdev == NULL)
>  		return;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  
>  	/* Check if mdev was ever registered at all */
>  	if (!media_devnode_is_registered(&mdev->devnode)) {
> -		spin_unlock(&mdev->lock);
> +		mutex_unlock(&mdev->graph_mutex);
>  		return;
>  	}
>  
> @@ -811,7 +807,7 @@ void media_device_unregister(struct media_device *mdev)
>  		kfree(intf);
>  	}
>  
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  
>  	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
>  	media_devnode_unregister(&mdev->devnode);
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index e95070b3a3d4..c53c1d5589a0 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -219,7 +219,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	entity->pads = pads;
>  
>  	if (mdev)
> -		spin_lock(&mdev->lock);
> +		mutex_lock(&mdev->graph_mutex);
>  
>  	for (i = 0; i < num_pads; i++) {
>  		pads[i].entity = entity;
> @@ -230,7 +230,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	}
>  
>  	if (mdev)
> -		spin_unlock(&mdev->lock);
> +		mutex_unlock(&mdev->graph_mutex);
>  
>  	return 0;
>  }
> @@ -747,9 +747,9 @@ void media_entity_remove_links(struct media_entity *entity)
>  	if (mdev == NULL)
>  		return;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_entity_remove_links(entity);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_entity_remove_links);
>  
> @@ -951,9 +951,9 @@ void media_remove_intf_link(struct media_link *link)
>  	if (mdev == NULL)
>  		return;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_remove_intf_link(link);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_remove_intf_link);
>  
> @@ -975,8 +975,8 @@ void media_remove_intf_links(struct media_interface *intf)
>  	if (mdev == NULL)
>  		return;
>  
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_remove_intf_links(intf);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index df74cfa7da4a..0c2de97181f3 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -25,7 +25,6 @@
>  
>  #include <linux/list.h>
>  #include <linux/mutex.h>
> -#include <linux/spinlock.h>
>  
>  #include <media/media-devnode.h>
>  #include <media/media-entity.h>
> @@ -304,8 +303,7 @@ struct media_entity_notify {
>   * @pads:	List of registered pads
>   * @links:	List of registered links
>   * @entity_notify: List of registered entity_notify callbacks
> - * @lock:	Entities list lock
> - * @graph_mutex: Entities graph operation lock
> + * @graph_mutex: Protects access to struct media_device data
>   * @pm_count_walk: Graph walk for power state walk. Access serialised using
>   *		   graph_mutex.
>   *
> @@ -371,8 +369,6 @@ struct media_device {
>  	/* notify callback list invoked when a new entity is registered */
>  	struct list_head entity_notify;
>  
> -	/* Protects the graph objects creation/removal */
> -	spinlock_t lock;
>  	/* Serializes graph operations. */
>  	struct mutex graph_mutex;
>  	struct media_entity_graph pm_count_walk;
>
Laurent Pinchart March 24, 2016, 10:11 p.m. UTC | #5
On Wednesday 16 Mar 2016 09:04:02 Mauro Carvalho Chehab wrote:
> Right now, the lock schema for media_device struct is messy,
> since sometimes, it is protected via a spin lock, while, for
> media graph traversal, it is protected by a mutex.
> 
> Solve this conflict by always using a mutex.
> 
> As a side effect, this prevents a bug where the media notifiers
> were called at atomic context.

And as a side effect the kernel now deadlocks in MEDIA_IOC_ENUM_LINKS. I'm all 
for fixing messy the lock schema, but maybe you should test patches before 
merging them ?

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>  drivers/media/media-device.c | 32 ++++++++++++++------------------
>  drivers/media/media-entity.c | 16 ++++++++--------
>  include/media/media-device.h |  6 +-----
>  3 files changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 6e43c95629ea..c32fa15cc76e 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -90,17 +90,17 @@ static struct media_entity *find_entity(struct
> media_device *mdev, u32 id)
> 
>  	id &= ~MEDIA_ENT_ID_FLAG_NEXT;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
> 
>  	media_device_for_each_entity(entity, mdev) {
>  		if (((media_entity_id(entity) == id) && !next) ||
>  		    ((media_entity_id(entity) > id) && next)) {
> -			spin_unlock(&mdev->lock);
> +			mutex_unlock(&mdev->graph_mutex);
>  			return entity;
>  		}
>  	}
> 
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
> 
>  	return NULL;
>  }
> @@ -590,12 +590,12 @@ int __must_check media_device_register_entity(struct
> media_device *mdev, if (!ida_pre_get(&mdev->entity_internal_idx,
> GFP_KERNEL))
>  		return -ENOMEM;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
> 
>  	ret = ida_get_new_above(&mdev->entity_internal_idx, 1,
>  				&entity->internal_idx);
>  	if (ret < 0) {
> -		spin_unlock(&mdev->lock);
> +		mutex_unlock(&mdev->graph_mutex);
>  		return ret;
>  	}
> 
> @@ -615,9 +615,6 @@ int __must_check media_device_register_entity(struct
> media_device *mdev, (notify)->notify(entity, notify->notify_data);
>  	}
> 
> -	spin_unlock(&mdev->lock);
> -
> -	mutex_lock(&mdev->graph_mutex);
>  	if (mdev->entity_internal_idx_max
> 
>  	    >= mdev->pm_count_walk.ent_enum.idx_max) {
> 
>  		struct media_entity_graph new = { .top = 0 };
> @@ -680,9 +677,9 @@ void media_device_unregister_entity(struct media_entity
> *entity) if (mdev == NULL)
>  		return;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_device_unregister_entity(entity);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_device_unregister_entity);
> 
> @@ -703,7 +700,6 @@ void media_device_init(struct media_device *mdev)
>  	INIT_LIST_HEAD(&mdev->pads);
>  	INIT_LIST_HEAD(&mdev->links);
>  	INIT_LIST_HEAD(&mdev->entity_notify);
> -	spin_lock_init(&mdev->lock);
>  	mutex_init(&mdev->graph_mutex);
>  	ida_init(&mdev->entity_internal_idx);
> 
> @@ -752,9 +748,9 @@ EXPORT_SYMBOL_GPL(__media_device_register);
>  int __must_check media_device_register_entity_notify(struct media_device
> *mdev, struct media_entity_notify *nptr)
>  {
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	list_add_tail(&nptr->list, &mdev->entity_notify);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
> @@ -771,9 +767,9 @@ static void
> __media_device_unregister_entity_notify(struct media_device *mdev, void
> media_device_unregister_entity_notify(struct media_device *mdev, struct
> media_entity_notify *nptr)
>  {
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_device_unregister_entity_notify(mdev, nptr);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
> 
> @@ -787,11 +783,11 @@ void media_device_unregister(struct media_device
> *mdev) if (mdev == NULL)
>  		return;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
> 
>  	/* Check if mdev was ever registered at all */
>  	if (!media_devnode_is_registered(&mdev->devnode)) {
> -		spin_unlock(&mdev->lock);
> +		mutex_unlock(&mdev->graph_mutex);
>  		return;
>  	}
> 
> @@ -811,7 +807,7 @@ void media_device_unregister(struct media_device *mdev)
>  		kfree(intf);
>  	}
> 
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
> 
>  	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
>  	media_devnode_unregister(&mdev->devnode);
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index e95070b3a3d4..c53c1d5589a0 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -219,7 +219,7 @@ int media_entity_pads_init(struct media_entity *entity,
> u16 num_pads, entity->pads = pads;
> 
>  	if (mdev)
> -		spin_lock(&mdev->lock);
> +		mutex_lock(&mdev->graph_mutex);
> 
>  	for (i = 0; i < num_pads; i++) {
>  		pads[i].entity = entity;
> @@ -230,7 +230,7 @@ int media_entity_pads_init(struct media_entity *entity,
> u16 num_pads, }
> 
>  	if (mdev)
> -		spin_unlock(&mdev->lock);
> +		mutex_unlock(&mdev->graph_mutex);
> 
>  	return 0;
>  }
> @@ -747,9 +747,9 @@ void media_entity_remove_links(struct media_entity
> *entity) if (mdev == NULL)
>  		return;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_entity_remove_links(entity);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_entity_remove_links);
> 
> @@ -951,9 +951,9 @@ void media_remove_intf_link(struct media_link *link)
>  	if (mdev == NULL)
>  		return;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_remove_intf_link(link);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_remove_intf_link);
> 
> @@ -975,8 +975,8 @@ void media_remove_intf_links(struct media_interface
> *intf) if (mdev == NULL)
>  		return;
> 
> -	spin_lock(&mdev->lock);
> +	mutex_lock(&mdev->graph_mutex);
>  	__media_remove_intf_links(intf);
> -	spin_unlock(&mdev->lock);
> +	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index df74cfa7da4a..0c2de97181f3 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -25,7 +25,6 @@
> 
>  #include <linux/list.h>
>  #include <linux/mutex.h>
> -#include <linux/spinlock.h>
> 
>  #include <media/media-devnode.h>
>  #include <media/media-entity.h>
> @@ -304,8 +303,7 @@ struct media_entity_notify {
>   * @pads:	List of registered pads
>   * @links:	List of registered links
>   * @entity_notify: List of registered entity_notify callbacks
> - * @lock:	Entities list lock
> - * @graph_mutex: Entities graph operation lock
> + * @graph_mutex: Protects access to struct media_device data
>   * @pm_count_walk: Graph walk for power state walk. Access serialised using
> *		   graph_mutex.
>   *
> @@ -371,8 +369,6 @@ struct media_device {
>  	/* notify callback list invoked when a new entity is registered */
>  	struct list_head entity_notify;
> 
> -	/* Protects the graph objects creation/removal */
> -	spinlock_t lock;
>  	/* Serializes graph operations. */
>  	struct mutex graph_mutex;
>  	struct media_entity_graph pm_count_walk;

Patch
diff mbox

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6e43c95629ea..c32fa15cc76e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -90,17 +90,17 @@  static struct media_entity *find_entity(struct media_device *mdev, u32 id)
 
 	id &= ~MEDIA_ENT_ID_FLAG_NEXT;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 
 	media_device_for_each_entity(entity, mdev) {
 		if (((media_entity_id(entity) == id) && !next) ||
 		    ((media_entity_id(entity) > id) && next)) {
-			spin_unlock(&mdev->lock);
+			mutex_unlock(&mdev->graph_mutex);
 			return entity;
 		}
 	}
 
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 
 	return NULL;
 }
@@ -590,12 +590,12 @@  int __must_check media_device_register_entity(struct media_device *mdev,
 	if (!ida_pre_get(&mdev->entity_internal_idx, GFP_KERNEL))
 		return -ENOMEM;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 
 	ret = ida_get_new_above(&mdev->entity_internal_idx, 1,
 				&entity->internal_idx);
 	if (ret < 0) {
-		spin_unlock(&mdev->lock);
+		mutex_unlock(&mdev->graph_mutex);
 		return ret;
 	}
 
@@ -615,9 +615,6 @@  int __must_check media_device_register_entity(struct media_device *mdev,
 		(notify)->notify(entity, notify->notify_data);
 	}
 
-	spin_unlock(&mdev->lock);
-
-	mutex_lock(&mdev->graph_mutex);
 	if (mdev->entity_internal_idx_max
 	    >= mdev->pm_count_walk.ent_enum.idx_max) {
 		struct media_entity_graph new = { .top = 0 };
@@ -680,9 +677,9 @@  void media_device_unregister_entity(struct media_entity *entity)
 	if (mdev == NULL)
 		return;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 	__media_device_unregister_entity(entity);
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister_entity);
 
@@ -703,7 +700,6 @@  void media_device_init(struct media_device *mdev)
 	INIT_LIST_HEAD(&mdev->pads);
 	INIT_LIST_HEAD(&mdev->links);
 	INIT_LIST_HEAD(&mdev->entity_notify);
-	spin_lock_init(&mdev->lock);
 	mutex_init(&mdev->graph_mutex);
 	ida_init(&mdev->entity_internal_idx);
 
@@ -752,9 +748,9 @@  EXPORT_SYMBOL_GPL(__media_device_register);
 int __must_check media_device_register_entity_notify(struct media_device *mdev,
 					struct media_entity_notify *nptr)
 {
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 	list_add_tail(&nptr->list, &mdev->entity_notify);
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
@@ -771,9 +767,9 @@  static void __media_device_unregister_entity_notify(struct media_device *mdev,
 void media_device_unregister_entity_notify(struct media_device *mdev,
 					struct media_entity_notify *nptr)
 {
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 	__media_device_unregister_entity_notify(mdev, nptr);
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
 
@@ -787,11 +783,11 @@  void media_device_unregister(struct media_device *mdev)
 	if (mdev == NULL)
 		return;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 
 	/* Check if mdev was ever registered at all */
 	if (!media_devnode_is_registered(&mdev->devnode)) {
-		spin_unlock(&mdev->lock);
+		mutex_unlock(&mdev->graph_mutex);
 		return;
 	}
 
@@ -811,7 +807,7 @@  void media_device_unregister(struct media_device *mdev)
 		kfree(intf);
 	}
 
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 
 	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
 	media_devnode_unregister(&mdev->devnode);
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index e95070b3a3d4..c53c1d5589a0 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -219,7 +219,7 @@  int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	entity->pads = pads;
 
 	if (mdev)
-		spin_lock(&mdev->lock);
+		mutex_lock(&mdev->graph_mutex);
 
 	for (i = 0; i < num_pads; i++) {
 		pads[i].entity = entity;
@@ -230,7 +230,7 @@  int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	}
 
 	if (mdev)
-		spin_unlock(&mdev->lock);
+		mutex_unlock(&mdev->graph_mutex);
 
 	return 0;
 }
@@ -747,9 +747,9 @@  void media_entity_remove_links(struct media_entity *entity)
 	if (mdev == NULL)
 		return;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 	__media_entity_remove_links(entity);
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_entity_remove_links);
 
@@ -951,9 +951,9 @@  void media_remove_intf_link(struct media_link *link)
 	if (mdev == NULL)
 		return;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 	__media_remove_intf_link(link);
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_remove_intf_link);
 
@@ -975,8 +975,8 @@  void media_remove_intf_links(struct media_interface *intf)
 	if (mdev == NULL)
 		return;
 
-	spin_lock(&mdev->lock);
+	mutex_lock(&mdev->graph_mutex);
 	__media_remove_intf_links(intf);
-	spin_unlock(&mdev->lock);
+	mutex_unlock(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_remove_intf_links);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index df74cfa7da4a..0c2de97181f3 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -25,7 +25,6 @@ 
 
 #include <linux/list.h>
 #include <linux/mutex.h>
-#include <linux/spinlock.h>
 
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
@@ -304,8 +303,7 @@  struct media_entity_notify {
  * @pads:	List of registered pads
  * @links:	List of registered links
  * @entity_notify: List of registered entity_notify callbacks
- * @lock:	Entities list lock
- * @graph_mutex: Entities graph operation lock
+ * @graph_mutex: Protects access to struct media_device data
  * @pm_count_walk: Graph walk for power state walk. Access serialised using
  *		   graph_mutex.
  *
@@ -371,8 +369,6 @@  struct media_device {
 	/* notify callback list invoked when a new entity is registered */
 	struct list_head entity_notify;
 
-	/* Protects the graph objects creation/removal */
-	spinlock_t lock;
 	/* Serializes graph operations. */
 	struct mutex graph_mutex;
 	struct media_entity_graph pm_count_walk;