From 288254383674be25b26cf137bc12bfe38fefbad6 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Tue, 19 May 2026 12:07:59 +0200 Subject: [PATCH] media: em28xx: use v4l2_device release callback The em28xx driver creates a lot of video devices, but life-time management is really bad. Instead use the struct v4l2_device release() callback to have a single place where memory can be freed once the last user has gone. Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Hans Verkuil Tested-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/em28xx/em28xx-video.c | 66 +++++++++++++++---------- drivers/media/usb/em28xx/em28xx.h | 1 - 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c index da0422c65e5f..42f5e7547cc4 100644 --- a/drivers/media/usb/em28xx/em28xx-video.c +++ b/drivers/media/usb/em28xx/em28xx-video.c @@ -2275,18 +2275,27 @@ static int radio_s_tuner(struct file *file, void *priv, } /* - * em28xx_free_v4l2() - Free struct em28xx_v4l2 + * em28xx_free_v4l2() - v4l2_device release callback * - * @ref: struct kref for struct em28xx_v4l2 + * @v4l2_dev: pointer to struct v4l2_device embedded in struct em28xx_v4l2 * - * Called when all users of struct em28xx_v4l2 are gone + * Called by the v4l2 core when the last reference to the v4l2_device is + * released. At this point no userspace file handle nor video_device node + * keeps the v4l2 instance alive anymore, so it is safe to release all + * v4l2-related resources and drop the em28xx device reference taken when + * the v4l2 extension was initialized. */ -static void em28xx_free_v4l2(struct kref *ref) +static void em28xx_free_v4l2(struct v4l2_device *v4l2_dev) { - struct em28xx_v4l2 *v4l2 = container_of(ref, struct em28xx_v4l2, ref); + struct em28xx_v4l2 *v4l2 = + container_of(v4l2_dev, struct em28xx_v4l2, v4l2_dev); + struct em28xx *dev = v4l2->dev; - v4l2->dev->v4l2 = NULL; + v4l2_ctrl_handler_free(&v4l2->ctrl_handler); + v4l2_device_unregister(v4l2_dev); + dev->v4l2 = NULL; kfree(v4l2); + kref_put(&dev->ref, em28xx_free_device); } /* @@ -2354,8 +2363,6 @@ static int em28xx_v4l2_open(struct file *filp) v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio); } - kref_get(&dev->ref); - kref_get(&v4l2->ref); v4l2->users++; mutex_unlock(&dev->lock); @@ -2411,14 +2418,14 @@ static int em28xx_v4l2_fini(struct em28xx *dev) video_unregister_device(&v4l2->vdev); } - v4l2_ctrl_handler_free(&v4l2->ctrl_handler); - v4l2_device_unregister(&v4l2->v4l2_dev); - - kref_put(&v4l2->ref, em28xx_free_v4l2); - mutex_unlock(&dev->lock); - kref_put(&dev->ref, em28xx_free_device); + /* + * Drop the initial reference taken at v4l2_device_register() time. + * The em28xx_free_v4l2() release callback will be invoked once all + * userspace file handles to the video device nodes are closed. + */ + v4l2_device_put(&v4l2->v4l2_dev); return 0; } @@ -2490,9 +2497,7 @@ static int em28xx_v4l2_close(struct file *filp) exit: v4l2->users--; - kref_put(&v4l2->ref, em28xx_free_v4l2); mutex_unlock(&dev->lock); - kref_put(&dev->ref, em28xx_free_device); return 0; } @@ -2711,7 +2716,6 @@ static int em28xx_v4l2_init(struct em28xx *dev) mutex_unlock(&dev->lock); return -ENOMEM; } - kref_init(&v4l2->ref); v4l2->dev = dev; dev->v4l2 = v4l2; @@ -2722,9 +2726,21 @@ static int em28xx_v4l2_init(struct em28xx *dev) if (ret < 0) { dev_err(&dev->intf->dev, "Call to v4l2_device_register() failed!\n"); - goto err; + dev->v4l2 = NULL; + kfree(v4l2); + mutex_unlock(&dev->lock); + return ret; } + /* + * From this point on, em28xx_free_v4l2() will be used to release + * v4l2-related resources when the v4l2_device refcount reaches + * zero. Take a reference to the em28xx device so that it cannot + * be freed before the v4l2 instance is released. + */ + v4l2->v4l2_dev.release = em28xx_free_v4l2; + kref_get(&dev->ref); + hdl = &v4l2->ctrl_handler; v4l2_ctrl_handler_init(hdl, 9); v4l2->v4l2_dev.ctrl_handler = hdl; @@ -3048,8 +3064,6 @@ static int em28xx_v4l2_init(struct em28xx *dev) dev_info(&dev->intf->dev, "V4L2 extension successfully initialized\n"); - kref_get(&dev->ref); - mutex_unlock(&dev->lock); return 0; @@ -3073,12 +3087,14 @@ static int em28xx_v4l2_init(struct em28xx *dev) video_unregister_device(&v4l2->vdev); } - v4l2_ctrl_handler_free(&v4l2->ctrl_handler); - v4l2_device_unregister(&v4l2->v4l2_dev); -err: - dev->v4l2 = NULL; - kref_put(&v4l2->ref, em28xx_free_v4l2); mutex_unlock(&dev->lock); + + /* + * Drop the initial reference. em28xx_free_v4l2() will be called + * once the last video_device node release has decremented the + * v4l2_device refcount to zero. + */ + v4l2_device_put(&v4l2->v4l2_dev); return ret; } diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h index 2b9b95b53307..7d70f37665a3 100644 --- a/drivers/media/usb/em28xx/em28xx.h +++ b/drivers/media/usb/em28xx/em28xx.h @@ -558,7 +558,6 @@ struct em28xx_eeprom { #define EM28XX_RESOURCE_VBI 0x02 struct em28xx_v4l2 { - struct kref ref; struct em28xx *dev; struct v4l2_device v4l2_dev;