[v4,05/26] Revert "[media] media-device: dynamically allocate struct media_devnode"

Message ID	20240610100530.1107771-6-sakari.ailus@linux.intel.com (mailing list archive)
State	New
Headers	show Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B8E1478B4E for <linux-media@vger.kernel.org>; Mon, 10 Jun 2024 10:05:52 +0000 (UTC) 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 Message-Id: <20240610100530.1107771-6-sakari.ailus@linux.intel.com> In-Reply-To: <20240610100530.1107771-1-sakari.ailus@linux.intel.com> References: <20240610100530.1107771-1-sakari.ailus@linux.intel.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	Media device lifetime management \| expand [v4,00/26] Media device lifetime management [v4,01/26] Revert "[media] media: fix media devnode ioctl/syscall and unregister race" [v4,02/26] Revert "media: utilize new cdev_device_add helper function" [v4,03/26] Revert "[media] media: fix use-after-free in cdev_put() when app exits after driver unbi… [v4,04/26] media: mc, cec: Make use of cdev_device_add() again [v4,05/26] Revert "[media] media-device: dynamically allocate struct media_devnode" [v4,06/26] media: mc: Drop nop release callback [v4,07/26] media: mc: Drop media_dev description from struct media_devnode [v4,08/26] media: mc: Do not call cdev_device_del() if cdev_device_add() fails [v4,09/26] media: mc: Delete character device early [v4,10/26] media: mc: Clear minor number reservation at unregistration time [v4,11/26] media: mc: Split initialising and adding media devnode [v4,12/26] media: mc: Shuffle functions around [v4,13/26] media: mc: Initialise media devnode in media_device_init() [v4,14/26] media: mc: Refcount the media device [v4,15/26] media: v4l: Acquire a reference to the media device for every video device [v4,16/26] media: mc: Postpone graph object removal until free [v4,17/26] media: omap3isp: Release the isp device struct by media device callback [v4,18/26] media: ipu3-cio2: Release the cio2 device context by media device callback [v4,19/26] media: vimc: Release resources on media device release [v4,20/26] media: Documentation: Document how Media device resources are released [v4,21/26] media: mc: Add per-file-handle data support [v4,22/26] media: mc: Maintain a list of open file handles in a media device [v4,23/26] media: mc: Implement best effort media device removal safety sans refcount [v4,24/26] media: mc: Warn about drivers not releasing media device safely [v4,25/26] media: mc: Enforce one-time registration [v4,26/26] media: Documentation: Document media device memory safety helper

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;

[v4,05/26] Revert "[media] media-device: dynamically allocate struct media_devnode"

Commit Message

Patch