[v4,16/26] media: mc: Postpone graph object removal until free
Commit Message
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
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");
@@ -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");