uvc_status_stop hangs if called from async_ctrl.work

BUG Sean Anderson 2 participants 6 messages Mar 10

Subsystems (3)

Media (V4L/DVB) USB USB video class

lore

Share & tools

lore

Sean Anderson

author

2026-03-10 19:57 UTC #0 lore raw

uvc_status_stop can be called from uvc_ctrl_status_event_work:

============================================
WARNING: possible recursive locking detected
6.19.6 #5 Not tainted
--------------------------------------------
kworker/3:1/59 is trying to acquire lock:
ffff8b6685d71e28 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)

but task is already holding lock:
ffffd3e4c0387e40 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3252 kernel/workqueue.c:3359)

other info that might help us debug this:

Possible unsafe locking scenario:

CPU0
----
lock((work_completion)(&dev->async_ctrl.work));
lock((work_completion)(&dev->async_ctrl.work));

*** DEADLOCK ***
May be due to missing lock nesting notation
5 locks held by kworker/3:1/59:
#0: ffff8b668004fb48 ((wq_completion)events){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3251 kernel/workqueue.c:3359)
#1: ffffd3e4c0387e40 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3252 kernel/workqueue.c:3359) #2: ffff8b66858524a0 (&chain->ctrl_mutex){+.+.}-{4:4}, at: uvc_ctrl_status_event (drivers/media/usb/uvc/uvc_ctrl.c:1955) uvcvideo
#3: ffff8b6685d71d90 (&dev->status_lock){+.+.}-{4:4}, at: uvc_status_put (drivers/media/usb/uvc/uvc_status.c:407) uvcvideo
#4: ffffffff86cefac0 (rcu_read_lock){....}-{1:3}, at: __flush_work (include/linux/rcupdate.h:331 include/linux/rcupdate.h:867 kernel/workqueue.c:4213 kernel/workqueue.c:4271)

