This patch introduces a new module parameter "outqueue" which allows userspace to set the amount of output URBs is a range from 1 to 7 with 7 being the default (illegal values are clamped to the next valid value).
It is assumed that the previous patch has been applied. Looking then at a multiport USB MIDI interface with a wMaxPacketSize of 64 and assuming an optimal output rate of 320us/byte the following is true:
16 MIDI events PER URB Up to 3 bytes per MIDI event (3 bytes are assumed below) A driver queue of 7 URBs which are completely filled. Resulting latency is 16*3*7*320us=107.52ms
So if all URBs are filled by e.g. a sysex transfer on one port, a realtime message delivered by userspace for another port will be delayed by 107.52ms which is far too long.
After applying this patch and selecting a queue of only 1 URB tests show that the throughput isn't really affected, the latency however drops to 16*3*1*320us=15.36ms which is considerably better.
As the default is not touched no implications are expected for existing users. By making the parameter writable via sysfs it is possible to do selective output URB queue size management from userspace via a script that manages USB authorize and the module option.
Signed-off-by: Andreas Steinmetz ast@domdv.de
--- a/sound/usb/midi.c 2020-03-13 19:19:28.614798593 +0100 +++ b/sound/usb/midi.c 2020-03-13 19:37:08.097411436 +0100 @@ -45,6 +45,7 @@ #include <linux/slab.h> #include <linux/timer.h> #include <linux/usb.h> +#include <linux/moduleparam.h> #include <linux/wait.h> #include <linux/usb/audio.h> #include <linux/module.h> @@ -77,6 +78,11 @@ MODULE_DESCRIPTION("USB Audio/MIDI helper module"); MODULE_LICENSE("Dual BSD/GPL");
+static int outqueue = OUTPUT_URBS; + +module_param(outqueue, int, 0644); +MODULE_PARM_DESC(outqueue, "Outgoing queue size, 1-7 (default: 7)."); +
struct usb_ms_header_descriptor { __u8 bLength; @@ -141,6 +147,7 @@ } urbs[OUTPUT_URBS]; unsigned int active_urbs; unsigned int drain_urbs; + unsigned int out_urbs; int max_transfer; /* size of urb buffer */ struct tasklet_struct tasklet; unsigned int next_urb; @@ -335,7 +342,7 @@ break; ep->active_urbs |= 1 << urb_index; } - if (++urb_index >= OUTPUT_URBS) + if (++urb_index >= ep->out_urbs) urb_index = 0; if (urb_index == ep->next_urb) break; @@ -1364,7 +1371,7 @@ { unsigned int i;
- for (i = 0; i < OUTPUT_URBS; ++i) + for (i = 0; i < ep->out_urbs; ++i) if (ep->urbs[i].urb) { free_urb_and_buffer(ep->umidi, ep->urbs[i].urb, ep->max_transfer); @@ -1397,7 +1404,14 @@ return -ENOMEM; ep->umidi = umidi;
- for (i = 0; i < OUTPUT_URBS; ++i) { + if (outqueue < 1) + ep->out_urbs = 1; + else if (outqueue > OUTPUT_URBS) + ep->out_urbs = OUTPUT_URBS; + else + ep->out_urbs = outqueue; + + for (i = 0; i < ep->out_urbs; ++i) { ep->urbs[i].urb = usb_alloc_urb(0, GFP_KERNEL); if (!ep->urbs[i].urb) { err = -ENOMEM; @@ -1434,7 +1448,7 @@ ep->max_transfer = 9; break; } - for (i = 0; i < OUTPUT_URBS; ++i) { + for (i = 0; i < ep->out_urbs; ++i) { buffer = usb_alloc_coherent(umidi->dev, ep->max_transfer, GFP_KERNEL, &ep->urbs[i].urb->transfer_dma); @@ -1525,7 +1539,7 @@ if (ep->out) tasklet_kill(&ep->out->tasklet); if (ep->out) { - for (j = 0; j < OUTPUT_URBS; ++j) + for (j = 0; j < ep->out->out_urbs; ++j) usb_kill_urb(ep->out->urbs[j].urb); if (umidi->usb_protocol_ops->finish_out_endpoint) umidi->usb_protocol_ops->finish_out_endpoint(ep->out);