Hi Hans, Thanks a lot for pushing this, it looks super useful (I missed the v2). On Sat, 2021-03-27 at 11:49 +0100, Hans Verkuil wrote: > 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. > I have always associated CONFIG_VIDEO_ADV_DEBUG to VIDIOC_DBG_S/G_REGISTER only. At least to me, re-using CONFIG_VIDEO_ADV_DEBUG is confusing: it's only documented in the code what this debug option does. How about just using CONFIG_DEBUG_FS, or adding a new one like CONFIG_MEDIA_CONTROLLER_DEBUG or CONFIG_MEDIA_CONTROLLER_REQUEST_DEBUG? (Coming to think about it, IMO the call-balance checks in drivers/media/common/videobuf2/videobuf2-core.c should be behind a specific option named like CONFIG_VIDEOBUF2_DEBUG) > 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)); Is it not better to print some entry per-requests and per-request objects instead of counters? I.e. for each request we create a file entry, and that file entry would read the objects. Thanks, Ezequiel
