public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] staging: most: video: replace BUG_ON() with pr_warn() in comp_exit()
@ 2026-04-08 11:28 Gabriel Rondon
  2026-04-08 11:51 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Gabriel Rondon @ 2026-04-08 11:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Christian Gromm, Parthiban Veerasooran, linux-staging,
	linux-kernel

Replace BUG_ON(!list_empty(&video_devices)) with an explicit check
and pr_warn() in the module exit function.

BUG_ON() is deprecated as it crashes the entire kernel on assertion
failure (see Documentation/process/deprecated.rst). WARN_ON() is
also inappropriate here since it crashes the system when
panic-on-warn is enabled. Use pr_warn() to log the unexpected
condition without any risk of crashing.

This is the last remaining BUG_ON() in the staging/most subsystem,
following the dim2 series that replaced five BUG_ON() calls.

Signed-off-by: Gabriel Rondon <grondon@gmail.com>
---
Changes since v1:
- Use pr_warn() instead of WARN_ON() to avoid crash with panic-on-warn
  (Greg Kroah-Hartman)

 drivers/staging/most/video/video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 04351f8ccccf..c5425c207552 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -577,7 +577,8 @@ static void __exit comp_exit(void)
 
 	most_deregister_configfs_subsys(&comp);
 	most_deregister_component(&comp);
-	BUG_ON(!list_empty(&video_devices));
+	if (!list_empty(&video_devices))
+		pr_warn("video_devices list not empty on exit\n");
 }
 
 module_init(comp_init);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: most: video: replace BUG_ON() with pr_warn() in comp_exit()
  2026-04-08 11:28 [PATCH v2] staging: most: video: replace BUG_ON() with pr_warn() in comp_exit() Gabriel Rondon
@ 2026-04-08 11:51 ` Greg Kroah-Hartman
  2026-04-08 13:43   ` Gabriel Rondon
  2026-04-12 22:23   ` [PATCH v3] staging: most: video: remove redundant cleanup " Gabriel Rondon
  0 siblings, 2 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2026-04-08 11:51 UTC (permalink / raw)
  To: Gabriel Rondon
  Cc: Christian Gromm, Parthiban Veerasooran, linux-staging,
	linux-kernel

On Wed, Apr 08, 2026 at 12:28:18PM +0100, Gabriel Rondon wrote:
> Replace BUG_ON(!list_empty(&video_devices)) with an explicit check
> and pr_warn() in the module exit function.
> 
> BUG_ON() is deprecated as it crashes the entire kernel on assertion
> failure (see Documentation/process/deprecated.rst). WARN_ON() is
> also inappropriate here since it crashes the system when
> panic-on-warn is enabled. Use pr_warn() to log the unexpected
> condition without any risk of crashing.
> 
> This is the last remaining BUG_ON() in the staging/most subsystem,
> following the dim2 series that replaced five BUG_ON() calls.
> 
> Signed-off-by: Gabriel Rondon <grondon@gmail.com>
> ---
> Changes since v1:
> - Use pr_warn() instead of WARN_ON() to avoid crash with panic-on-warn
>   (Greg Kroah-Hartman)
> 
>  drivers/staging/most/video/video.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
> index 04351f8ccccf..c5425c207552 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -577,7 +577,8 @@ static void __exit comp_exit(void)
>  
>  	most_deregister_configfs_subsys(&comp);
>  	most_deregister_component(&comp);
> -	BUG_ON(!list_empty(&video_devices));
> +	if (!list_empty(&video_devices))
> +		pr_warn("video_devices list not empty on exit\n");

How can this ever happen?  Why not fix that to ensure it can never
happen, and then this check can just be removed entirely?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: most: video: replace BUG_ON() with pr_warn() in comp_exit()
  2026-04-08 11:51 ` Greg Kroah-Hartman
@ 2026-04-08 13:43   ` Gabriel Rondon
  2026-04-12 22:23   ` [PATCH v3] staging: most: video: remove redundant cleanup " Gabriel Rondon
  1 sibling, 0 replies; 4+ messages in thread
