It can be hard to debug if all requests and request objects are correctly cleaned up when all related filehandles are closed. This patch adds a new 'requests' debugfs entry (if CONFIG_VIDEO_ADV_DEBUG is set) to report the number of open requests and request objects for a given media device node. Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> --- This patch has been outstanding for too long, it is time that this is merged since it is really useful to have, especially for regression testing. Changes since v2: https://patchwork.linuxtv.org/project/linux-media/patch/20200818143719.102128-2-hverkuil-cisco@xxxxxxxxx/ - Fix some CamelCase occurances in comments --- drivers/media/mc/mc-device.c | 27 +++++++++++++++++++++++++++ drivers/media/mc/mc-devnode.c | 13 +++++++++++++ drivers/media/mc/mc-request.c | 5 +++++ include/media/media-device.h | 9 +++++++++ include/media/media-devnode.h | 4 ++++ include/media/media-request.h | 2 ++ 6 files changed, 60 insertions(+) diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c index 9e56d2ad6b94..868ac3200eb7 100644 --- a/drivers/media/mc/mc-device.c +++ b/drivers/media/mc/mc-device.c @@ -691,6 +691,23 @@ void media_device_unregister_entity(struct media_entity *entity) } EXPORT_SYMBOL_GPL(media_device_unregister_entity); +#if IS_ENABLED(CONFIG_DEBUG_FS) && IS_ENABLED(CONFIG_VIDEO_ADV_DEBUG) +/* + * Log the state of media requests. + * Very useful for debugging. + */ +static int media_device_requests(struct seq_file *file, void *priv) +{ + struct media_device *dev = dev_get_drvdata(file->private); + + seq_printf(file, "number of requests: %d\n", + atomic_read(&dev->num_requests)); + seq_printf(file, "number of request objects: %d\n", + atomic_read(&dev->num_request_objects)); + return 0; +} +#endif + /** * media_device_init() - initialize a media device * @mdev: The media device @@ -714,6 +731,8 @@ void media_device_init(struct media_device *mdev) ida_init(&mdev->entity_internal_idx); atomic_set(&mdev->request_id, 0); + atomic_set(&mdev->num_requests, 0); + atomic_set(&mdev->num_request_objects, 0); dev_dbg(mdev->dev, "Media device initialized\n"); } @@ -766,6 +785,13 @@ int __must_check __media_device_register(struct media_device *mdev, dev_dbg(mdev->dev, "Media device registered\n"); +#if IS_ENABLED(CONFIG_DEBUG_FS) && IS_ENABLED(CONFIG_VIDEO_ADV_DEBUG) + mdev->media_dir = debugfs_create_dir(dev_name(&devnode->dev), + media_top_dir); + debugfs_create_devm_seqfile(&devnode->dev, "requests", + mdev->media_dir, media_device_requests); +#endif + return 0; } EXPORT_SYMBOL_GPL(__media_device_register); @@ -843,6 +869,7 @@ void media_device_unregister(struct media_device *mdev) dev_dbg(mdev->dev, "Media device unregistered\n"); + debugfs_remove_recursive(mdev->media_dir); device_remove_file(&mdev->devnode->dev, &dev_attr_model); media_devnode_unregister(mdev->devnode); /* devnode free is handled in media_devnode_*() */ diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c index f11382afe23b..cb229556c989 100644 --- a/drivers/media/mc/mc-devnode.c +++ b/drivers/media/mc/mc-devnode.c @@ -45,6 +45,12 @@ static dev_t media_dev_t; static DEFINE_MUTEX(media_devnode_lock); static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES); +/* + * debugfs + */ +struct dentry *media_top_dir; + + /* Called when the last user of the media device exits. */ static void media_devnode_release(struct device *cd) { @@ -252,6 +258,8 @@ int __must_check media_devnode_register(struct media_device *mdev, goto cdev_add_error; } + dev_set_drvdata(&devnode->dev, mdev); + /* Part 4: Activate this minor. The char device can now be used. */ set_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); @@ -304,6 +312,10 @@ static int __init media_devnode_init(void) return ret; } +#if IS_ENABLED(CONFIG_DEBUG_FS) && IS_ENABLED(CONFIG_VIDEO_ADV_DEBUG) + media_top_dir = debugfs_create_dir("media", NULL); +#endif + ret = bus_register(&media_bus_type); if (ret < 0) { unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); @@ -316,6 +328,7 @@ static int __init media_devnode_init(void) static void __exit media_devnode_exit(void) { + debugfs_remove_recursive(media_top_dir); bus_unregister(&media_bus_type); unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); } diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c index c0782fd96c59..3b013deaeb06 100644 --- a/drivers/media/mc/mc-request.c +++ b/drivers/media/mc/mc-request.c @@ -74,6 +74,7 @@ static void media_request_release(struct kref *kref) mdev->ops->req_free(req); else kfree(req); + atomic_dec(&mdev->num_requests); } void media_request_put(struct media_request *req) @@ -330,6 +331,7 @@ int media_request_alloc(struct media_device *mdev, int *alloc_fd) snprintf(req->debug_str, sizeof(req->debug_str), "%u:%d", atomic_inc_return(&mdev->request_id), fd); + atomic_inc(&mdev->num_requests); dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str); fd_install(fd, filp); @@ -357,6 +359,7 @@ static void media_request_object_release(struct kref *kref) if (WARN_ON(req)) media_request_object_unbind(obj); obj->ops->release(obj); + atomic_dec(&obj->mdev->num_request_objects); } struct media_request_object * @@ -420,6 +423,7 @@ int media_request_object_bind(struct media_request *req, obj->req = req; obj->ops = ops; obj->priv = priv; + obj->mdev = req->mdev; if (is_buffer) list_add_tail(&obj->list, &req->objects); @@ -427,6 +431,7 @@ int media_request_object_bind(struct media_request *req, list_add(&obj->list, &req->objects); req->num_incomplete_objects++; ret = 0; + atomic_inc(&obj->mdev->num_request_objects); unlock: spin_unlock_irqrestore(&req->lock, flags); diff --git a/include/media/media-device.h b/include/media/media-device.h index 1345e6da688a..493cb337c5f9 100644 --- a/include/media/media-device.h +++ b/include/media/media-device.h @@ -13,6 +13,7 @@ #include <linux/list.h> #include <linux/mutex.h> +#include <linux/atomic.h> #include <media/media-devnode.h> #include <media/media-entity.h> @@ -106,6 +107,9 @@ struct media_device_ops { * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t. * other operations that stop or start streaming. * @request_id: Used to generate unique request IDs + * @num_requests: number of associated requests + * @num_request_objects: number of associated request objects + * @media_dir: debugfs media directory * * This structure represents an abstract high-level media device. It allows easy * access to entities and provides basic media device-level support. The @@ -179,6 +183,11 @@ struct media_device { struct mutex req_queue_mutex; atomic_t request_id; + atomic_t num_requests; + atomic_t num_request_objects; + + /* debugfs */ + struct dentry *media_dir; }; /* We don't need to include pci.h or usb.h here */ diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h index d27c1c646c28..796a83df7336 100644 --- a/include/media/media-devnode.h +++ b/include/media/media-devnode.h @@ -20,9 +20,13 @@ #include <linux/fs.h> #include <linux/device.h> #include <linux/cdev.h> +#include <linux/debugfs.h> struct media_device; +/* debugfs top-level media directory */ +extern struct dentry *media_top_dir; + /* * 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 diff --git a/include/media/media-request.h b/include/media/media-request.h index 3cd25a2717ce..94d432557f00 100644 --- a/include/media/media-request.h +++ b/include/media/media-request.h @@ -256,6 +256,7 @@ struct media_request_object_ops { * struct media_request_object - An opaque object that belongs to a media * request * + * @mdev: media device this object belongs to * @ops: object's operations * @priv: object's priv pointer * @req: the request this object belongs to (can be NULL) @@ -267,6 +268,7 @@ struct media_request_object_ops { * another struct that contains the actual data for this request object. */ struct media_request_object { + struct media_device *mdev; const struct media_request_object_ops *ops; void *priv; struct media_request *req; -- 2.30.1