mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] media: wave5: Fix critical issues in driver remove path
@ 2025-12-04  9:41 Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 1/3] media: wave5: Fix PM runtime usage count underflow Xulin Sun
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Xulin Sun @ 2025-12-04  9:41 UTC (permalink / raw)
  To: nas.chung, jackson.lee, mchehab
  Cc: linux-media, linux-kernel, nicolas.dufresne, hverkuil,
	dafna.hirschfeld, xulin.sun

This patch series fixes three critical issues in the wave5 VPU driver's
remove path that cause kernel warnings and system crashes.

The issues were discovered and consistently reproduced on kernel
6.12.58-rt6 with the TI AM62A platform during extensive testing with
gstreamer encoding pipelines and module load/unload cycles.

v1: https://lore.kernel.org/linux-media/20251203040935.2685490-1-xulin.sun@windriver.com

Changes in v2:
- Reformatted commit messages per maintainer feedback
  * Removed "Symptom:/Root cause:/Fix:" section titles
  * Reordered content: fix description → root cause → symptoms
  * Added Fixes: tags for backport tracking
- Retested on media/master (6.18-rc5)
  * All three issues still exist in the code logic
  * Issues are harder to trigger on 6.18-rc5
  * Applied patches resolve all issues on both 6.12.58-rt6 and 6.18-rc5

Patch 1: Fixes PM runtime reference count underflow
Patch 2: Fixes kthread worker destruction warning
Patch 3: Fixes kernel panic due to incorrect cleanup order

Testing performed:
- 50+ consecutive H.264/H.265 encoding operations
- 100+ modprobe/rmmod cycles
- Tested on both 6.12.58-rt6 (TI AM62A) and 6.18-rc5
- No warnings or crashes observed after applying patches

Xulin Sun (3):
  media: wave5: Fix PM runtime usage count underflow
  media: wave5: Fix kthread worker destruction in polling mode
  media: wave5: Fix device cleanup order to prevent kernel panic

 drivers/media/platform/chips-media/wave5/wave5-vpu.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.49.1

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

* [PATCH v2 1/3] media: wave5: Fix PM runtime usage count underflow
  2025-12-04  9:41 [PATCH v2 0/3] media: wave5: Fix critical issues in driver remove path Xulin Sun
@ 2025-12-04  9:41 ` Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 2/3] media: wave5: Fix kthread worker destruction in polling mode Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 3/3] media: wave5: Fix device cleanup order to prevent kernel panic Xulin Sun
  2 siblings, 0 replies; 4+ messages in thread
From: Xulin Sun @ 2025-12-04  9:41 UTC (permalink / raw)
  To: nas.chung, jackson.lee, mchehab
  Cc: linux-media, linux-kernel, nicolas.dufresne, hverkuil,
	dafna.hirschfeld, xulin.sun

Replace pm_runtime_put_sync() with pm_runtime_dont_use_autosuspend() in
the remove path to properly pair with pm_runtime_use_autosuspend() from
probe. This allows pm_runtime_disable() to handle reference count cleanup
correctly regardless of current suspend state.

The driver calls pm_runtime_put_sync() unconditionally in remove, but the
device may already be suspended due to autosuspend configured in probe.
When autosuspend has already suspended the device, the usage count is 0,
and pm_runtime_put_sync() decrements it to -1.

This causes the following warning on module unload:

  ------------[ cut here ]------------
  WARNING: CPU: 1 PID: 963 at kernel/kthread.c:1430
    kthread_destroy_worker+0x84/0x98
  ...
  vdec 30210000.video-codec: Runtime PM usage count underflow!

Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index e1715d3f43b0..23aa3ab51a0e 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -356,7 +356,7 @@ static void wave5_vpu_remove(struct platform_device *pdev)
 		hrtimer_cancel(&dev->hrtimer);
 	}
 
-	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	mutex_destroy(&dev->dev_lock);
-- 
2.49.1

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

* [PATCH v2 2/3] media: wave5: Fix kthread worker destruction in polling mode
  2025-12-04  9:41 [PATCH v2 0/3] media: wave5: Fix critical issues in driver remove path Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 1/3] media: wave5: Fix PM runtime usage count underflow Xulin Sun
@ 2025-12-04  9:41 ` Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 3/3] media: wave5: Fix device cleanup order to prevent kernel panic Xulin Sun
  2 siblings, 0 replies; 4+ messages in thread
From: Xulin Sun @ 2025-12-04  9:41 UTC (permalink / raw)
  To: nas.chung, jackson.lee, mchehab
  Cc: linux-media, linux-kernel, nicolas.dufresne, hverkuil,
	dafna.hirschfeld, xulin.sun

Fix the cleanup order in polling mode (irq < 0) to prevent kernel warnings
during module removal. Cancel the hrtimer before destroying the kthread
worker to ensure work queues are empty.

In polling mode, the driver uses hrtimer to periodically trigger
wave5_vpu_timer_callback() which queues work via kthread_queue_work().
The kthread_destroy_worker() function validates that both work queues
are empty with WARN_ON(!list_empty(&worker->work_list)) and
WARN_ON(!list_empty(&worker->delayed_work_list)).

The original code called kthread_destroy_worker() before hrtimer_cancel(),
creating a race condition where the timer could fire during worker
destruction and queue new work, triggering the WARN_ON.