stack backtrace:
CPU: 3 UID: 0 PID: 59 Comm: kworker/3:1 Not tainted 6.19.6 #5 PREEMPT(full)  4105649303813dfd90b7c3b8911a9bfd5ad160d7
Hardware name: SECO S.p.A. C93/C93, BIOS 1.12.02 Corinne 04 07/03/2025
Workqueue: events uvc_ctrl_status_event_work [uvcvideo]
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:124)
print_deadlock_bug (kernel/locking/lockdep.c:3044)
__lock_acquire (kernel/locking/lockdep.c:3897 kernel/locking/lockdep.c:5237)
lock_acquire (kernel/locking/lockdep.c:470 kernel/locking/lockdep.c:5870 kernel/locking/lockdep.c:5825)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? mark_held_locks (kernel/locking/lockdep.c:4325)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
__flush_work (kernel/workqueue.c:3986 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? __pfx_wq_barrier_func (kernel/workqueue.c:3794)
__cancel_work_sync (kernel/workqueue.c:4429)
 uvc_status_stop (drivers/media/usb/uvc/uvc_status.c:343 drivers/media/usb/uvc/uvc_status.c:322) uvcvideo
 uvc_status_put (drivers/media/usb/uvc/uvc_status.c:408) uvcvideo
 uvc_pm_put (drivers/media/usb/uvc/uvc_v4l2.c:47) uvcvideo
 uvc_ctrl_clear_handle.isra.0 (drivers/media/usb/uvc/uvc_ctrl.c:1939) uvcvideo
 uvc_ctrl_status_event (drivers/media/usb/uvc/uvc_ctrl.c:1959) uvcvideo
 uvc_ctrl_status_event_work (drivers/media/usb/uvc/uvc_ctrl.c:1996) uvcvideo
process_scheduled_works (arch/x86/include/asm/jump_label.h:37 include/trace/events/workqueue.h:110 kernel/workqueue.c:3281 kernel/workqueue.c:3359)

worker_thread (kernel/workqueue.c:3440)
? __pfx_worker_thread (kernel/workqueue.c:3386)
kthread (kernel/kthread.c:463)
? __pfx_kthread (kernel/kthread.c:412)
ret_from_fork (arch/x86/kernel/process.c:164)
? __pfx_kthread (kernel/kthread.c:412)
ret_from_fork_asm (arch/x86/entry/entry_64.S:256)

</TASK>

The task will wait forever for itself. This causes all future UVC status accesses to block with TASK_UNINTERRUPTIBLE and will prevent removing the UVC device (and this a clean shutdown).

I can reliably induce a hang by running qv4l2 and changing any control (but guvcview seems fine).

--Sean

Sean Anderson

author

2026-03-10 20:11 UTC #1 lore raw

Quoted 61 lines · Sean Anderson

uvc_status_stop can be called from uvc_ctrl_status_event_work:
============================================
WARNING: possible recursive locking detected
6.19.6 #5 Not tainted
--------------------------------------------
kworker/3:1/59 is trying to acquire lock:
ffff8b6685d71e28 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
but task is already holding lock:
ffffd3e4c0387e40 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3252 kernel/workqueue.c:3359)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock((work_completion)(&dev->async_ctrl.work));
lock((work_completion)(&dev->async_ctrl.work));
*** DEADLOCK ***
May be due to missing lock nesting notation
5 locks held by kworker/3:1/59:
#0: ffff8b668004fb48 ((wq_completion)events){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3251 kernel/workqueue.c:3359)
#1: ffffd3e4c0387e40 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3252 kernel/workqueue.c:3359) #2: ffff8b66858524a0 (&chain->ctrl_mutex){+.+.}-{4:4}, at: uvc_ctrl_status_event (drivers/media/usb/uvc/uvc_ctrl.c:1955) uvcvideo
#3: ffff8b6685d71d90 (&dev->status_lock){+.+.}-{4:4}, at: uvc_status_put (drivers/media/usb/uvc/uvc_status.c:407) uvcvideo
#4: ffffffff86cefac0 (rcu_read_lock){....}-{1:3}, at: __flush_work (include/linux/rcupdate.h:331 include/linux/rcupdate.h:867 kernel/workqueue.c:4213 kernel/workqueue.c:4271)
stack backtrace:
CPU: 3 UID: 0 PID: 59 Comm: kworker/3:1 Not tainted 6.19.6 #5 PREEMPT(full)  4105649303813dfd90b7c3b8911a9bfd5ad160d7
Hardware name: SECO S.p.A. C93/C93, BIOS 1.12.02 Corinne 04 07/03/2025
Workqueue: events uvc_ctrl_status_event_work [uvcvideo]
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:124)
print_deadlock_bug (kernel/locking/lockdep.c:3044)
__lock_acquire (kernel/locking/lockdep.c:3897 kernel/locking/lockdep.c:5237)
lock_acquire (kernel/locking/lockdep.c:470 kernel/locking/lockdep.c:5870 kernel/locking/lockdep.c:5825)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? mark_held_locks (kernel/locking/lockdep.c:4325)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
__flush_work (kernel/workqueue.c:3986 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? __pfx_wq_barrier_func (kernel/workqueue.c:3794)
__cancel_work_sync (kernel/workqueue.c:4429)
 uvc_status_stop (drivers/media/usb/uvc/uvc_status.c:343 drivers/media/usb/uvc/uvc_status.c:322) uvcvideo
 uvc_status_put (drivers/media/usb/uvc/uvc_status.c:408) uvcvideo
 uvc_pm_put (drivers/media/usb/uvc/uvc_v4l2.c:47) uvcvideo
 uvc_ctrl_clear_handle.isra.0 (drivers/media/usb/uvc/uvc_ctrl.c:1939) uvcvideo
 uvc_ctrl_status_event (drivers/media/usb/uvc/uvc_ctrl.c:1959) uvcvideo
 uvc_ctrl_status_event_work (drivers/media/usb/uvc/uvc_ctrl.c:1996) uvcvideo
process_scheduled_works (arch/x86/include/asm/jump_label.h:37 include/trace/events/workqueue.h:110 kernel/workqueue.c:3281 kernel/workqueue.c:3359)
worker_thread (kernel/workqueue.c:3440)
? __pfx_worker_thread (kernel/workqueue.c:3386)
kthread (kernel/kthread.c:463)
? __pfx_kthread (kernel/kthread.c:412)
ret_from_fork (arch/x86/kernel/process.c:164)
? __pfx_kthread (kernel/kthread.c:412)
ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
</TASK>

Quoted 8 lines

The task will wait forever for itself. This causes all future UVC status accesses to block with TASK_UNINTERRUPTIBLE and will prevent removing the UVC device (and this a clean shutdown).
I can reliably induce a hang by running qv4l2 and changing any control (but guvcview seems fine).
--Sean

+CC Ricardo since he seems to have worked on this area in the past

Ricardo Ribalda

2026-03-10 20:57 UTC #2 lore raw

Hi Sean

Thanks for the report.

I have not been able to repro with qv4l2 on my computer :(.

Could you try if this patch works for you? Not saying that it is beautiful patch, or the way to do it.... but it will let me know if I am looking in the right place.

(quoted diff) +0 -0

b/drivers/media/usb/uvc/uvc_status.c
index 231cfee8e7c2..cca2aed162c3 100644

drivers/media/usb/uvc/uvc_status.c +4 -2

@@ -340,7 +340,9 @@ static void uvc_status_stop(struct uvc_device *dev)
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work == current_work())
+               cancel_work(&w->work);
+       else if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
        /* Kill the urb. */

@@ -352,7 +354,7 @@ static void uvc_status_stop(struct uvc_device *dev)
         * cancelled before returning or it could then race with a future
         * uvc_status_start() call.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work != current_work() && cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
        /*

Quoted 73 lines · Sean Anderson

Quoted 61 lines · Sean Anderson
uvc_status_stop can be called from uvc_ctrl_status_event_work:
============================================
WARNING: possible recursive locking detected
6.19.6 #5 Not tainted
--------------------------------------------
kworker/3:1/59 is trying to acquire lock:
ffff8b6685d71e28 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
but task is already holding lock:
ffffd3e4c0387e40 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3252 kernel/workqueue.c:3359)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock((work_completion)(&dev->async_ctrl.work));
lock((work_completion)(&dev->async_ctrl.work));
*** DEADLOCK ***
May be due to missing lock nesting notation
5 locks held by kworker/3:1/59:
#0: ffff8b668004fb48 ((wq_completion)events){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3251 kernel/workqueue.c:3359)
#1: ffffd3e4c0387e40 ((work_completion)(&dev->async_ctrl.work)){+.+.}-{0:0}, at: process_scheduled_works (kernel/workqueue.c:3252 kernel/workqueue.c:3359) #2: ffff8b66858524a0 (&chain->ctrl_mutex){+.+.}-{4:4}, at: uvc_ctrl_status_event (drivers/media/usb/uvc/uvc_ctrl.c:1955) uvcvideo
#3: ffff8b6685d71d90 (&dev->status_lock){+.+.}-{4:4}, at: uvc_status_put (drivers/media/usb/uvc/uvc_status.c:407) uvcvideo
#4: ffffffff86cefac0 (rcu_read_lock){....}-{1:3}, at: __flush_work (include/linux/rcupdate.h:331 include/linux/rcupdate.h:867 kernel/workqueue.c:4213 kernel/workqueue.c:4271)
stack backtrace:
CPU: 3 UID: 0 PID: 59 Comm: kworker/3:1 Not tainted 6.19.6 #5 PREEMPT(full)  4105649303813dfd90b7c3b8911a9bfd5ad160d7
Hardware name: SECO S.p.A. C93/C93, BIOS 1.12.02 Corinne 04 07/03/2025
Workqueue: events uvc_ctrl_status_event_work [uvcvideo]
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:124)
print_deadlock_bug (kernel/locking/lockdep.c:3044)
__lock_acquire (kernel/locking/lockdep.c:3897 kernel/locking/lockdep.c:5237)
lock_acquire (kernel/locking/lockdep.c:470 kernel/locking/lockdep.c:5870 kernel/locking/lockdep.c:5825)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? mark_held_locks (kernel/locking/lockdep.c:4325)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
__flush_work (kernel/workqueue.c:3986 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? __flush_work (kernel/workqueue.c:3985 kernel/workqueue.c:4239 kernel/workqueue.c:4271)
? __pfx_wq_barrier_func (kernel/workqueue.c:3794)
__cancel_work_sync (kernel/workqueue.c:4429)
 uvc_status_stop (drivers/media/usb/uvc/uvc_status.c:343 drivers/media/usb/uvc/uvc_status.c:322) uvcvideo
 uvc_status_put (drivers/media/usb/uvc/uvc_status.c:408) uvcvideo
 uvc_pm_put (drivers/media/usb/uvc/uvc_v4l2.c:47) uvcvideo
 uvc_ctrl_clear_handle.isra.0 (drivers/media/usb/uvc/uvc_ctrl.c:1939) uvcvideo
 uvc_ctrl_status_event (drivers/media/usb/uvc/uvc_ctrl.c:1959) uvcvideo
 uvc_ctrl_status_event_work (drivers/media/usb/uvc/uvc_ctrl.c:1996) uvcvideo
process_scheduled_works (arch/x86/include/asm/jump_label.h:37 include/trace/events/workqueue.h:110 kernel/workqueue.c:3281 kernel/workqueue.c:3359)
worker_thread (kernel/workqueue.c:3440)
? __pfx_worker_thread (kernel/workqueue.c:3386)
kthread (kernel/kthread.c:463)
? __pfx_kthread (kernel/kthread.c:412)
ret_from_fork (arch/x86/kernel/process.c:164)
? __pfx_kthread (kernel/kthread.c:412)
ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
</TASK>
Quoted 8 lines
The task will wait forever for itself. This causes all future UVC status accesses to block with TASK_UNINTERRUPTIBLE and will prevent removing the UVC device (and this a clean shutdown).
I can reliably induce a hang by running qv4l2 and changing any control (but guvcview seems fine).
--Sean
+CC Ricardo since he seems to have worked on this area in the past

Signature · 1 line

Ricardo Ribalda

Sean Anderson

author

2026-03-10 21:23 UTC #3 lore raw

Quoted 28 lines · Ricardo Ribalda

Hi Sean

Thanks for the report.

I have not been able to repro with qv4l2 on my computer :(.

Could you try if this patch works for you? Not saying that it is beautiful patch, or the way to do it.... but it will let me know if I am looking in the right place.

drivers/media/usb/uvc/uvc_status.c +3 -1

index 231cfee8e7c2..cca2aed162c3 100644
@@ -340,7 +340,9 @@ static void uvc_status_stop(struct uvc_device *dev)
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work == current_work())
+               cancel_work(&w->work);
+       else if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
        /* Kill the urb. */

drivers/media/usb/uvc/uvc_status.c static void uvc_status_stop(struct uvc_device *dev) · line 360

Quoted 8 lines

drivers/media/usb/uvc/uvc_status.c +1 -1

@@ -352,7 +354,7 @@ static void uvc_status_stop(struct uvc_device *dev)
         * cancelled before returning or it could then race with a future
         * uvc_status_start() call.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work != current_work() && cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
        /*

I don't think this works since the urb will be rescheduled as flush_status is set to false again. However, the following patch works for me:

drivers/media/usb/uvc/uvc_status.c +16 -9

index 231cfee8e7c2c..2a23606c7f4c6 100644
@@ -316,6 +316,14 @@ static int uvc_status_start(struct uvc_device *dev, gfp_t flags)
        if (!dev->int_urb)
                return 0;
 
+       /*
+        * If the work called uvc_status_stop it may still be running. Wait for
+        * it to finish before we submit the urb.
+        */
+       cancel_work_sync(&dev->async_ctrl.work);
+
+       /* Clear the flush status if we were previously stopped */
+       smp_store_release(&dev->flush_status, false);
        return usb_submit_urb(dev->int_urb, flags);
 }

