From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: seq module loading issue Date: Thu, 09 Oct 2014 08:42:31 +0200 Message-ID: References: Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 6505626052B for ; Thu, 9 Oct 2014 08:42:33 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Adam Goode Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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: > > 1. Connect to announce port > 2. Listen for any announcements for client/port start/end > 3. Send MIDIConnectionEvent events to JavaScript clients ( > http://webaudio.github.io/web-midi-api/#midiconnectionevent-interface) > > Here is the scenario where this fails: > > 1. Cold boot a machine with no MIDI devices plugged in > 2. Start client, subscribe on the seq announce port > 3. Insert USB MIDI device > 4. snd_rawmidi module is loaded, triggered by insertion of MIDI device > 5. 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: > > 1. 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. > > 2. 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. > > 3. 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 "); 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);