Re: [PATCH v2 2/2] ALSA: usb: qcom: manage offload device usage

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Mar 17, 2026 at 04:45:00PM -0400, Guan-Yu Lin wrote:
> > On Wed, Mar 11, 2026 at 5:31 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > You have multiple levels of locks here, which is always a sign that
> > > something has gone really wrong.  This looks even more fragile and easy
> > > to get wrong than before.  Are you _SURE_ this is the only way to solve
> > > this?  The whole usb_offload_get() api seems more wrong to me than
> > > before (and I didn't like it even then...)
> > >
> > > In other words, this patch set feels rough, and adds more complexity
> > > overall, requiring a reviewer to "know" where locks are held and not
> > > held while before they did not.  That's a lot to push onto us for
> > > something that feels like is due to a broken hardware design?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> 
> Hi Greg,
> 
> Thank you for the feedback. I understand the concern regarding locking
> complexity and the reviewer burden. The usb_offload_get/put functions
> track sideband activity that runtime PM cannot reflect. This is
> necessary to prevent the USB stack from suspending the device while a
> sideband stream is active. Ensuring atomicity requires orchestrating
> three asynchronous subsystems: System PM, Runtime PM, and USB Core.
> 
> To address the concerns effectively in the next iteration, I would
> appreciate clarification on your primary concern:
> 1. Preference for fine-grained locking:
> Using USB device lock ensures atomicity across these subsystems, which
> is inherently a device-wide requirement. A fine-grained approach risks
> races during concurrent software events, such as a reset occurring
> during a runtime suspend transition.
> 2. Preference for improved transparency:
> If the coarse lock is acceptable but the implementation is too opaque,
> I will refactor the next version to be more explicit. I plan to
> include device_lock_assert() calls, __must_hold() macros, and add a
> "Locking Hierarchy" comment block documenting the vendor-mutex and
> USB-core lock interactions.

At the very least, this is going to have to be required so that we can
catch any future changes and ensure that changes do not create locking
inversions and the like.  I guess we are stuck with this for now, due to
this being "outside" of the normal runtime PM, which still feels wrong
to me as runtime PM _should_ be able to handle this (if not, why is this
case somehow unique from all other hardware types?)

> To clarify the "broken hardware" point: this isn't a hardware bug.

It was described as triggering when a shock happened to the system to
cause the system to reset in places, which is a hardware issue :)

> These races are triggered by standard software events, such as a reset
> occurring while a sideband stream is active. Please let me know which
> direction you think is more appropriate for the kernel, and I will
> refactor the next version accordingly.

And how exactly can a "reset while active" happen as just a normal
software driven event?

thanks,

greg k-h