@@ -336,6 +344,14 @@ static void uvc_status_stop(struct uvc_device *dev)
         */
        smp_store_release(&dev->flush_status, true);
 
+       /*
+        * We will deadlock if we are currently in the work function.
+        * Fortunately, we know that the URB is already dead and that no
+        * further work can be queued, so there's nothing left for us to do.
+        */
+       if (current_work() == &w->work)
+               return;
+
        /*
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.

@@ -354,15 +370,6 @@ static void uvc_status_stop(struct uvc_device *dev)
         */
        if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
-
-       /*
-        * From this point, there are no events on the queue and the status URB
-        * is dead. No events will be queued until uvc_status_start() is called.
-        * The barrier is needed to make sure that flush_status is visible to
-        * uvc_ctrl_status_event_work() when uvc_status_start() will be called
-        * again.
-        */
-       smp_store_release(&dev->flush_status, false);
 }
 
 int uvc_status_resume(struct uvc_device *dev)

The first cancel_work_sync also seems superfluous to me, since we have to cancel again anyway.

--Sean

Ricardo Ribalda

2026-03-10 22:08 UTC #4 lore raw

Hi Sean

Quoted 37 lines · Sean Anderson

Quoted 35 lines · Ricardo Ribalda

Hi Sean

Thanks for the report.

