On 10/12/20(Thu) 21:40, Alexandre Ratchov wrote: > On Thu, Dec 10, 2020 at 05:27:16PM +0100, Marcus Glocker wrote: > > Hi All, > > > > I recently started to play around with uvideo(4) and uaudio(4) on my > > amd64 iMacs. There I quickly noticed regular freezes when streaming > > USB video or audio. On some of those machines it was very frequent, > > like every few seconds the video or audio stream did freeze for ~1s, > > then resume, while the rest of the system did continue to operate fine. > > > > First I found that when running the machine with an SP kernel, the issue > > disappears. > > On SP kernels, interrupts are still working while the CPU is spinning > in kernel mode (as long as current IPL permits it). That's why audio > works better. > > > Secondly some debugging hours, and quite some e-mail > > exchanges with mpi@ later, I found that the freeze is getting triggered > > by the asmc(4) driver, specifically by the sensor_task_register() > > update function. My first intention was to change > > sensor_task_register() to call taskq_create() with the TASKQ_MPSAFE > > flag for a test, to remove the kernel lock, which also resolved the > > freezing with an MP kernel. [1] > > > > In the end I found that the asmc(4) sensor update code is calling a > > busy loop in asmc_wait(), where the delay call is spending ~50ms in > > average. Doing that during the KERNEL_LOCK() is resulting in > > noticeable USB ISOC transfer delays. Obviously replacing the delay(9) > > with tsleep_nsec(9) in asmc(4) did fix the issue as well. [2] > > > > I'm not sure if just applying diff [2] to the driver is the right > > approach finally or if we need to take a more generic path to address > > this problem. Any feedback, help, comments appreciated. > > > > Would asmc(4) work if we sleep for very long (tsleep_nsec() has no > upper bound)? Spinning during 50ms in kernel mode doesn't look right, > so using tsleep() looks as a step forward as long as sleeping doesn't > break asmc(4).
Another way to look at the issue is: Can this delay be reduced? Considering that asmc_wait() is called twice per asmc_write() and once per amsc_read() that means it is called at least 14 times in asmc_command() and up to 20 times when reading fan informations, If any of the operations times out the whole command is restarted, possibly twice. So it could be interesting to know if a particular query fail and is restarted? In this case, is it fixable and/or why do we need to restart the command 3 times? Or is the loop in asmc_wait() doing too big steps? Or too smalls which would imply that some commands fail? Improving/fixing the delays might be easier than re-structuring the driver and could fix both issues raised in this thread: holding the KERNEL_LOCK() for too long and delaying other sensors.
