All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: laurent.pinchart@ideasonboard.com, hverkuil@xs4all.nl
Subject: [PATCH v4 05/26] Revert "[media] media-device: dynamically allocate struct media_devnode"
Date: Mon, 10 Jun 2024 13:05:09 +0300	[thread overview]
Message-ID: <20240610100530.1107771-6-sakari.ailus@linux.intel.com> (raw)
In-Reply-To: <20240610100530.1107771-1-sakari.ailus@linux.intel.com>

This reverts commit a087ce704b80 ("[media] media-device: dynamically
allocate struct media_devnode"). The commit was part of an original
patchset to avoid crashes when an unregistering device is in use.

This revert is performed to roll back to a state which is more suitable
for the objective: making media device refcountable.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/mc/mc-device.c                  | 44 ++++++-------------
 drivers/media/mc/mc-devnode.c                 |  7 +--
 .../vcodec/decoder/mtk_vcodec_dec_drv.c       |  2 +-
 drivers/media/test-drivers/visl/visl-core.c   |  2 +-
 drivers/media/usb/au0828/au0828-core.c        |  4 +-
 drivers/media/usb/uvc/uvc_driver.c            |  2 +-
 drivers/staging/media/sunxi/cedrus/cedrus.c   |  2 +-
 include/media/media-device.h                  |  5 ++-
 include/media/media-devnode.h                 | 15 +------
 sound/usb/media.c                             |  8 ++--
 10 files changed, 31 insertions(+), 60 deletions(-)

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index 4772a7f55112..d4553a3705f5 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -435,7 +435,7 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 			       unsigned long __arg)
 {
 	struct media_devnode *devnode = media_devnode_data(filp);
-	struct media_device *dev = devnode->media_dev;
+	struct media_device *dev = to_media_device(devnode);
 	const struct media_ioctl_info *info;
 	void __user *arg = (void __user *)__arg;
 	char __karg[256], *karg = __karg;
@@ -519,7 +519,7 @@ static long media_device_compat_ioctl(struct file *filp, unsigned int cmd,
 				      unsigned long arg)
 {
 	struct media_devnode *devnode = media_devnode_data(filp);
-	struct media_device *dev = devnode->media_dev;
+	struct media_device *dev = to_media_device(devnode);
 	long ret;
 
 	switch (cmd) {
@@ -555,8 +555,7 @@ static const struct media_file_operations media_device_fops = {
 static ssize_t model_show(struct device *cd,
 			  struct device_attribute *attr, char *buf)
 {
-	struct media_devnode *devnode = to_media_devnode(cd);
-	struct media_device *mdev = devnode->media_dev;
+	struct media_device *mdev = to_media_device(to_media_devnode(cd));
 
 	return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
 }
@@ -714,34 +713,23 @@ EXPORT_SYMBOL_GPL(media_device_cleanup);
 int __must_check __media_device_register(struct media_device *mdev,
 					 struct module *owner)
 {
-	struct media_devnode *devnode;
 	int ret;
 
-	devnode = kzalloc(sizeof(*devnode), GFP_KERNEL);
-	if (!devnode)
-		return -ENOMEM;
-
 	/* Register the device node. */
-	mdev->devnode = devnode;
-	devnode->fops = &media_device_fops;
-	devnode->parent = mdev->dev;
-	devnode->release = media_device_release;
+	mdev->devnode.fops = &media_device_fops;
+	mdev->devnode.parent = mdev->dev;
+	mdev->devnode.release = media_device_release;
 
 	/* Set version 0 to indicate user-space that the graph is static */
 	mdev->topology_version = 0;
 
-	ret = media_devnode_register(mdev, devnode, owner);
-	if (ret < 0) {
-		mdev->devnode = NULL;
-		kfree(devnode);
+	ret = media_devnode_register(&mdev->devnode, owner);
+	if (ret < 0)
 		return ret;
-	}
 
-	ret = device_create_file(&devnode->dev, &dev_attr_model);
+	ret = device_create_file(&mdev->devnode.dev, &dev_attr_model);
 	if (ret < 0) {
-		mdev->devnode = NULL;
-		media_devnode_unregister(devnode);
-		kfree(devnode);
+		media_devnode_unregister(&mdev->devnode);
 		return ret;
 	}
 
@@ -791,7 +779,7 @@ void media_device_unregister(struct media_device *mdev)
 	mutex_lock(&mdev->graph_mutex);
 
 	/* Check if mdev was ever registered at all */
-	if (!media_devnode_is_registered(mdev->devnode)) {
+	if (!media_devnode_is_registered(&mdev->devnode)) {
 		mutex_unlock(&mdev->graph_mutex);
 		return;
 	}
@@ -818,13 +806,9 @@ void media_device_unregister(struct media_device *mdev)
 
 	mutex_unlock(&mdev->graph_mutex);
 
-	dev_dbg(mdev->dev, "Media device unregistered\n");
-
-	/* 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);
-	}
+	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
+	dev_dbg(mdev->dev, "Media device unregistering\n");
+	media_devnode_unregister(&mdev->devnode);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
index 64bbff0f90cd..2e33c2007f08 100644
--- a/drivers/media/mc/mc-devnode.c
+++ b/drivers/media/mc/mc-devnode.c
@@ -32,7 +32,6 @@
 #include <linux/uaccess.h>
 
 #include <media/media-devnode.h>
-#include <media/media-device.h>
 
 #define MEDIA_NUM_DEVICES	256
 #define MEDIA_NAME		"media"
@@ -63,8 +62,6 @@ static void media_devnode_release(struct device *cd)
 	/* Release media_devnode and perform other cleanups as needed. */
 	if (devnode->release)
 		devnode->release(devnode);
-
-	kfree(devnode);
 }
 
 static const struct bus_type media_bus_type = {
@@ -210,8 +207,7 @@ static const struct file_operations media_devnode_fops = {
 	.llseek = no_llseek,
 };
 
-int __must_check media_devnode_register(struct media_device *mdev,
-					struct media_devnode *devnode,
+int __must_check media_devnode_register(struct media_devnode *devnode,
 					struct module *owner)
 {
 	int minor;
@@ -230,7 +226,6 @@ int __must_check media_devnode_register(struct media_device *mdev,
 	mutex_unlock(&media_devnode_lock);
 
 	devnode->minor = minor;
-	devnode->media_dev = mdev;
 
 	/* Part 2: Initialize the media and character devices */
 	cdev_init(&devnode->cdev, &media_devnode_fops);
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index 2073781ccadb..0fd681b4571d 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -570,7 +570,7 @@ static void mtk_vcodec_dec_remove(struct platform_device *pdev)
 
 	destroy_workqueue(dev->decode_workqueue);
 
-	if (media_devnode_is_registered(dev->mdev_dec.devnode)) {
+	if (media_devnode_is_registered(&dev->mdev_dec.devnode)) {
 		media_device_unregister(&dev->mdev_dec);
 		v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec);
 		media_device_cleanup(&dev->mdev_dec);
diff --git a/drivers/media/test-drivers/visl/visl-core.c b/drivers/media/test-drivers/visl/visl-core.c
index c46464bcaf2e..151d4a09559c 100644
--- a/drivers/media/test-drivers/visl/visl-core.c
+++ b/drivers/media/test-drivers/visl/visl-core.c
@@ -513,7 +513,7 @@ static void visl_remove(struct platform_device *pdev)
 	v4l2_info(&dev->v4l2_dev, "Removing " VISL_NAME);
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (media_devnode_is_registered(dev->mdev.devnode)) {
+	if (media_devnode_is_registered(&dev->mdev.devnode)) {
 		media_device_unregister(&dev->mdev);
 		v4l2_m2m_unregister_media_controller(dev->m2m_dev);
 	}
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index 1e246b47766d..4101e12cda5a 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -128,7 +128,7 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
 	struct media_device *mdev = dev->media_dev;
 	struct media_entity_notify *notify, *nextp;
 
-	if (!mdev || !media_devnode_is_registered(mdev->devnode))
+	if (!mdev || !media_devnode_is_registered(&mdev->devnode))
 		return;
 
 	/* Remove au0828 entity_notify callbacks */
@@ -566,7 +566,7 @@ static int au0828_media_device_register(struct au0828_dev *dev,
 	if (!dev->media_dev)
 		return 0;
 
-	if (!media_devnode_is_registered(dev->media_dev->devnode)) {
+	if (!media_devnode_is_registered(&dev->media_dev->devnode)) {
 
 		/* register media device */
 		ret = media_device_register(dev->media_dev);
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 8fe24c98087e..436c1d361906 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1923,7 +1923,7 @@ static void uvc_unregister_video(struct uvc_device *dev)
 	if (dev->vdev.dev)
 		v4l2_device_unregister(&dev->vdev);
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (media_devnode_is_registered(dev->mdev.devnode))
+	if (media_devnode_is_registered(&dev->mdev.devnode))
 		media_device_unregister(&dev->mdev);
 #endif
 }
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index f52df6836045..f8f678835a4c 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -548,7 +548,7 @@ static void cedrus_remove(struct platform_device *pdev)
 	struct cedrus_dev *dev = platform_get_drvdata(pdev);
 
 	cancel_delayed_work_sync(&dev->watchdog_work);
-	if (media_devnode_is_registered(dev->mdev.devnode)) {
+	if (media_devnode_is_registered(&dev->mdev.devnode)) {
 		media_device_unregister(&dev->mdev);
 		v4l2_m2m_unregister_media_controller(dev->m2m_dev);
 		media_device_cleanup(&dev->mdev);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 53d2a16a70b0..c791f3d5ad77 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -145,7 +145,7 @@ struct media_device_ops {
 struct media_device {
 	/* dev->driver_data points to this struct. */
 	struct device *dev;
-	struct media_devnode *devnode;
+	struct media_devnode devnode;
 
 	char model[32];
 	char driver_name[32];
@@ -191,6 +191,9 @@ struct usb_device;
 #define MEDIA_DEV_NOTIFY_PRE_LINK_CH	0
 #define MEDIA_DEV_NOTIFY_POST_LINK_CH	1
 
+/* media_devnode to media_device */
+#define to_media_device(node) container_of(node, struct media_device, devnode)
+
 /**
  * media_device_init() - Initializes a media device element
  *
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index 46f0d3ae44d1..1117d1dfd6bf 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -21,8 +21,6 @@
 #include <linux/device.h>
 #include <linux/cdev.h>
 
-struct media_device;
-
 /*
  * Flag to mark the media_devnode struct as registered. Drivers must not touch
  * this flag directly, it will be set and cleared by media_devnode_register and
@@ -73,8 +71,6 @@ struct media_file_operations {
  * before registering the node.
  */
 struct media_devnode {
-	struct media_device *media_dev;
-
 	/* device ops */
 	const struct media_file_operations *fops;
 
@@ -97,8 +93,7 @@ struct media_devnode {
 /**
  * media_devnode_register - register a media device node
  *
- * @mdev: struct media_device we want to register a device node
- * @devnode: media device node structure we want to register
+ * @devnode: struct media_devnode we want to register a device node
  * @owner: should be filled with %THIS_MODULE
  *
  * The registration code assigns minor numbers and registers the new device node
@@ -111,8 +106,7 @@ struct media_devnode {
  * the media_devnode structure is *not* called, so the caller is responsible for
  * freeing any data.
  */
-int __must_check media_devnode_register(struct media_device *mdev,
-					struct media_devnode *devnode,
+int __must_check media_devnode_register(struct media_devnode *devnode,
 					struct module *owner);
 
 /**
@@ -142,14 +136,9 @@ static inline struct media_devnode *media_devnode_data(struct file *filp)
  *	false otherwise.
  *
  * @devnode: pointer to struct &media_devnode.
- *
- * Note: If mdev is NULL, it also returns false.
  */
 static inline int media_devnode_is_registered(struct media_devnode *devnode)
 {
-	if (!devnode)
-		return false;
-
 	return test_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
 }
 
diff --git a/sound/usb/media.c b/sound/usb/media.c
index d48db6f3ae65..f97e937378ee 100644
--- a/sound/usb/media.c
+++ b/sound/usb/media.c
@@ -122,7 +122,7 @@ void snd_media_stream_delete(struct snd_usb_substream *subs)
 		struct media_device *mdev;
 
 		mdev = mctl->media_dev;
-		if (mdev && media_devnode_is_registered(mdev->devnode)) {
+		if (mdev && media_devnode_is_registered(&mdev->devnode)) {
 			media_devnode_remove(mctl->intf_devnode);
 			media_device_unregister_entity(&mctl->media_entity);
 			media_entity_cleanup(&mctl->media_entity);
@@ -239,14 +239,14 @@ static void snd_media_mixer_delete(struct snd_usb_audio *chip)
 		if (!mixer->media_mixer_ctl)
 			continue;
 
-		if (media_devnode_is_registered(mdev->devnode)) {
+		if (media_devnode_is_registered(&mdev->devnode)) {
 			media_device_unregister_entity(&mctl->media_entity);
 			media_entity_cleanup(&mctl->media_entity);
 		}
 		kfree(mctl);
 		mixer->media_mixer_ctl = NULL;
 	}
-	if (media_devnode_is_registered(mdev->devnode))
+	if (media_devnode_is_registered(&mdev->devnode))
 		media_devnode_remove(chip->ctl_intf_media_devnode);
 	chip->ctl_intf_media_devnode = NULL;
 }
@@ -284,7 +284,7 @@ int snd_media_device_create(struct snd_usb_audio *chip,
 			"Couldn't create media mixer entities. Error: %d\n",
 			ret);
 
-	if (!media_devnode_is_registered(mdev->devnode)) {
+	if (!media_devnode_is_registered(&mdev->devnode)) {
 		/* don't register if snd_media_mixer_init() failed */
 		if (ret)
 			goto create_fail;
-- 
2.39.2


  parent reply	other threads:[~2024-06-10 10:05 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-10 10:05 [PATCH v4 00/26] Media device lifetime management Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 01/26] Revert "[media] media: fix media devnode ioctl/syscall and unregister race" Sakari Ailus
2024-06-27  6:53   ` Hans Verkuil
2024-06-27  7:04     ` Sakari Ailus
2024-06-27  7:15       ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 02/26] Revert "media: utilize new cdev_device_add helper function" Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 03/26] Revert "[media] media: fix use-after-free in cdev_put() when app exits after driver unbind" Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 04/26] media: mc, cec: Make use of cdev_device_add() again Sakari Ailus
2024-06-10 10:05 ` Sakari Ailus [this message]
2024-06-10 10:05 ` [PATCH v4 06/26] media: mc: Drop nop release callback Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 07/26] media: mc: Drop media_dev description from struct media_devnode Sakari Ailus
2024-06-17  9:02   ` Hans Verkuil
2024-06-17 11:43     ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 08/26] media: mc: Do not call cdev_device_del() if cdev_device_add() fails Sakari Ailus
2024-06-17  9:13   ` Hans Verkuil
2024-06-17 12:15     ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 09/26] media: mc: Delete character device early Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 10/26] media: mc: Clear minor number reservation at unregistration time Sakari Ailus
2024-06-27  6:43   ` Hans Verkuil
2024-06-27  6:58     ` Sakari Ailus
2024-06-27  7:10       ` Sakari Ailus
2024-06-27  7:22         ` Hans Verkuil
2025-08-22  8:05   ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 11/26] media: mc: Split initialising and adding media devnode Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 12/26] media: mc: Shuffle functions around Sakari Ailus
2024-06-17  9:41   ` Hans Verkuil
2024-06-17 17:59     ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 13/26] media: mc: Initialise media devnode in media_device_init() Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 14/26] media: mc: Refcount the media device Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 15/26] media: v4l: Acquire a reference to the media device for every video device Sakari Ailus
2024-06-17  9:39   ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 16/26] media: mc: Postpone graph object removal until free Sakari Ailus
2024-06-17  9:44   ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 17/26] media: omap3isp: Release the isp device struct by media device callback Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 18/26] media: ipu3-cio2: Release the cio2 device context " Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 19/26] media: vimc: Release resources on media device release Sakari Ailus
2024-06-17  9:49   ` Hans Verkuil
2024-06-17 10:09     ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 20/26] media: Documentation: Document how Media device resources are released Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 21/26] media: mc: Add per-file-handle data support Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 22/26] media: mc: Maintain a list of open file handles in a media device Sakari Ailus
2024-06-17  9:57   ` Hans Verkuil
2024-06-17 17:46     ` Sakari Ailus
2024-06-18  5:35       ` Hans Verkuil
2024-06-18  6:27         ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 23/26] media: mc: Implement best effort media device removal safety sans refcount Sakari Ailus
2024-06-17 11:54   ` Hans Verkuil
2024-06-17 20:28     ` Sakari Ailus
2024-06-18 10:33       ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 24/26] media: mc: Warn about drivers not releasing media device safely Sakari Ailus
2024-06-17 10:40   ` Hans Verkuil
2024-06-17 17:59     ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 25/26] media: mc: Enforce one-time registration Sakari Ailus
2024-06-17 10:42   ` Hans Verkuil
2024-06-18  6:39     ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 26/26] media: Documentation: Document media device memory safety helper Sakari Ailus
2024-06-17 11:55 ` [PATCH v4 00/26] Media device lifetime management Hans Verkuil
2024-06-18 10:30   ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240610100530.1107771-6-sakari.ailus@linux.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.