I have not been able to repro with qv4l2 on my computer :(.

Could you try if this patch works for you? Not saying that it is beautiful patch, or the way to do it.... but it will let me know if I am looking in the right place.

drivers/media/usb/uvc/uvc_status.c +4 -2

index 231cfee8e7c2..cca2aed162c3 100644
@@ -340,7 +340,9 @@ static void uvc_status_stop(struct uvc_device *dev)
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work == current_work())
+               cancel_work(&w->work);
+       else if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
        /* Kill the urb. */

@@ -352,7 +354,7 @@ static void uvc_status_stop(struct uvc_device *dev)
         * cancelled before returning or it could then race with a future
         * uvc_status_start() call.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work != current_work() && cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);

Quoted 4 lines

drivers/media/usb/uvc/uvc_status.c static void uvc_status_stop(struct uvc_device *dev) · line 360
Quoted 1 line
drivers/media/usb/uvc/uvc_status.c +0 -0
        /*
I don't think this works since the urb will be rescheduled as flush_status is set to false again. However, the following patch works for me:

It does not work, in the sense that the urb is re submited... but if would have confirmed the rootcause of the lockdep. But you have already proven that with your patch, which I think is correct :). Thanks for that

Can you resend it as a proper patch?

You probably want to add:

Fixes a32d9c41bdb8 ("media: uvcvideo: Make power management granular")

I will try to review with extra care and a big cup of tea tomorrow morning.

Thanks!

Quoted 53 lines

drivers/media/usb/uvc/uvc_status.c +16 ~~-10~~

index 231cfee8e7c2c..2a23606c7f4c6 100644
@@ -316,6 +316,14 @@ static int uvc_status_start(struct uvc_device *dev, gfp_t flags)
        if (!dev->int_urb)
                return 0;
 
+       /*
+        * If the work called uvc_status_stop it may still be running. Wait for
+        * it to finish before we submit the urb.
+        */
+       cancel_work_sync(&dev->async_ctrl.work);
+
+       /* Clear the flush status if we were previously stopped */
+       smp_store_release(&dev->flush_status, false);
        return usb_submit_urb(dev->int_urb, flags);
 }

