[PATCH 1/3] ALSA USB MIDI: Fix port starvation

[Andreas Steinmetz]

On Wed, 2020-03-18 at 10:35 +0100, Clemens Ladisch wrote:
> Andreas Steinmetz wrote:
> > On Tue, 2020-03-17 at 11:26 +0100, Clemens Ladisch wrote:
> > > Why is it necessary to immedately get a complete class-compliant
> > > message?
> > 
> > Maybe that's a misunderstanding caused by me using 'message'
> > instead of
> > 'event'. We need a complete MIDI event (or up to 3 sysex bytes) to
> > form
> > the class compliant 4 byte USB event. That's what the 'repeat'
> > shortcut
> > is for. And after one such event is collected to the URB the code
> > switches to the next port.
> 
> Why is it necessary to get a complete USB event before switching to
> the
> next port?  Why not just process one byte in each loop iteration?
> 

Because of fairness. Let's assume a keyboard sends 6 note on messages,
that's 6 3 byte events on a port, while another port just sends channel
pressure which is two bytes and with running status only one byte.
Instead of fair scheduling the new notes will be delayed by the channel
pressure when doing only one byte per iteration. So the grouped
together note on events would be torn apart further than necessary
which may be audible.

> > > The point is that we should not restart with the first port in
> > > each
> > > packet.
> > 
> > I was thinking about that but in practice I do not know any class
> > compliant interface that does have a number of outputs that is not
> > equal to 2^n, i.e. 1, 2, 4, 8, 16.
> > 
> > Then wMaxPacketSize is also always a 2^m value (16, 64, 512).
> > 
> > With m>=n+2 (I don't believe that there is any class compliant
> > interface for which this is not true)
> 
> At least ESI M4U has n=2, m=2.  It's for devices like this that the
> driver uses multiple packets.  (I do not know if this (revision of
> the) device is still used.)
> 

Well, in this rare case the patch wouldn't do any harm. The only thing
is that there would be no improvement in behaviour to the previous code
version for this special device.

Please note that I did take especially care to not increase cpu usage
(on x86_64) because I wanted to make sure that (hopefully) there is no
load increase for slow embedded systems.

> It's certainly true that using the full packet size and 7 packets is
> excessive in most situations.  The driver should _automatically_
> reduce these values when it is safe.  (I'm not saying that module
> parameters are completely superfluous, but they should never be
> necessary if the driver can avoid it.)
> 

If automatic tuning is possible, fine. Manual override should still be
possible, though, for the case the user knows better than an automatism
for the use case of the user.

> 
> I think the lowest-hanging fruit are the hardcoded 0x10, and that
> port->active is not a single bitfield; the latter will make
> searching for active ports easier.  I'll create patches for this.

I'm very interested.

Cheers
-- 
Andreas Steinmetz