[BUG] uvc_status_stop hangs if called from async_ctrl.work

[BUG] uvc_status_stop hangs if called from async_ctrl.work

[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>

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

Re: [BUG] uvc_status_stop hangs if called from async_ctrl.work

[Sean Anderson]

On 3/10/26 15:57, Sean Anderson wrote:
> 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

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

Re: [BUG] uvc_status_stop hangs if called from async_ctrl.work

[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.


diff --git a/drivers/media/usb/uvc/uvc_status.c
b/drivers/media/usb/uvc/uvc_status.c
index 231cfee8e7c2..cca2aed162c3 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -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);

        /*

On Tue, 10 Mar 2026 at 21:11, Sean Anderson <sean.anderson@linux.dev> wrote:
>
> On 3/10/26 15:57, Sean Anderson wrote:
> > 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
>
> +CC Ricardo since he seems to have worked on this area in the past



-- 
Ricardo Ribalda

Re: [BUG] uvc_status_stop hangs if called from async_ctrl.work

[Sean Anderson]

On 3/10/26 16:56, Ricardo Ribalda wrote:
> 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.
> 
> 
> diff --git a/drivers/media/usb/uvc/uvc_status.c
> b/drivers/media/usb/uvc/uvc_status.c
> index 231cfee8e7c2..cca2aed162c3 100644
> --- a/drivers/media/usb/uvc/uvc_status.c
> +++ b/drivers/media/usb/uvc/uvc_status.c
> @@ -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);
> 
>         /*

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:

diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index 231cfee8e7c2c..2a23606c7f4c6 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -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

Re: [BUG] uvc_status_stop hangs if called from async_ctrl.work

[Ricardo Ribalda]

Hi Sean

On Tue, 10 Mar 2026 at 22:23, Sean Anderson <sean.anderson@linux.dev> wrote:
>
> On 3/10/26 16:56, Ricardo Ribalda wrote:
> > 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.
> >
> >
> > diff --git a/drivers/media/usb/uvc/uvc_status.c
> > b/drivers/media/usb/uvc/uvc_status.c
> > index 231cfee8e7c2..cca2aed162c3 100644
> > --- a/drivers/media/usb/uvc/uvc_status.c
> > +++ b/drivers/media/usb/uvc/uvc_status.c
> > @@ -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);
> >
> >         /*
>
> 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!

>
> diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
> index 231cfee8e7c2c..2a23606c7f4c6 100644
> --- a/drivers/media/usb/uvc/uvc_status.c
> +++ b/drivers/media/usb/uvc/uvc_status.c
> @@ -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.

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.

>
> --Sean



-- 
Ricardo Ribalda

Re: [BUG] uvc_status_stop hangs if called from async_ctrl.work

[Sean Anderson]

On 3/10/26 18:08, Ricardo Ribalda wrote:
> Hi Sean
> 
> On Tue, 10 Mar 2026 at 22:23, Sean Anderson <sean.anderson@linux.dev> wrote:
>>
>> On 3/10/26 16:56, Ricardo Ribalda wrote:
>> > 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.
>> >
>> >
>> > diff --git a/drivers/media/usb/uvc/uvc_status.c
>> > b/drivers/media/usb/uvc/uvc_status.c
>> > index 231cfee8e7c2..cca2aed162c3 100644
>> > --- a/drivers/media/usb/uvc/uvc_status.c
>> > +++ b/drivers/media/usb/uvc/uvc_status.c
>> > @@ -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);
>> >
>> >         /*
>>
>> 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")

Done.

> I will try to review with extra care and a big cup of tea tomorrow morning.
> 
> Thanks!
> 
>>
>> diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
>> index 231cfee8e7c2c..2a23606c7f4c6 100644
>> --- a/drivers/media/usb/uvc/uvc_status.c
>> +++ b/drivers/media/usb/uvc/uvc_status.c
>> @@ -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.
> 
> 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