On Tue May 19 12:07:59 2026 +0200, Hans Verkuil wrote:
> 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 <[email protected]>
> Tested-by: Hans Verkuil <[email protected]>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Patch committed.
Thanks,
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 @@ unregister_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;
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
