| Message ID | 20180804124526.46206-35-hverkuil@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Request API | expand |
Em Sat, 4 Aug 2018 14:45:26 +0200 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > From: Hans Verkuil <hans.verkuil@cisco.com> > > Keep track of the number of requests and request objects of a media > device. Helps to verify that all request-related memory is freed. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/media/media-device.c | 41 +++++++++++++++++++++++++++++++++++ > drivers/media/media-devnode.c | 17 +++++++++++++++ > drivers/media/media-request.c | 5 +++++ > include/media/media-device.h | 11 ++++++++++ > include/media/media-devnode.h | 4 ++++ > include/media/media-request.h | 2 ++ > 6 files changed, 80 insertions(+) > > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c > index 4b9a8de05562..28a891b53886 100644 > --- a/drivers/media/media-device.c > +++ b/drivers/media/media-device.c > @@ -691,6 +691,23 @@ void media_device_unregister_entity(struct media_entity *entity) > } > EXPORT_SYMBOL_GPL(media_device_unregister_entity); > > +#ifdef CONFIG_DEBUG_FS Patch itself looks good. Yet, perhaps we could request both CONFIG_DEBUG_FS and CONFIG_VIDEO_ADV_DEBUG. Also, instead of ifdef, please use IS_ENABLED for DEBUG_FS. That tends to be safer long term. With that: Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > +/* > + * 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 > @@ -713,6 +730,9 @@ void media_device_init(struct media_device *mdev) > mutex_init(&mdev->graph_mutex); > ida_init(&mdev->entity_internal_idx); > > + atomic_set(&mdev->num_requests, 0); > + atomic_set(&mdev->num_request_objects, 0); > + > dev_dbg(mdev->dev, "Media device initialized\n"); > } > EXPORT_SYMBOL_GPL(media_device_init); > @@ -764,6 +784,26 @@ int __must_check __media_device_register(struct media_device *mdev, > > dev_dbg(mdev->dev, "Media device registered\n"); > > +#ifdef CONFIG_DEBUG_FS > + if (!media_top_dir) > + return 0; > + > + mdev->media_dir = debugfs_create_dir(dev_name(&devnode->dev), > + media_top_dir); > + if (IS_ERR_OR_NULL(mdev->media_dir)) { > + dev_warn(mdev->dev, "Failed to create debugfs dir\n"); > + return 0; > + } > + mdev->requests_file = debugfs_create_devm_seqfile(&devnode->dev, > + "requests", mdev->media_dir, media_device_requests); > + if (IS_ERR_OR_NULL(mdev->requests_file)) { > + dev_warn(mdev->dev, "Failed to create requests file\n"); > + debugfs_remove_recursive(mdev->media_dir); > + mdev->media_dir = NULL; > + return 0; > + } > +#endif > + > return 0; > } > EXPORT_SYMBOL_GPL(__media_device_register); > @@ -841,6 +881,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/media-devnode.c b/drivers/media/media-devnode.c > index 6b87a721dc49..4358ed22f208 100644 > --- a/drivers/media/media-devnode.c > +++ b/drivers/media/media-devnode.c > @@ -53,6 +53,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) > { > @@ -259,6 +265,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); > > @@ -310,6 +318,14 @@ static int __init media_devnode_init(void) > return ret; > } > > +#ifdef CONFIG_DEBUG_FS > + media_top_dir = debugfs_create_dir("media", NULL); > + if (IS_ERR_OR_NULL(media_top_dir)) { > + pr_warn("Failed to create debugfs media dir\n"); > + media_top_dir = NULL; > + } > +#endif > + > ret = bus_register(&media_bus_type); > if (ret < 0) { > unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); > @@ -322,6 +338,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/media-request.c b/drivers/media/media-request.c > index a5b70a4e613b..8ba97a9c4bf1 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -72,6 +72,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) > @@ -318,6 +319,7 @@ int media_request_alloc(struct media_device *mdev, > get_task_comm(comm, current); > snprintf(req->debug_str, sizeof(req->debug_str), "%s:%d", > comm, fd); > + atomic_inc(&mdev->num_requests); > dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str); > > fd_install(fd, filp); > @@ -342,6 +344,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 * > @@ -405,10 +408,12 @@ int media_request_object_bind(struct media_request *req, > obj->req = req; > obj->ops = ops; > obj->priv = priv; > + obj->mdev = req->mdev; > > list_add_tail(&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 110b89567671..2de0606938d4 100644 > --- a/include/media/media-device.h > +++ b/include/media/media-device.h > @@ -21,6 +21,7 @@ > > #include <linux/list.h> > #include <linux/mutex.h> > +#include <linux/atomic.h> > > #include <media/media-devnode.h> > #include <media/media-entity.h> > @@ -107,6 +108,10 @@ struct media_device_ops { > * @ops: Operation handler callbacks > * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t. > * other operations that stop or start streaming. > + * @num_requests: number of associated requests > + * @num_request_objects: number of associated request objects > + * @media_dir: DebugFS media directory > + * @requests_file: DebugFS requests file > * > * 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 +184,12 @@ struct media_device { > const struct media_device_ops *ops; > > struct mutex req_queue_mutex; > + atomic_t num_requests; > + atomic_t num_request_objects; > + > + /* debugfs */ > + struct dentry *media_dir; > + struct dentry *requests_file; > }; > > /* 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 dc2f64e1b08f..984b62b634d3 100644 > --- a/include/media/media-devnode.h > +++ b/include/media/media-devnode.h > @@ -28,9 +28,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 fd08d7a431a1..76727d4a89c3 100644 > --- a/include/media/media-request.h > +++ b/include/media/media-request.h > @@ -210,6 +210,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) > @@ -221,6 +222,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; Thanks, Mauro
On 13/08/18 17:15, Mauro Carvalho Chehab wrote: > Em Sat, 4 Aug 2018 14:45:26 +0200 > Hans Verkuil <hverkuil@xs4all.nl> escreveu: > >> From: Hans Verkuil <hans.verkuil@cisco.com> >> >> Keep track of the number of requests and request objects of a media >> device. Helps to verify that all request-related memory is freed. >> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > > >> --- >> drivers/media/media-device.c | 41 +++++++++++++++++++++++++++++++++++ >> drivers/media/media-devnode.c | 17 +++++++++++++++ >> drivers/media/media-request.c | 5 +++++ >> include/media/media-device.h | 11 ++++++++++ >> include/media/media-devnode.h | 4 ++++ >> include/media/media-request.h | 2 ++ >> 6 files changed, 80 insertions(+) >> >> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c >> index 4b9a8de05562..28a891b53886 100644 >> --- a/drivers/media/media-device.c >> +++ b/drivers/media/media-device.c >> @@ -691,6 +691,23 @@ void media_device_unregister_entity(struct media_entity *entity) >> } >> EXPORT_SYMBOL_GPL(media_device_unregister_entity); >> >> +#ifdef CONFIG_DEBUG_FS > > Patch itself looks good. Yet, perhaps we could request both > CONFIG_DEBUG_FS and CONFIG_VIDEO_ADV_DEBUG. > > Also, instead of ifdef, please use IS_ENABLED for DEBUG_FS. That tends > to be safer long term. > > With that: > > Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> I don't intend to merge this patch yet. I'm not happy with it: I would really like to count the objects per-request instead of globally, and provide some more information about requests and their objects. It was really a quick hack so that I could verify that all requests and objects are properly freed. Regards, Hans > >> +/* >> + * 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 >> @@ -713,6 +730,9 @@ void media_device_init(struct media_device *mdev) >> mutex_init(&mdev->graph_mutex); >> ida_init(&mdev->entity_internal_idx); >> >> + atomic_set(&mdev->num_requests, 0); >> + atomic_set(&mdev->num_request_objects, 0); >> + >> dev_dbg(mdev->dev, "Media device initialized\n"); >> } >> EXPORT_SYMBOL_GPL(media_device_init); >> @@ -764,6 +784,26 @@ int __must_check __media_device_register(struct media_device *mdev, >> >> dev_dbg(mdev->dev, "Media device registered\n"); >> >> +#ifdef CONFIG_DEBUG_FS >> + if (!media_top_dir) >> + return 0; >> + >> + mdev->media_dir = debugfs_create_dir(dev_name(&devnode->dev), >> + media_top_dir); >> + if (IS_ERR_OR_NULL(mdev->media_dir)) { >> + dev_warn(mdev->dev, "Failed to create debugfs dir\n"); >> + return 0; >> + } >> + mdev->requests_file = debugfs_create_devm_seqfile(&devnode->dev, >> + "requests", mdev->media_dir, media_device_requests); >> + if (IS_ERR_OR_NULL(mdev->requests_file)) { >> + dev_warn(mdev->dev, "Failed to create requests file\n"); >> + debugfs_remove_recursive(mdev->media_dir); >> + mdev->media_dir = NULL; >> + return 0; >> + } >> +#endif >> + >> return 0; >> } >> EXPORT_SYMBOL_GPL(__media_device_register); >> @@ -841,6 +881,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/media-devnode.c b/drivers/media/media-devnode.c >> index 6b87a721dc49..4358ed22f208 100644 >> --- a/drivers/media/media-devnode.c >> +++ b/drivers/media/media-devnode.c >> @@ -53,6 +53,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) >> { >> @@ -259,6 +265,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); >> >> @@ -310,6 +318,14 @@ static int __init media_devnode_init(void) >> return ret; >> } >> >> +#ifdef CONFIG_DEBUG_FS >> + media_top_dir = debugfs_create_dir("media", NULL); >> + if (IS_ERR_OR_NULL(media_top_dir)) { >> + pr_warn("Failed to create debugfs media dir\n"); >> + media_top_dir = NULL; >> + } >> +#endif >> + >> ret = bus_register(&media_bus_type); >> if (ret < 0) { >> unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); >> @@ -322,6 +338,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/media-request.c b/drivers/media/media-request.c >> index a5b70a4e613b..8ba97a9c4bf1 100644 >> --- a/drivers/media/media-request.c >> +++ b/drivers/media/media-request.c >> @@ -72,6 +72,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) >> @@ -318,6 +319,7 @@ int media_request_alloc(struct media_device *mdev, >> get_task_comm(comm, current); >> snprintf(req->debug_str, sizeof(req->debug_str), "%s:%d", >> comm, fd); >> + atomic_inc(&mdev->num_requests); >> dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str); >> >> fd_install(fd, filp); >> @@ -342,6 +344,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 * >> @@ -405,10 +408,12 @@ int media_request_object_bind(struct media_request *req, >> obj->req = req; >> obj->ops = ops; >> obj->priv = priv; >> + obj->mdev = req->mdev; >> >> list_add_tail(&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 110b89567671..2de0606938d4 100644 >> --- a/include/media/media-device.h >> +++ b/include/media/media-device.h >> @@ -21,6 +21,7 @@ >> >> #include <linux/list.h> >> #include <linux/mutex.h> >> +#include <linux/atomic.h> >> >> #include <media/media-devnode.h> >> #include <media/media-entity.h> >> @@ -107,6 +108,10 @@ struct media_device_ops { >> * @ops: Operation handler callbacks >> * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t. >> * other operations that stop or start streaming. >> + * @num_requests: number of associated requests >> + * @num_request_objects: number of associated request objects >> + * @media_dir: DebugFS media directory >> + * @requests_file: DebugFS requests file >> * >> * 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 +184,12 @@ struct media_device { >> const struct media_device_ops *ops; >> >> struct mutex req_queue_mutex; >> + atomic_t num_requests; >> + atomic_t num_request_objects; >> + >> + /* debugfs */ >> + struct dentry *media_dir; >> + struct dentry *requests_file; >> }; >> >> /* 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 dc2f64e1b08f..984b62b634d3 100644 >> --- a/include/media/media-devnode.h >> +++ b/include/media/media-devnode.h >> @@ -28,9 +28,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 fd08d7a431a1..76727d4a89c3 100644 >> --- a/include/media/media-request.h >> +++ b/include/media/media-request.h >> @@ -210,6 +210,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) >> @@ -221,6 +222,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; > > > > Thanks, > Mauro >
Em Tue, 14 Aug 2018 09:33:16 +0200 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > On 13/08/18 17:15, Mauro Carvalho Chehab wrote: > > Em Sat, 4 Aug 2018 14:45:26 +0200 > > Hans Verkuil <hverkuil@xs4all.nl> escreveu: > > > >> From: Hans Verkuil <hans.verkuil@cisco.com> > >> > >> Keep track of the number of requests and request objects of a media > >> device. Helps to verify that all request-related memory is freed. Ok. So, let's skip it on your next patchset. Debugfs can be added any time after the requests API main patchset gets merged. > >> > >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > > > > > > >> --- > >> drivers/media/media-device.c | 41 +++++++++++++++++++++++++++++++++++ > >> drivers/media/media-devnode.c | 17 +++++++++++++++ > >> drivers/media/media-request.c | 5 +++++ > >> include/media/media-device.h | 11 ++++++++++ > >> include/media/media-devnode.h | 4 ++++ > >> include/media/media-request.h | 2 ++ > >> 6 files changed, 80 insertions(+) > >> > >> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c > >> index 4b9a8de05562..28a891b53886 100644 > >> --- a/drivers/media/media-device.c > >> +++ b/drivers/media/media-device.c > >> @@ -691,6 +691,23 @@ void media_device_unregister_entity(struct media_entity *entity) > >> } > >> EXPORT_SYMBOL_GPL(media_device_unregister_entity); > >> > >> +#ifdef CONFIG_DEBUG_FS > > > > Patch itself looks good. Yet, perhaps we could request both > > CONFIG_DEBUG_FS and CONFIG_VIDEO_ADV_DEBUG. > > > > Also, instead of ifdef, please use IS_ENABLED for DEBUG_FS. That tends > > to be safer long term. > > > > With that: > > > > Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > > I don't intend to merge this patch yet. I'm not happy with it: I would > really like to count the objects per-request instead of globally, and > provide some more information about requests and their objects. > > It was really a quick hack so that I could verify that all requests and > objects are properly freed. > > Regards, > > Hans > > > > >> +/* > >> + * 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 > >> @@ -713,6 +730,9 @@ void media_device_init(struct media_device *mdev) > >> mutex_init(&mdev->graph_mutex); > >> ida_init(&mdev->entity_internal_idx); > >> > >> + atomic_set(&mdev->num_requests, 0); > >> + atomic_set(&mdev->num_request_objects, 0); > >> + > >> dev_dbg(mdev->dev, "Media device initialized\n"); > >> } > >> EXPORT_SYMBOL_GPL(media_device_init); > >> @@ -764,6 +784,26 @@ int __must_check __media_device_register(struct media_device *mdev, > >> > >> dev_dbg(mdev->dev, "Media device registered\n"); > >> > >> +#ifdef CONFIG_DEBUG_FS > >> + if (!media_top_dir) > >> + return 0; > >> + > >> + mdev->media_dir = debugfs_create_dir(dev_name(&devnode->dev), > >> + media_top_dir); > >> + if (IS_ERR_OR_NULL(mdev->media_dir)) { > >> + dev_warn(mdev->dev, "Failed to create debugfs dir\n"); > >> + return 0; > >> + } > >> + mdev->requests_file = debugfs_create_devm_seqfile(&devnode->dev, > >> + "requests", mdev->media_dir, media_device_requests); > >> + if (IS_ERR_OR_NULL(mdev->requests_file)) { > >> + dev_warn(mdev->dev, "Failed to create requests file\n"); > >> + debugfs_remove_recursive(mdev->media_dir); > >> + mdev->media_dir = NULL; > >> + return 0; > >> + } > >> +#endif > >> + > >> return 0; > >> } > >> EXPORT_SYMBOL_GPL(__media_device_register); > >> @@ -841,6 +881,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/media-devnode.c b/drivers/media/media-devnode.c > >> index 6b87a721dc49..4358ed22f208 100644 > >> --- a/drivers/media/media-devnode.c > >> +++ b/drivers/media/media-devnode.c > >> @@ -53,6 +53,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) > >> { > >> @@ -259,6 +265,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); > >> > >> @@ -310,6 +318,14 @@ static int __init media_devnode_init(void) > >> return ret; > >> } > >> > >> +#ifdef CONFIG_DEBUG_FS > >> + media_top_dir = debugfs_create_dir("media", NULL); > >> + if (IS_ERR_OR_NULL(media_top_dir)) { > >> + pr_warn("Failed to create debugfs media dir\n"); > >> + media_top_dir = NULL; > >> + } > >> +#endif > >> + > >> ret = bus_register(&media_bus_type); > >> if (ret < 0) { > >> unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); > >> @@ -322,6 +338,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/media-request.c b/drivers/media/media-request.c > >> index a5b70a4e613b..8ba97a9c4bf1 100644 > >> --- a/drivers/media/media-request.c > >> +++ b/drivers/media/media-request.c > >> @@ -72,6 +72,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) > >> @@ -318,6 +319,7 @@ int media_request_alloc(struct media_device *mdev, > >> get_task_comm(comm, current); > >> snprintf(req->debug_str, sizeof(req->debug_str), "%s:%d", > >> comm, fd); > >> + atomic_inc(&mdev->num_requests); > >> dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str); > >> > >> fd_install(fd, filp); > >> @@ -342,6 +344,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 * > >> @@ -405,10 +408,12 @@ int media_request_object_bind(struct media_request *req, > >> obj->req = req; > >> obj->ops = ops; > >> obj->priv = priv; > >> + obj->mdev = req->mdev; > >> > >> list_add_tail(&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 110b89567671..2de0606938d4 100644 > >> --- a/include/media/media-device.h > >> +++ b/include/media/media-device.h > >> @@ -21,6 +21,7 @@ > >> > >> #include <linux/list.h> > >> #include <linux/mutex.h> > >> +#include <linux/atomic.h> > >> > >> #include <media/media-devnode.h> > >> #include <media/media-entity.h> > >> @@ -107,6 +108,10 @@ struct media_device_ops { > >> * @ops: Operation handler callbacks > >> * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t. > >> * other operations that stop or start streaming. > >> + * @num_requests: number of associated requests > >> + * @num_request_objects: number of associated request objects > >> + * @media_dir: DebugFS media directory > >> + * @requests_file: DebugFS requests file > >> * > >> * 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 +184,12 @@ struct media_device { > >> const struct media_device_ops *ops; > >> > >> struct mutex req_queue_mutex; > >> + atomic_t num_requests; > >> + atomic_t num_request_objects; > >> + > >> + /* debugfs */ > >> + struct dentry *media_dir; > >> + struct dentry *requests_file; > >> }; > >> > >> /* 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 dc2f64e1b08f..984b62b634d3 100644 > >> --- a/include/media/media-devnode.h > >> +++ b/include/media/media-devnode.h > >> @@ -28,9 +28,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 fd08d7a431a1..76727d4a89c3 100644 > >> --- a/include/media/media-request.h > >> +++ b/include/media/media-request.h > >> @@ -210,6 +210,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) > >> @@ -221,6 +222,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; > > > > > > > > Thanks, > > Mauro > > > Thanks, Mauro
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index 4b9a8de05562..28a891b53886 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -691,6 +691,23 @@ void media_device_unregister_entity(struct media_entity *entity) } EXPORT_SYMBOL_GPL(media_device_unregister_entity); +#ifdef CONFIG_DEBUG_FS +/* + * 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 @@ -713,6 +730,9 @@ void media_device_init(struct media_device *mdev) mutex_init(&mdev->graph_mutex); ida_init(&mdev->entity_internal_idx); + atomic_set(&mdev->num_requests, 0); + atomic_set(&mdev->num_request_objects, 0); + dev_dbg(mdev->dev, "Media device initialized\n"); } EXPORT_SYMBOL_GPL(media_device_init); @@ -764,6 +784,26 @@ int __must_check __media_device_register(struct media_device *mdev, dev_dbg(mdev->dev, "Media device registered\n"); +#ifdef CONFIG_DEBUG_FS + if (!media_top_dir) + return 0; + + mdev->media_dir = debugfs_create_dir(dev_name(&devnode->dev), + media_top_dir); + if (IS_ERR_OR_NULL(mdev->media_dir)) { + dev_warn(mdev->dev, "Failed to create debugfs dir\n"); + return 0; + } + mdev->requests_file = debugfs_create_devm_seqfile(&devnode->dev, + "requests", mdev->media_dir, media_device_requests); + if (IS_ERR_OR_NULL(mdev->requests_file)) { + dev_warn(mdev->dev, "Failed to create requests file\n"); + debugfs_remove_recursive(mdev->media_dir); + mdev->media_dir = NULL; + return 0; + } +#endif + return 0; } EXPORT_SYMBOL_GPL(__media_device_register); @@ -841,6 +881,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/media-devnode.c b/drivers/media/media-devnode.c index 6b87a721dc49..4358ed22f208 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -53,6 +53,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) { @@ -259,6 +265,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); @@ -310,6 +318,14 @@ static int __init media_devnode_init(void) return ret; } +#ifdef CONFIG_DEBUG_FS + media_top_dir = debugfs_create_dir("media", NULL); + if (IS_ERR_OR_NULL(media_top_dir)) { + pr_warn("Failed to create debugfs media dir\n"); + media_top_dir = NULL; + } +#endif + ret = bus_register(&media_bus_type); if (ret < 0) { unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); @@ -322,6 +338,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/media-request.c b/drivers/media/media-request.c index a5b70a4e613b..8ba97a9c4bf1 100644 --- a/drivers/media/media-request.c +++ b/drivers/media/media-request.c @@ -72,6 +72,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) @@ -318,6 +319,7 @@ int media_request_alloc(struct media_device *mdev, get_task_comm(comm, current); snprintf(req->debug_str, sizeof(req->debug_str), "%s:%d", comm, fd); + atomic_inc(&mdev->num_requests); dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str); fd_install(fd, filp); @@ -342,6 +344,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 * @@ -405,10 +408,12 @@ int media_request_object_bind(struct media_request *req, obj->req = req; obj->ops = ops; obj->priv = priv; + obj->mdev = req->mdev; list_add_tail(&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 110b89567671..2de0606938d4 100644 --- a/include/media/media-device.h +++ b/include/media/media-device.h @@ -21,6 +21,7 @@ #include <linux/list.h> #include <linux/mutex.h> +#include <linux/atomic.h> #include <media/media-devnode.h> #include <media/media-entity.h> @@ -107,6 +108,10 @@ struct media_device_ops { * @ops: Operation handler callbacks * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t. * other operations that stop or start streaming. + * @num_requests: number of associated requests + * @num_request_objects: number of associated request objects + * @media_dir: DebugFS media directory + * @requests_file: DebugFS requests file * * 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 +184,12 @@ struct media_device { const struct media_device_ops *ops; struct mutex req_queue_mutex; + atomic_t num_requests; + atomic_t num_request_objects; + + /* debugfs */ + struct dentry *media_dir; + struct dentry *requests_file; }; /* 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 dc2f64e1b08f..984b62b634d3 100644 --- a/include/media/media-devnode.h +++ b/include/media/media-devnode.h @@ -28,9 +28,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 fd08d7a431a1..76727d4a89c3 100644 --- a/include/media/media-request.h +++ b/include/media/media-request.h @@ -210,6 +210,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) @@ -221,6 +222,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;