At Thu, 09 Oct 2014 11:21:28 +0200, Takashi Iwai wrote:
At Thu, 09 Oct 2014 11:14:57 +0200, Clemens Ladisch wrote:
Takashi Iwai wrote:
Clemens Ladisch wrote:
Takashi Iwai wrote:
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.
But there would be no changed behaviour as far as the API is concerned (except for this particular issue, which is a bug).
Currently, the sequencer stuff can be suppressed by simply not loading snd-seq core module itself. Do you mean to drop this feature?
Yes.
Some distros don't load sequencer modules nowadays as default.
Do you know why? Any reason except memory?
Mostly yes, and also reduce the installation base.
So it would result in a clear behavior change on the whole system.
And alsa-lib tries its best to do autoloading to hide the fact that snd-seq might not have been loaded. Therefore, it has never been possible to assume, at any time, that snd-seq is _not_ loaded.
What if the sound system doesn't exist at all like a server? The module is built and provided even on such a system by a distro, but it's just not enabled.
Is the high risk of incompatibility that you mentioned this assumption, or anything else?
It results in a surprising outcome, and it's what I'd like to avoid.
Thinking of this again, what's missing right now is the instantaneous binding *only when* sequencer stuff is ready to use. That is, if snd-seq isn't loaded, we should assume that it's not for use, thus not forcibly load the module there. If so, a patch like below should work (again untested). It's a bit more than the previous one, but it's still simple enough and doesn't break.
Takashi
-- diff --git a/include/sound/seq_kernel.h b/include/sound/seq_kernel.h index 2398521f0998..24c1dc1523bb 100644 --- a/include/sound/seq_kernel.h +++ b/include/sound/seq_kernel.h @@ -106,9 +106,11 @@ int snd_seq_event_port_attach(int client, struct snd_seq_port_callback *pcbp, int snd_seq_event_port_detach(int client, int port);
#ifdef CONFIG_MODULES +void snd_seq_autoload_init(void); void snd_seq_autoload_lock(void); void snd_seq_autoload_unlock(void); #else +#define snd_seq_autoload_init() #define snd_seq_autoload_lock() #define snd_seq_autoload_unlock() #endif diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 225c73152ee9..92ce558cc9b6 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2589,6 +2589,7 @@ int __init snd_sequencer_device_init(void) mutex_unlock(®ister_mutex);
+ snd_seq_autoload_init(); return 0; }
diff --git a/sound/core/seq/seq_device.c b/sound/core/seq/seq_device.c index 91a786a783e1..a77455c67f1d 100644 --- a/sound/core/seq/seq_device.c +++ b/sound/core/seq/seq_device.c @@ -127,7 +127,7 @@ static void snd_seq_device_info(struct snd_info_entry *entry,
#ifdef CONFIG_MODULES /* avoid auto-loading during module_init() */ -static int snd_seq_in_init; +static int snd_seq_in_init = 1; /* default blocked until cleared by seq core */ void snd_seq_autoload_lock(void) { snd_seq_in_init++; @@ -137,6 +137,25 @@ void snd_seq_autoload_unlock(void) { snd_seq_in_init--; } + +static void try_autoload(struct ops_list *ops) +{ + if (!snd_seq_in_init && + !(ops->driver & DRIVER_LOADED) && + !(ops->driver & DRIVER_REQUESTED)) { + ops->driver |= DRIVER_REQUESTED; + request_module("snd-%s", ops->id); + } +} + +/* called by sequencer core */ +void snd_seq_autoload_init(void) +{ + snd_seq_in_init = 0; + snd_seq_device_load_drivers(); +} +#else +#define try_autoload(ops) #endif
void snd_seq_device_load_drivers(void) @@ -214,13 +233,15 @@ int snd_seq_device_new(struct snd_card *card, int device, char *id, int argsize, ops->num_devices++; mutex_unlock(&ops->reg_mutex);
- unlock_driver(ops); - if ((err = snd_device_new(card, SNDRV_DEV_SEQUENCER, dev, &dops)) < 0) { + unlock_driver(ops); snd_seq_device_free(dev); return err; } + try_autoload(ops); + unlock_driver(ops); + if (result) *result = dev;
@@ -572,4 +593,5 @@ EXPORT_SYMBOL(snd_seq_device_unregister_driver); #ifdef CONFIG_MODULES EXPORT_SYMBOL(snd_seq_autoload_lock); EXPORT_SYMBOL(snd_seq_autoload_unlock); +EXPORT_SYMBOL(snd_seq_autoload_init); #endif