[alsa-devel] [PATCH v2] [media] media-device: use kref for media_device instance

Mauro Carvalho Chehab mchehab at osg.samsung.com
Sat Mar 19 01:42:16 CET 2016


Now that the media_device can be used by multiple drivers,
via devres, we need to be sure that it will be dropped only
when all drivers stop using it.

Signed-off-by: Mauro Carvalho Chehab <mchehab at osg.samsung.com>
---

v2: The kref is now used only when media_device is allocated via 
    the media_device*_devress. This warrants that other drivers won't be
    affected, and that we can keep media_device_cleanup() balanced with
    media_device_init().

 drivers/media/media-device.c           | 117 +++++++++++++++++++++++++--------
 drivers/media/usb/au0828/au0828-core.c |   3 +-
 include/media/media-device.h           |  28 ++++++++
 sound/usb/media.c                      |   3 +-
 4 files changed, 118 insertions(+), 33 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index c32fa15cc76e..4a97d92a7e7d 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -707,11 +707,16 @@ void media_device_init(struct media_device *mdev)
 }
 EXPORT_SYMBOL_GPL(media_device_init);
 
-void media_device_cleanup(struct media_device *mdev)
+static void __media_device_cleanup(struct media_device *mdev)
 {
 	ida_destroy(&mdev->entity_internal_idx);
 	mdev->entity_internal_idx_max = 0;
 	media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
+}
+
+void media_device_cleanup(struct media_device *mdev)
+{
+	__media_device_cleanup(mdev);
 	mutex_destroy(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_device_cleanup);
@@ -721,6 +726,9 @@ int __must_check __media_device_register(struct media_device *mdev,
 {
 	int ret;
 
+	/* Check if mdev was ever registered at all */
+	mutex_lock(&mdev->graph_mutex);
+
 	/* Register the device node. */
 	mdev->devnode.fops = &media_device_fops;
 	mdev->devnode.parent = mdev->dev;
@@ -731,17 +739,19 @@ int __must_check __media_device_register(struct media_device *mdev,
 
 	ret = media_devnode_register(&mdev->devnode, owner);
 	if (ret < 0)
-		return ret;
+		goto err;
 
 	ret = device_create_file(&mdev->devnode.dev, &dev_attr_model);
 	if (ret < 0) {
 		media_devnode_unregister(&mdev->devnode);
-		return ret;
+		goto err;
 	}
 
 	dev_dbg(mdev->dev, "Media device registered\n");
 
-	return 0;
+err:
+	mutex_unlock(&mdev->graph_mutex);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(__media_device_register);
 
@@ -773,24 +783,13 @@ void media_device_unregister_entity_notify(struct media_device *mdev,
 }
 EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
 
-void media_device_unregister(struct media_device *mdev)
+static 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);
-		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);
@@ -807,38 +806,98 @@ void media_device_unregister(struct media_device *mdev)
 		kfree(intf);
 	}
 
-	mutex_unlock(&mdev->graph_mutex);
-
-	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
-	media_devnode_unregister(&mdev->devnode);
+	/* Check if mdev devnode was registered */
+	if (media_devnode_is_registered(&mdev->devnode)) {
+		device_remove_file(&mdev->devnode.dev, &dev_attr_model);
+		media_devnode_unregister(&mdev->devnode);
+	}
 
 	dev_dbg(mdev->dev, "Media device unregistered\n");
 }
+
+void media_device_unregister(struct media_device *mdev)
+{
+	if (mdev == NULL)
+		return;
+
+	mutex_lock(&mdev->graph_mutex);
+	__media_device_unregister(mdev);
+	mutex_unlock(&mdev->graph_mutex);
+}
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
 static void media_device_release_devres(struct device *dev, void *res)
 {
 }
 
