At Wed, 8 Oct 2014 18:00:41 -0400, Adam Goode wrote:
Hi,
I've observed a scenario in ALSA that I would like some feedback on solving. On Linux, Chrome uses seq for WebMIDI. Eventually, there will be hotplug detection code in Chrome that looks like this:
- Connect to announce port
- Listen for any announcements for client/port start/end
- Send MIDIConnectionEvent events to JavaScript clients (
http://webaudio.github.io/web-midi-api/#midiconnectionevent-interface)
Here is the scenario where this fails:
- Cold boot a machine with no MIDI devices plugged in
- Start client, subscribe on the seq announce port
- Insert USB MIDI device
- snd_rawmidi module is loaded, triggered by insertion of MIDI device
- snd_seq_midi is NOT loaded, but marked for loading later
At this point, there is no client start event sent to the announce port. This is because of the deferred module loading, which won't trigger except for a handful of operations (snd_seq_client_info_get_client is an example).
This is easily replicated with looking at aseqdump -p 0:1.
My question is: how to solve this? There are a few options:
- Periodically poll for new clients. This is fairly ugly, but will work to
trigger module loading and won't result in latency once any device has been plugged in, since announce will again work.
- Somehow trigger module loading if there are any clients listening on the
announce port. This seems pretty tricky and requires keeping track of some new state.
- Do away with deferred module loading in seq. This is pretty invasive and
would increase the number of modules loaded, but would result in a lot of deleted code in the kernel and a clean fix.
Number 3 is my preferred solution, but I wanted to see if these sorts of patches would be acceptable, since it is invasive and would result in more modules loaded.
I don't like that option, it's too risky at this stage. If it were a simple cleanup, I'm fine with it. But this leads to a major behavior change, which has a high risk of incompatibility.
How about a simple patch like below (totally untested)?
Takashi
--- diff --git a/sound/core/seq/seq_device.c b/sound/core/seq/seq_device.c index 91a786a783e1..38e400bce205 100644 --- a/sound/core/seq/seq_device.c +++ b/sound/core/seq/seq_device.c @@ -51,6 +51,17 @@ MODULE_AUTHOR("Takashi Iwai tiwai@suse.de"); MODULE_DESCRIPTION("ALSA sequencer device management"); MODULE_LICENSE("GPL");
+static void call_autoload(struct work_struct *work) +{ + snd_seq_device_load_drivers(); +} + +static DECLARE_WORK(autoload_work, call_autoload); + +static bool autoload; +module_param(autoload, bool, 0644); +MODULE_PARM_DESC(autoload, "Load sequencer driver module automatically"); + /* driver state */ #define DRIVER_EMPTY 0 #define DRIVER_LOADED (1<<0) @@ -221,6 +232,8 @@ int snd_seq_device_new(struct snd_card *card, int device, char *id, int argsize, return err; } + if (autoload) + schedule_work(&autoload_work); if (result) *result = dev;
@@ -554,6 +567,7 @@ static int __init alsa_seq_device_init(void)
static void __exit alsa_seq_device_exit(void) { + cancel_work_sync(&autoload_work); remove_drivers(); #ifdef CONFIG_PROC_FS snd_info_free_entry(info_entry);