@@ -336,6 +344,14 @@ static void uvc_status_stop(struct uvc_device *dev)
         */
        smp_store_release(&dev->flush_status, true);
 
+       /*
+        * We will deadlock if we are currently in the work function.
+        * Fortunately, we know that the URB is already dead and that no
+        * further work can be queued, so there's nothing left for us to do.
+        */
+       if (current_work() == &w->work)
+               return;
+
        /*
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.

@@ -354,15 +370,6 @@ static void uvc_status_stop(struct uvc_device *dev)
         */
        if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
-
-       /*
-        * From this point, there are no events on the queue and the status URB
-        * is dead. No events will be queued until uvc_status_start() is called.
-        * The barrier is needed to make sure that flush_status is visible to
-        * uvc_ctrl_status_event_work() when uvc_status_start() will be called
-        * again.
-        */
-       smp_store_release(&dev->flush_status, false);
 }
 
 int uvc_status_resume(struct uvc_device *dev)
--

Quoted 2 lines

The first cancel_work_sync also seems superfluous to me, since we have to cancel again anyway.

It has been a while since we did this, but I believe that since we did not use locks, the only way to guarantee the event queue was flushed and the URB was killed was to have the double cancel_work() in that order.

Quoted 2 lines

--Sean

Signature · 1 line

Ricardo Ribalda

Sean Anderson

author

2026-03-10 22:31 UTC #5 lore raw

