[PATCH v2] staging: most: video: prevent probes during component exit

[PATCH v2] staging: most: video: prevent probes during component exit

[Pavan Kumar Yalagada]

Previously, comp_probe_channel() would always add a new device to
video_devices, even if comp_exit() was running. This caused
BUG_ON(!list_empty(&video_devices)) in comp_exit() because
the list could be repopulated during teardown.

This patch adds a global static flag `comp_exiting`:

- Set `comp_exiting = true` at the start of comp_exit()
  to signal shutdown.
- Check `comp_exiting` at the start of comp_probe_channel()
  and return -EBUSY if set.

This prevents new devices from being added while the component
is exiting, ensuring video_devices stays empty and avoiding
BUG_ON triggers.

Signed-off-by: Pavan Kumar Yalagada <pavankumaryalagada@gmail.com>
---
 drivers/staging/most/video/video.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 32f71d9a9cf7..96366b7a0e29 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -60,6 +60,8 @@ static inline struct comp_fh *to_comp_fh(struct file *filp)
 static LIST_HEAD(video_devices);
 static DEFINE_SPINLOCK(list_lock);
 
+static bool comp_exiting;
+
 static inline bool data_ready(struct most_video_dev *mdev)
 {
 	return !list_empty(&mdev->pending_mbos);
@@ -453,6 +455,9 @@ static int comp_probe_channel(struct most_interface *iface, int channel_idx,
 			      struct most_channel_config *ccfg, char *name,
 			      char *args)
 {
+	if (comp_exiting)
+		return -EBUSY;
+
 	int ret;
 	struct most_video_dev *mdev = get_comp_dev(iface, channel_idx);
 
@@ -553,6 +558,7 @@ static int __init comp_init(void)
 
 static void __exit comp_exit(void)
 {
+	comp_exiting = true;
 	struct most_video_dev *mdev, *tmp;
 
 	/*
@@ -575,7 +581,7 @@ static void __exit comp_exit(void)
 
 	most_deregister_configfs_subsys(&comp);
 	most_deregister_component(&comp);
-	BUG_ON(!list_empty(&video_devices));
+	WARN_ON_ONCE(!list_empty(&video_devices));
 }
 
 module_init(comp_init);
-- 
2.47.3

Re: [PATCH v2] staging: most: video: prevent probes during component exit

[Greg KH]

On Wed, Nov 26, 2025 at 10:59:10AM -0500, Pavan Kumar Yalagada wrote:
> Previously, comp_probe_channel() would always add a new device to
> video_devices, even if comp_exit() was running. This caused
> BUG_ON(!list_empty(&video_devices)) in comp_exit() because
> the list could be repopulated during teardown.
> 
> This patch adds a global static flag `comp_exiting`:
> 
> - Set `comp_exiting = true` at the start of comp_exit()
>   to signal shutdown.
> - Check `comp_exiting` at the start of comp_probe_channel()
>   and return -EBUSY if set.
> 
> This prevents new devices from being added while the component
> is exiting, ensuring video_devices stays empty and avoiding
> BUG_ON triggers.
> 
> Signed-off-by: Pavan Kumar Yalagada <pavankumaryalagada@gmail.com>
> ---
>  drivers/staging/most/video/video.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
> index 32f71d9a9cf7..96366b7a0e29 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -60,6 +60,8 @@ static inline struct comp_fh *to_comp_fh(struct file *filp)
>  static LIST_HEAD(video_devices);
>  static DEFINE_SPINLOCK(list_lock);
>  
> +static bool comp_exiting;
> +
>  static inline bool data_ready(struct most_video_dev *mdev)
>  {
>  	return !list_empty(&mdev->pending_mbos);
> @@ -453,6 +455,9 @@ static int comp_probe_channel(struct most_interface *iface, int channel_idx,
>  			      struct most_channel_config *ccfg, char *name,
>  			      char *args)
>  {
> +	if (comp_exiting)
> +		return -EBUSY;

This will not work at all.

Also, you need to properly document a v2 patch.

thanks,

greg k-h

Re: [PATCH v2] staging: most: video: prevent probes during component exit

[Dan Carpenter]

On Wed, Nov 26, 2025 at 10:59:10AM -0500, Pavan Kumar Yalagada wrote:
> @@ -453,6 +455,9 @@ static int comp_probe_channel(struct most_interface *iface, int channel_idx,
>  			      struct most_channel_config *ccfg, char *name,
>  			      char *args)
>  {
> +	if (comp_exiting)
> +		return -EBUSY;
> +
>  	int ret;
>  	struct most_video_dev *mdev = get_comp_dev(iface, channel_idx);

You've added this check above the declaration block which is ugly.

But what would happen if comp_exiting is false when we check here but
it's true by the time we get to the line which says:

	list_add(&mdev->list, &video_devices);

There would still be a race.

>  
> @@ -553,6 +558,7 @@ static int __init comp_init(void)
>  
>  static void __exit comp_exit(void)
>  {
> +	comp_exiting = true;
>  	struct most_video_dev *mdev, *tmp;

In front of the declaration block.

>  
>  	/*
> @@ -575,7 +581,7 @@ static void __exit comp_exit(void)
>  
>  	most_deregister_configfs_subsys(&comp);
>  	most_deregister_component(&comp);
> -	BUG_ON(!list_empty(&video_devices));
> +	WARN_ON_ONCE(!list_empty(&video_devices));

When the race condition is fixed, then we can just remove this.

regards,
dan carpenter