Re: CVS commit: [isaki-audio2] src/sys/dev/usb

[Tetsuya Isaki <isaki%pastel-flower.jp@localhost>]

At Thu, 02 May 2019 10:46:14 +1000,
matthew green wrote:
> > Committed By:	isaki
> > Date:		Wed May  1 13:09:34 UTC 2019
> > 
> > Modified Files:
> > 	src/sys/dev/usb [isaki-audio2]: uaudio.c
> > 
> > Log Message:
> > Don't release sc_lock and sc_intr_lock in trigger_{input,output}.
> > In the past, sc_lock was IPL_SCHED and (probably) it had conflicted
> > with usb subroutines.  But at some point, sc_lock has changed to use
> > IPL_SOFTUSB so such problems should been gone.
> 
> sc_lock was ever IPL_SCHED.  only sc_intr_lock has been
> that in uaudio.

Oh, it's typo... it's sc_intr_lock.

> what are you trying to avoid here?  the problem this
> solved was not always a problem, but eg:
> 
> - drop the audio thread lock when calling into usb when it may sleep,
>   avoiding a deadlock between audiowrite and audioioctl.  this fixes
>   mixerctl -a vs. playing hanging the system
>   XXX probably need to check this in a bunch more places.
> 
> is the commit related to that change.  can you confirm
> that you can be playing and run mixerctl -a at the same
> time with your change in place?

Yes, I can run mixerctl -a or mixerctl -w while playing.  But
I think it's not related because my change is about trigger_*
and trigger_* is not called during playback/recording running.

What I tried to solve is rev1.148-1.149 which releases locks
(unnecessarily, I think).

trigger_* is called with sc_lock and sc_intr_lock held, so
it's (basically) strange to release such locks (Of course
I know it's necessary in some cases).
rev1.148-1.149 releases both of sc_lock and sc_intr_lock to
call uaudio_chan_open(), uaudio_chan_alloc_buffers() and
uaudio_chan_ptransfer().  But I've read(traveled) source codes
of these three USB functions and I believe that these can work
with IPL_SOFTUSB/IPL_NONE held (in other words, releasing locks
here is unnecessary).

Thanks,
---
Tetsuya Isaki <isaki%pastel-flower.jp@localhost / isaki%NetBSD.org@localhost>