-struct media_device *media_device_get_devres(struct device *dev)
+static void do_media_device_unregister_devres(struct kref *kref)
 {
+	struct media_device_devres *mdev_devres;
 	struct media_device *mdev;
+	int ret;
 
-	mdev = devres_find(dev, media_device_release_devres, NULL, NULL);
-	if (mdev)
-		return mdev;
+	mdev_devres = container_of(kref, struct media_device_devres, kref);
 
-	mdev = devres_alloc(media_device_release_devres,
-				sizeof(struct media_device), GFP_KERNEL);
-	if (!mdev)
+	if (!mdev_devres)
+		return;
+
+	mdev = &mdev_devres->mdev;
+
+	mutex_lock(&mdev->graph_mutex);
+	__media_device_unregister(mdev);
+	__media_device_cleanup(mdev);
+	mutex_unlock(&mdev->graph_mutex);
+	mutex_destroy(&mdev->graph_mutex);
+
+	ret = devres_destroy(mdev->dev, media_device_release_devres,
+			     NULL, NULL);
+	pr_debug("%s: devres_destroy() returned %d\n", __func__, ret);
+}
+
+void media_device_unregister_devres(struct media_device *mdev)
+{
+	struct media_device_devres *mdev_devres;
+
+	mdev_devres = container_of(mdev, struct media_device_devres, mdev);
+	kref_put(&mdev_devres->kref, do_media_device_unregister_devres);
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_devres);
+
+struct media_device *media_device_get_devres(struct device *dev)
+{
+	struct media_device_devres *mdev_devres, *ptr;
+
+	mdev_devres = devres_find(dev, media_device_release_devres, NULL, NULL);
+	if (mdev_devres) {
+		kref_get(&mdev_devres->kref);
+		return &mdev_devres->mdev;
+	}
+
+	mdev_devres = devres_alloc(media_device_release_devres,
+				   sizeof(struct media_device_devres),
+				   GFP_KERNEL);
+	if (!mdev_devres)
 		return NULL;
-	return devres_get(dev, mdev, NULL, NULL);
+
+	ptr = devres_get(dev, mdev_devres, NULL, NULL);
+	if (ptr)
+		kref_init(&ptr->kref);
+	else
+		devres_free(mdev_devres);
+
+	return &ptr->mdev;
 }
 EXPORT_SYMBOL_GPL(media_device_get_devres);
 
 struct media_device *media_device_find_devres(struct device *dev)
 {
-	return devres_find(dev, media_device_release_devres, NULL, NULL);
+	struct media_device_devres *mdev_devres;
+
+	mdev_devres = devres_find(dev, media_device_release_devres, NULL, NULL);
+	if (!mdev_devres)
+		return NULL;
+
+	return &mdev_devres->mdev;
 }
 EXPORT_SYMBOL_GPL(media_device_find_devres);
 
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index 06da73f1ff22..060904ed8f20 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -157,8 +157,7 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
 	dev->media_dev->enable_source = NULL;
 	dev->media_dev->disable_source = NULL;
 
-	media_device_unregister(dev->media_dev);
-	media_device_cleanup(dev->media_dev);
+	media_device_unregister_devres(dev->media_dev);
 	dev->media_dev = NULL;
 #endif
 }
diff --git a/include/media/media-device.h b/include/media/media-device.h
index b21ef244ad3e..e59772ed8494 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -23,6 +23,7 @@
 #ifndef _MEDIA_DEVICE_H
 #define _MEDIA_DEVICE_H
 
+#include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 
@@ -382,6 +383,16 @@ struct media_device {
 			   unsigned int notification);
 };
 
+/**
+ * struct media_device_devres - Media device device resource
+ * @mdev:	pointer to struct media_device
+ * @kref:	Object refcount
+ */
+struct media_device_devres {
+	struct media_device mdev;
+	struct kref kref;
+};
+
 /* We don't need to include pci.h or usb.h here */
 struct pci_dev;
 struct usb_device;
@@ -604,6 +615,19 @@ struct media_device *media_device_get_devres(struct device *dev);
  */
 struct media_device *media_device_find_devres(struct device *dev);
 
+/**
+ * media_device_unregister_devres) - Unregister media device allocated as
+ *				     as device resource
+ *
+ * @dev: pointer to struct &device.
+ *
+ * Devices allocated via media_device_get_devres should be de-alocalted
+ * and freed via this function. Callers should not call
+ * media_device_unregister() nor media_device_cleanup() on devices
+ * allocated via media_device_get_devres().
+ */
+void media_device_unregister_devres(struct media_device *mdev);
+
 /* Iterate over all entities. */
 #define media_device_for_each_entity(entity, mdev)			\
 	list_for_each_entry(entity, &(mdev)->entities, graph_obj.list)
@@ -688,6 +712,10 @@ static inline struct media_device *media_device_find_devres(struct device *dev)
 	return NULL;
 }
 
+static inline void media_device_unregister_devres(struct media_device *mdev)
+{
+}
+
 static inline void media_device_pci_init(struct media_device *mdev,
 					 struct pci_dev *pci_dev,
 					 char *name)
diff --git a/sound/usb/media.c b/sound/usb/media.c
index 93a50d01490c..f78955fd0d6e 100644
--- a/sound/usb/media.c
+++ b/sound/usb/media.c
@@ -311,8 +311,7 @@ void media_snd_device_delete(struct snd_usb_audio *chip)
 	media_snd_mixer_delete(chip);
 
 	if (mdev) {
-		if (media_devnode_is_registered(&mdev->devnode))
-			media_device_unregister(mdev);
+		media_device_unregister_devres(mdev);
 		chip->media_dev = NULL;
 	}
 }
-- 
2.5.0




More information about the Alsa-devel mailing list