diff mbox series

[v4,16/26] media: mc: Postpone graph object removal until free

Message ID 20240610100530.1107771-17-sakari.ailus@linux.intel.com (mailing list archive)
State New
Headers show
Series Media device lifetime management | expand

Commit Message

Sakari Ailus June 10, 2024, 10:05 a.m. UTC
The media device itself will be unregistered based on it being unbound and
driver's remove callback being called. The graph objects themselves may
still be in use; rely on the media device release callback to release
them.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/mc/mc-device.c | 59 ++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 33 deletions(-)

Comments

Hans Verkuil June 17, 2024, 9:44 a.m. UTC | #1
On 10/06/2024 12:05, Sakari Ailus wrote:
> The media device itself will be unregistered based on it being unbound and
> driver's remove callback being called. The graph objects themselves may
> still be in use; rely on the media device release callback to release
> them.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Also please update to <hverkuil-cisco@xs4all.nl>.

Thank you!

	Hans

> ---
>  drivers/media/mc/mc-device.c | 59 ++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index bbc233e726d2..f1a88edb7573 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -702,8 +702,33 @@ EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
>  
>  static void __media_device_release(struct media_device *mdev)
>  {
> +	struct media_entity *entity;
> +	struct media_entity *next;
> +	struct media_interface *intf, *tmp_intf;
> +	struct media_entity_notify *notify, *nextp;
> +
>  	dev_dbg(mdev->dev, "Media device released\n");
>  
> +	/* Remove all entities from the media device */
> +	list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
> +		__media_device_unregister_entity(entity);
> +
> +	/* Remove all entity_notify callbacks from the media device */
> +	list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
> +		__media_device_unregister_entity_notify(mdev, notify);
> +
> +	/* Remove all interfaces from the media device */
> +	list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
> +				 graph_obj.list) {
> +		/*
> +		 * Unlink the interface, but don't free it here; the
> +		 * module which created it is responsible for freeing
> +		 * it
> +		 */
> +		__media_remove_intf_links(intf);
> +		media_gobj_destroy(&intf->graph_obj);
> +	}
> +
>  	ida_destroy(&mdev->entity_internal_idx);
>  	mdev->entity_internal_idx_max = 0;
>  	media_graph_walk_cleanup(&mdev->pm_count_walk);
> @@ -787,43 +812,11 @@ EXPORT_SYMBOL_GPL(__media_device_register);
>  
>  void media_device_unregister(struct media_device *mdev)
>  {
> -	struct media_entity *entity;
> -	struct media_entity *next;
> -	struct media_interface *intf, *tmp_intf;
> -	struct media_entity_notify *notify, *nextp;
> -
>  	if (mdev == NULL)
>  		return;
>  
> -	mutex_lock(&mdev->graph_mutex);
> -
> -	/* Check if mdev was ever registered at all */
> -	if (!media_devnode_is_registered(&mdev->devnode)) {
> -		mutex_unlock(&mdev->graph_mutex);
> +	if (!media_devnode_is_registered(&mdev->devnode))
>  		return;
> -	}
> -
> -	/* Remove all entities from the media device */
> -	list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
> -		__media_device_unregister_entity(entity);
> -
> -	/* Remove all entity_notify callbacks from the media device */
> -	list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
> -		__media_device_unregister_entity_notify(mdev, notify);
> -
> -	/* Remove all interfaces from the media device */
> -	list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
> -				 graph_obj.list) {
> -		/*
> -		 * Unlink the interface, but don't free it here; the
> -		 * module which created it is responsible for freeing
> -		 * it
> -		 */
> -		__media_remove_intf_links(intf);
> -		media_gobj_destroy(&intf->graph_obj);
> -	}
> -
> -	mutex_unlock(&mdev->graph_mutex);
>  
>  	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
>  	dev_dbg(mdev->dev, "Media device unregistering\n");
diff mbox series

Patch

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index bbc233e726d2..f1a88edb7573 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -702,8 +702,33 @@  EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
 
 static void __media_device_release(struct media_device *mdev)
 {
+	struct media_entity *entity;
+	struct media_entity *next;
+	struct media_interface *intf, *tmp_intf;
+	struct media_entity_notify *notify, *nextp;
+
 	dev_dbg(mdev->dev, "Media device released\n");
 
+	/* Remove all entities from the media device */
+	list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
+		__media_device_unregister_entity(entity);
+
+	/* Remove all entity_notify callbacks from the media device */
+	list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
+		__media_device_unregister_entity_notify(mdev, notify);
+
+	/* Remove all interfaces from the media device */
+	list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
+				 graph_obj.list) {
+		/*
+		 * Unlink the interface, but don't free it here; the
+		 * module which created it is responsible for freeing
+		 * it
+		 */
+		__media_remove_intf_links(intf);
+		media_gobj_destroy(&intf->graph_obj);
+	}
+
 	ida_destroy(&mdev->entity_internal_idx);
 	mdev->entity_internal_idx_max = 0;
 	media_graph_walk_cleanup(&mdev->pm_count_walk);
@@ -787,43 +812,11 @@  EXPORT_SYMBOL_GPL(__media_device_register);
 
 void media_device_unregister(struct media_device *mdev)
 {
-	struct media_entity *entity;
-	struct media_entity *next;
-	struct media_interface *intf, *tmp_intf;
-	struct media_entity_notify *notify, *nextp;
-
 	if (mdev == NULL)
 		return;
 
-	mutex_lock(&mdev->graph_mutex);
-
-	/* Check if mdev was ever registered at all */
-	if (!media_devnode_is_registered(&mdev->devnode)) {
-		mutex_unlock(&mdev->graph_mutex);
+	if (!media_devnode_is_registered(&mdev->devnode))
 		return;
-	}
-
-	/* Remove all entities from the media device */
-	list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
-		__media_device_unregister_entity(entity);
-
-	/* Remove all entity_notify callbacks from the media device */
-	list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
-		__media_device_unregister_entity_notify(mdev, notify);
-
-	/* Remove all interfaces from the media device */
-	list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
-				 graph_obj.list) {
-		/*
-		 * Unlink the interface, but don't free it here; the
-		 * module which created it is responsible for freeing
-		 * it
-		 */
-		__media_remove_intf_links(intf);
-		media_gobj_destroy(&intf->graph_obj);
-	}
-
-	mutex_unlock(&mdev->graph_mutex);
 
 	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
 	dev_dbg(mdev->dev, "Media device unregistering\n");