* [PATCH 1/2] [media] media-device: move media entity register/unregister functions
@ 2015-12-15 10:43 Mauro Carvalho Chehab
2015-12-15 10:43 ` [PATCH 2/2] [media] media-device: better lock media_device_unregister() Mauro Carvalho Chehab
2015-12-21 12:35 ` [PATCH 1/2] [media] media-device: move media entity register/unregister functions Javier Martinez Canillas
0 siblings, 2 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2015-12-15 10:43 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Shuah Khan, Javier Martinez Canillas
media entity register and unregister functions are called by media
device register/unregister. Move them to occur earlier, as we'll need
an unlocked version of media_device_entity_unregister() and we don't
want to add a function prototype without needing it.
No functional changes.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
drivers/media/media-device.c | 160 +++++++++++++++++++++----------------------
1 file changed, 80 insertions(+), 80 deletions(-)
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index da4126863ecc..1222fa642ad8 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -529,6 +529,86 @@ static void media_device_release(struct media_devnode *mdev)
}
/**
+ * media_device_register_entity - Register an entity with a media device
+ * @mdev: The media device
+ * @entity: The entity
+ */
+int __must_check media_device_register_entity(struct media_device *mdev,
+ struct media_entity *entity)
+{
+ unsigned int i;
+
+ if (entity->function == MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN ||
+ entity->function == MEDIA_ENT_F_UNKNOWN)
+ dev_warn(mdev->dev,
+ "Entity type for entity %s was not initialized!\n",
+ entity->name);
+
+ /* Warn if we apparently re-register an entity */
+ WARN_ON(entity->graph_obj.mdev != NULL);
+ entity->graph_obj.mdev = mdev;
+ INIT_LIST_HEAD(&entity->links);
+ entity->num_links = 0;
+ entity->num_backlinks = 0;
+
+ spin_lock(&mdev->lock);
+ /* Initialize media_gobj embedded at the entity */
+ media_gobj_create(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
+
+ /* Initialize objects at the pads */
+ for (i = 0; i < entity->num_pads; i++)
+ media_gobj_create(mdev, MEDIA_GRAPH_PAD,
+ &entity->pads[i].graph_obj);
+
+ spin_unlock(&mdev->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(media_device_register_entity);
+
+/**
+ * media_device_unregister_entity - Unregister an entity
+ * @entity: The entity
+ *
+ * If the entity has never been registered this function will return
+ * immediately.
+ */
+void media_device_unregister_entity(struct media_entity *entity)
+{
+ struct media_device *mdev = entity->graph_obj.mdev;
+ struct media_link *link, *tmp;
+ struct media_interface *intf;
+ unsigned int i;
+
+ if (mdev == NULL)
+ return;
+
+ spin_lock(&mdev->lock);
+
+ /* Remove all interface links pointing to this entity */
+ list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
+ list_for_each_entry_safe(link, tmp, &intf->links, list) {
+ if (link->entity == entity)
+ __media_remove_intf_link(link);
+ }
+ }
+
+ /* Remove all data links that belong to this entity */
+ __media_entity_remove_links(entity);
+
+ /* Remove all pads that belong to this entity */
+ for (i = 0; i < entity->num_pads; i++)
+ media_gobj_destroy(&entity->pads[i].graph_obj);
+
+ /* Remove the entity */
+ media_gobj_destroy(&entity->graph_obj);
+
+ spin_unlock(&mdev->lock);
+ entity->graph_obj.mdev = NULL;
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_entity);
+
+/**
* media_device_register - register a media device
* @mdev: The media device
*
@@ -611,86 +691,6 @@ void media_device_unregister(struct media_device *mdev)
}
EXPORT_SYMBOL_GPL(media_device_unregister);
-/**
- * media_device_register_entity - Register an entity with a media device
- * @mdev: The media device
- * @entity: The entity
- */
-int __must_check media_device_register_entity(struct media_device *mdev,
- struct media_entity *entity)
-{
- unsigned int i;
-
- if (entity->function == MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN ||
- entity->function == MEDIA_ENT_F_UNKNOWN)
- dev_warn(mdev->dev,
- "Entity type for entity %s was not initialized!\n",
- entity->name);
-
- /* Warn if we apparently re-register an entity */
- WARN_ON(entity->graph_obj.mdev != NULL);
- entity->graph_obj.mdev = mdev;
- INIT_LIST_HEAD(&entity->links);
- entity->num_links = 0;
- entity->num_backlinks = 0;
-
- spin_lock(&mdev->lock);
- /* Initialize media_gobj embedded at the entity */
- media_gobj_create(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
-
- /* Initialize objects at the pads */
- for (i = 0; i < entity->num_pads; i++)
- media_gobj_create(mdev, MEDIA_GRAPH_PAD,
- &entity->pads[i].graph_obj);
-
- spin_unlock(&mdev->lock);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(media_device_register_entity);
-
-/**
- * media_device_unregister_entity - Unregister an entity
- * @entity: The entity
- *
- * If the entity has never been registered this function will return
- * immediately.
- */
-void media_device_unregister_entity(struct media_entity *entity)
-{
- struct media_device *mdev = entity->graph_obj.mdev;
- struct media_link *link, *tmp;
- struct media_interface *intf;
- unsigned int i;
-
- if (mdev == NULL)
- return;
-
- spin_lock(&mdev->lock);
-
- /* Remove all interface links pointing to this entity */
- list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
- list_for_each_entry_safe(link, tmp, &intf->links, list) {
- if (link->entity == entity)
- __media_remove_intf_link(link);
- }
- }
-
- /* Remove all data links that belong to this entity */
- __media_entity_remove_links(entity);
-
- /* Remove all pads that belong to this entity */
- for (i = 0; i < entity->num_pads; i++)
- media_gobj_destroy(&entity->pads[i].graph_obj);
-
- /* Remove the entity */
- media_gobj_destroy(&entity->graph_obj);
-
- spin_unlock(&mdev->lock);
- entity->graph_obj.mdev = NULL;
-}
-EXPORT_SYMBOL_GPL(media_device_unregister_entity);
-
static void media_device_release_devres(struct device *dev, void *res)
{
}
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] [media] media-device: better lock media_device_unregister()
2015-12-15 10:43 [PATCH 1/2] [media] media-device: move media entity register/unregister functions Mauro Carvalho Chehab
@ 2015-12-15 10:43 ` Mauro Carvalho Chehab
2015-12-21 12:38 ` Javier Martinez Canillas
2015-12-21 12:35 ` [PATCH 1/2] [media] media-device: move media entity register/unregister functions Javier Martinez Canillas
1 sibling, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2015-12-15 10:43 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Shuah Khan, Javier Martinez Canillas
If media_device_unregister() is called by two different
drivers, a race condition may happen, as the check if the
device is not registered is not protected.
Move the spin_lock() to happen earlier in the function, in order
to prevent such race condition.
Reported-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
drivers/media/media-device.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 1222fa642ad8..189c2ba8c3d3 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -573,18 +573,13 @@ EXPORT_SYMBOL_GPL(media_device_register_entity);
* If the entity has never been registered this function will return
* immediately.
*/
-void media_device_unregister_entity(struct media_entity *entity)
+static void __media_device_unregister_entity(struct media_entity *entity)
{
struct media_device *mdev = entity->graph_obj.mdev;
struct media_link *link, *tmp;
struct media_interface *intf;
unsigned int i;
- if (mdev == NULL)
- return;
-
- spin_lock(&mdev->lock);
-
/* Remove all interface links pointing to this entity */
list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
list_for_each_entry_safe(link, tmp, &intf->links, list) {
@@ -603,11 +598,23 @@ void media_device_unregister_entity(struct media_entity *entity)
/* Remove the entity */
media_gobj_destroy(&entity->graph_obj);
- spin_unlock(&mdev->lock);
entity->graph_obj.mdev = NULL;
}
+
+void media_device_unregister_entity(struct media_entity *entity)
+{
+ struct media_device *mdev = entity->graph_obj.mdev;
+
+ if (mdev == NULL)
+ return;
+
+ spin_lock(&mdev->lock);
+ __media_device_unregister_entity(entity);
+ spin_unlock(&mdev->lock);
+}
EXPORT_SYMBOL_GPL(media_device_unregister_entity);
+
/**
* media_device_register - register a media device
* @mdev: The media device
@@ -666,22 +673,29 @@ void media_device_unregister(struct media_device *mdev)
struct media_entity *next;
struct media_interface *intf, *tmp_intf;
+ if (mdev == NULL)
+ return;
+
+ spin_lock(&mdev->lock);
+
/* Check if mdev was ever registered at all */
- if (!media_devnode_is_registered(&mdev->devnode))
+ if (!media_devnode_is_registered(&mdev->devnode)) {
+ spin_unlock(&mdev->lock);
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);
+ __media_device_unregister_entity(entity);
/* Remove all interfaces from the media device */
- spin_lock(&mdev->lock);
list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
graph_obj.list) {
__media_remove_intf_links(intf);
media_gobj_destroy(&intf->graph_obj);
kfree(intf);
}
+
spin_unlock(&mdev->lock);
device_remove_file(&mdev->devnode.dev, &dev_attr_model);
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] [media] media-device: move media entity register/unregister functions
2015-12-15 10:43 [PATCH 1/2] [media] media-device: move media entity register/unregister functions Mauro Carvalho Chehab
2015-12-15 10:43 ` [PATCH 2/2] [media] media-device: better lock media_device_unregister() Mauro Carvalho Chehab
@ 2015-12-21 12:35 ` Javier Martinez Canillas
1 sibling, 0 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2015-12-21 12:35 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Shuah Khan
Hello Mauro,
On 12/15/2015 07:43 AM, Mauro Carvalho Chehab wrote:
> media entity register and unregister functions are called by media
> device register/unregister. Move them to occur earlier, as we'll need
> an unlocked version of media_device_entity_unregister() and we don't
> want to add a function prototype without needing it.
>
> No functional changes.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Patch looks good to me.
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] [media] media-device: better lock media_device_unregister()
2015-12-15 10:43 ` [PATCH 2/2] [media] media-device: better lock media_device_unregister() Mauro Carvalho Chehab
@ 2015-12-21 12:38 ` Javier Martinez Canillas
0 siblings, 0 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2015-12-21 12:38 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Shuah Khan
Hello Mauro,
On 12/15/2015 07:43 AM, Mauro Carvalho Chehab wrote:
> If media_device_unregister() is called by two different
> drivers, a race condition may happen, as the check if the
> device is not registered is not protected.
>
> Move the spin_lock() to happen earlier in the function, in order
> to prevent such race condition.
>
> Reported-by: Shuah Khan <shuahkh@osg.samsung.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
The patch looks good and I also tested it on an OMAP3 IGEPv2 board:
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-21 12:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15 10:43 [PATCH 1/2] [media] media-device: move media entity register/unregister functions Mauro Carvalho Chehab
2015-12-15 10:43 ` [PATCH 2/2] [media] media-device: better lock media_device_unregister() Mauro Carvalho Chehab
2015-12-21 12:38 ` Javier Martinez Canillas
2015-12-21 12:35 ` [PATCH 1/2] [media] media-device: move media entity register/unregister functions Javier Martinez Canillas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).