This causes the following warning on every module unload in polling mode:

  ------------[ cut here ]------------
  WARNING: CPU: 2 PID: 1034 at kernel/kthread.c:1430
    kthread_destroy_worker+0x84/0x98
  Modules linked in: wave5(-) rpmsg_ctrl rpmsg_char ...
  Call trace:
   kthread_destroy_worker+0x84/0x98
   wave5_vpu_remove+0xc8/0xe0 [wave5]
   platform_remove+0x30/0x58
  ...
  ---[ end trace 0000000000000000 ]---

Fixes: ed7276ed2fd0 ("media: chips-media: wave5: Add hrtimer based polling support")
Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 23aa3ab51a0e..0bcd48df49d0 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -352,8 +352,9 @@ static void wave5_vpu_remove(struct platform_device *pdev)
 	struct vpu_device *dev = dev_get_drvdata(&pdev->dev);
 
 	if (dev->irq < 0) {
-		kthread_destroy_worker(dev->worker);
 		hrtimer_cancel(&dev->hrtimer);
+		kthread_cancel_work_sync(&dev->work);
+		kthread_destroy_worker(dev->worker);
 	}
 
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
-- 
2.49.1

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

* [PATCH v2 3/3] media: wave5: Fix device cleanup order to prevent kernel panic
  2025-12-04  9:41 [PATCH v2 0/3] media: wave5: Fix critical issues in driver remove path Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 1/3] media: wave5: Fix PM runtime usage count underflow Xulin Sun
  2025-12-04  9:41 ` [PATCH v2 2/3] media: wave5: Fix kthread worker destruction in polling mode Xulin Sun
@ 2025-12-04  9:41 ` Xulin Sun
  2 siblings, 0 replies; 4+ messages in thread
From: Xulin Sun @ 2025-12-04  9:41 UTC (permalink / raw)
  To: nas.chung, jackson.lee, mchehab
  Cc: linux-media, linux-kernel, nicolas.dufresne, hverkuil,
	dafna.hirschfeld, xulin.sun

Move video device unregistration to the beginning of the remove function
to ensure all video operations are stopped before cleaning up the worker
thread and disabling PM runtime. This prevents hardware register access
after the device has been powered down.

In polling mode, the hrtimer periodically triggers
wave5_vpu_timer_callback() which queues work to the kthread worker.
The worker executes wave5_vpu_irq_work_fn() which reads hardware
registers via wave5_vdi_read_register().

The original cleanup order disabled PM runtime and powered down hardware
before unregistering video devices. When autosuspend triggers and powers
off the hardware, the video devices are still registered and the worker
thread can still be triggered by the hrtimer, causing it to attempt
reading registers from powered-off hardware. This results in a bus error
(synchronous external abort) and kernel panic.

This causes random kernel panics during encoding operations:

  Internal error: synchronous external abort: 0000000096000010
    [#1] PREEMPT SMP
  Modules linked in: wave5 rpmsg_ctrl rpmsg_char ...
  CPU: 0 UID: 0 PID: 1520 Comm: vpu_irq_thread
    Tainted: G   M    W
  pc : wave5_vdi_read_register+0x10/0x38 [wave5]
  lr : wave5_vpu_irq_work_fn+0x28/0x60 [wave5]
  Call trace:
   wave5_vdi_read_register+0x10/0x38 [wave5]
   kthread_worker_fn+0xd8/0x238
   kthread+0x104/0x120
   ret_from_fork+0x10/0x20
  Code: aa1e03e9 d503201f f9416800 8b214000 (b9400000)
  ---[ end trace 0000000000000000 ]---
  Kernel panic - not syncing: synchronous external abort:
    Fatal exception

Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 0bcd48df49d0..77d6c934d0b9 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -351,6 +351,10 @@ static void wave5_vpu_remove(struct platform_device *pdev)
 {
 	struct vpu_device *dev = dev_get_drvdata(&pdev->dev);
 
+	wave5_vpu_enc_unregister_device(dev);
+	wave5_vpu_dec_unregister_device(dev);
+	v4l2_device_unregister(&dev->v4l2_dev);
+
 	if (dev->irq < 0) {
 		hrtimer_cancel(&dev->hrtimer);
 		kthread_cancel_work_sync(&dev->work);
@@ -364,9 +368,6 @@ static void wave5_vpu_remove(struct platform_device *pdev)
 	mutex_destroy(&dev->hw_lock);
 	reset_control_assert(dev->resets);
 	clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
-	wave5_vpu_enc_unregister_device(dev);
-	wave5_vpu_dec_unregister_device(dev);
-	v4l2_device_unregister(&dev->v4l2_dev);
 	wave5_vdi_release(&pdev->dev);
 	ida_destroy(&dev->inst_ida);
 }
-- 
2.49.1

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

end of thread, other threads:[~2025-12-04  9:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-04  9:41 [PATCH v2 0/3] media: wave5: Fix critical issues in driver remove path Xulin Sun
2025-12-04  9:41 ` [PATCH v2 1/3] media: wave5: Fix PM runtime usage count underflow Xulin Sun
2025-12-04  9:41 ` [PATCH v2 2/3] media: wave5: Fix kthread worker destruction in polling mode Xulin Sun
2025-12-04  9:41 ` [PATCH v2 3/3] media: wave5: Fix device cleanup order to prevent kernel panic Xulin Sun

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

all inboxes | Powered by JetHome®