Quoted 50 lines · Ricardo Ribalda

Hi Sean
Quoted 37 lines · Sean Anderson
Quoted 35 lines · Ricardo Ribalda
Hi Sean
Thanks for the report.
I have not been able to repro with qv4l2 on my computer :(.
Could you try if this patch works for you? Not saying that it is beautiful patch, or the way to do it.... but it will let me know if I am looking in the right place.
drivers/media/usb/uvc/uvc_status.c +4 -2
index 231cfee8e7c2..cca2aed162c3 100644
@@ -340,7 +340,9 @@ static void uvc_status_stop(struct uvc_device *dev)
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work == current_work())
+               cancel_work(&w->work);
+       else if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
        /* Kill the urb. */
@@ -352,7 +354,7 @@ static void uvc_status_stop(struct uvc_device *dev)
         * cancelled before returning or it could then race with a future
         * uvc_status_start() call.
         */
-       if (cancel_work_sync(&w->work))
+       if (&w->work != current_work() && cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
 
Quoted 4 lines
drivers/media/usb/uvc/uvc_status.c static void uvc_status_stop(struct uvc_device *dev) · line 360
Quoted 1 line
drivers/media/usb/uvc/uvc_status.c +0 -0
        /*
I don't think this works since the urb will be rescheduled as flush_status is set to false again. However, the following patch works for me:
It does not work, in the sense that the urb is re submited... but if would have confirmed the rootcause of the lockdep. But you have already proven that with your patch, which I think is correct :). Thanks for that

Quoted 4 lines

Can you resend it as a proper patch?
You probably want to add:
Fixes a32d9c41bdb8 ("media: uvcvideo: Make power management granular")

Done.

Quoted 57 lines

I will try to review with extra care and a big cup of tea tomorrow morning.

Thanks!

Quoted 53 lines

drivers/media/usb/uvc/uvc_status.c +16 ~~-10~~

index 231cfee8e7c2c..2a23606c7f4c6 100644
@@ -316,6 +316,14 @@ static int uvc_status_start(struct uvc_device *dev, gfp_t flags)
        if (!dev->int_urb)
                return 0;
 
+       /*
+        * If the work called uvc_status_stop it may still be running. Wait for
+        * it to finish before we submit the urb.
+        */
+       cancel_work_sync(&dev->async_ctrl.work);
+
+       /* Clear the flush status if we were previously stopped */
+       smp_store_release(&dev->flush_status, false);
        return usb_submit_urb(dev->int_urb, flags);
 }

@@ -336,6 +344,14 @@ static void uvc_status_stop(struct uvc_device *dev)
         */
        smp_store_release(&dev->flush_status, true);
 
+       /*
+        * We will deadlock if we are currently in the work function.
+        * Fortunately, we know that the URB is already dead and that no
+        * further work can be queued, so there's nothing left for us to do.
+        */
+       if (current_work() == &w->work)
+               return;
+
        /*
         * Cancel any pending asynchronous work. If any status event was queued,
         * process it synchronously.

@@ -354,15 +370,6 @@ static void uvc_status_stop(struct uvc_device *dev)
         */
        if (cancel_work_sync(&w->work))
                uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
-
-       /*
-        * From this point, there are no events on the queue and the status URB
-        * is dead. No events will be queued until uvc_status_start() is called.
-        * The barrier is needed to make sure that flush_status is visible to
-        * uvc_ctrl_status_event_work() when uvc_status_start() will be called
-        * again.
-        */
-       smp_store_release(&dev->flush_status, false);
 }
 
 int uvc_status_resume(struct uvc_device *dev)
--

Quoted 7 lines

Quoted 2 lines
The first cancel_work_sync also seems superfluous to me, since we have to cancel again anyway.
It has been a while since we did this, but I believe that since we did not use locks, the only way to guarantee the event queue was flushed and the URB was killed was to have the double cancel_work() in that order.

Oh, I see because we could have something like

CPU A CPU B
================================= ======================= uvc_ctrl_status_event_work

    smp_load_acqure(flush_status)
                                  uvc_status_stop()
                                      smp_store_release()
                                      usb_kill_urb()
                                      cancel_work_sync()
    usb_submit_urb()

--Sean