From: Gabriel Rondon @ 2026-04-08 13:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Christian Gromm, Parthiban Veerasooran, linux-staging,
	linux-kernel

On Wed, Apr 08, 2026 at 01:51:59PM +0200, Greg Kroah-Hartman wrote:
> How can this ever happen?  Why not fix that to ensure it can never
> happen, and then this check can just be removed entirely?

Thanks for the review, Greg.

I looked at most_deregister_component() in core.c — it already calls
disconnect_channel() for every linked channel via bus_for_each_dev().
So the manual cleanup loop in comp_exit() and the comment saying
"mostcore currently doesn't call disconnect_channel()" are outdated.

For v3 I plan to remove the manual cleanup loop, the stale comment,
and the BUG_ON entirely — leaving comp_exit() to just call
most_deregister_configfs_subsys() and most_deregister_component(),
which already handles the full teardown.

Does that match what you had in mind?

Gabriel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3] staging: most: video: remove redundant cleanup in comp_exit()
  2026-04-08 11:51 ` Greg Kroah-Hartman
  2026-04-08 13:43   ` Gabriel Rondon
@ 2026-04-12 22:23   ` Gabriel Rondon
  1 sibling, 0 replies; 4+ messages in thread
From: Gabriel Rondon @ 2026-04-12 22:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Christian Gromm, Parthiban Veerasooran, linux-staging,
	linux-kernel

most_deregister_component() already calls disconnect_channel() for
every linked channel via bus_for_each_dev() in core.c, which invokes
comp_disconnect_channel() to remove each entry from the video_devices
list and tear down the V4L2 device.

The manual cleanup loop in comp_exit() duplicates this work and is
guarded by a stale comment claiming that "mostcore currently doesn't
call disconnect_channel() for linked channels" — but the core has
since been fixed to do exactly that.

Remove the redundant manual cleanup loop, the outdated comment, and
the BUG_ON() assertion that checked for a condition that can no longer
occur.

Signed-off-by: Gabriel Rondon <grondon@gmail.com>
---
Changes since v2:
- Remove the cleanup loop and BUG_ON() entirely instead of replacing
  BUG_ON() with pr_warn(). most_deregister_component() already calls
  disconnect_channel() for every linked channel via bus_for_each_dev()
  in core.c, which handles the teardown. (Greg Kroah-Hartman)

Changes since v1:
- v2 used pr_warn() instead of WARN_ON() to avoid a crash with
  panic-on-warn (Greg Kroah-Hartman). Superseded by the v3 approach
  above.

 drivers/staging/most/video/video.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 04351f8ccccf..98988b0d9330 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -555,29 +555,8 @@ static int __init comp_init(void)
 
 static void __exit comp_exit(void)
 {
-	struct most_video_dev *mdev, *tmp;
-	LIST_HEAD(free_list);
-
-	/*
-	 * As the mostcore currently doesn't call disconnect_channel()
-	 * for linked channels while we call most_deregister_component()
-	 * we simulate this call here.
-	 * This must be fixed in core.
-	 */
-	spin_lock_irq(&list_lock);
-	list_replace_init(&video_devices, &free_list);
-	spin_unlock_irq(&list_lock);
-
-	list_for_each_entry_safe(mdev, tmp, &free_list, list) {
-		list_del_init(&mdev->list);
-		comp_unregister_videodev(mdev);
-		v4l2_device_disconnect(&mdev->v4l2_dev);
-		v4l2_device_put(&mdev->v4l2_dev);
-	}
-
 	most_deregister_configfs_subsys(&comp);
 	most_deregister_component(&comp);
-	BUG_ON(!list_empty(&video_devices));
 }
 
 module_init(comp_init);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-12 22:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 11:28 [PATCH v2] staging: most: video: replace BUG_ON() with pr_warn() in comp_exit() Gabriel Rondon
2026-04-08 11:51 ` Greg Kroah-Hartman
2026-04-08 13:43   ` Gabriel Rondon
2026-04-12 22:23   ` [PATCH v3] staging: most: video: remove redundant cleanup " Gabriel Rondon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox