For fun, here's a rough proof of concept patch to resolve the
snd-usb-audio khubd hang. It alters the behavior of snd_device
dev_disconnect methods for abstract layers control, info, and pcm to
also unregister sysfs/procfs entries. It adds dev_disconnect methods
to layers hwdep and timer to similar effect, and alters the behavior
in the pcm_oss and mixer_oss modules through their special interfaces.
Omitted are the seq and rawmidi layers, which I haven't looked at.
It moves the calls to snd_info_unregister() and snd_info_card_free()
from snd_card_free() to snd_card_disconnect(). Finally, it breaks
snd_card_free() into smaller functions, and leaves the
wait-for-last-close semantic intact, but adds an
snd_card_free_when_closed() function that causes the interesting parts
of snd_card_free() to be called in the context of
snd_card_file_remove().
Thanks.
-Sam Revitch
diff -r 08577d0b45ef core/control.c
--- a/core/control.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/control.c Thu Jun 15 03:02:53 2006 -0700
@@ -1375,6 +1375,11 @@ static int snd_ctl_dev_disconnect(struct
struct snd_card *card = device->device_data;
struct list_head *flist;
struct snd_ctl_file *ctl;
+ int err, cardnum;
+
+ snd_assert(card != NULL, return -ENXIO);
+ cardnum = card->number;
+ snd_assert(cardnum >= 0 && cardnum < SNDRV_CARDS, return -ENXIO);
down_read(&card->controls_rwsem);
list_for_each(flist, &card->ctl_files) {
@@ -1383,6 +1388,10 @@ static int snd_ctl_dev_disconnect(struct
kill_fasync(&ctl->fasync, SIGIO, POLL_ERR);
}
up_read(&card->controls_rwsem);
+
+ if ((err = snd_unregister_device(SNDRV_DEVICE_TYPE_CONTROL,
+ card, -1)) < 0)
+ return err;
return 0;
}
@@ -1408,15 +1417,8 @@ static int snd_ctl_dev_free(struct snd_d
*/
static int snd_ctl_dev_unregister(struct snd_device *device)
{
- struct snd_card *card = device->device_data;
- int err, cardnum;
-
- snd_assert(card != NULL, return -ENXIO);
- cardnum = card->number;
- snd_assert(cardnum >= 0 && cardnum < SNDRV_CARDS, return -ENXIO);
- if ((err = snd_unregister_device(SNDRV_DEVICE_TYPE_CONTROL,
- card, -1)) < 0)
- return err;
+ if (device->state == SNDRV_DEV_REGISTERED)
+ (void) snd_ctl_dev_disconnect(device);
return snd_ctl_dev_free(device);
}
diff -r 08577d0b45ef core/hwdep.c
--- a/core/hwdep.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/hwdep.c Thu Jun 15 03:02:53 2006 -0700
@@ -42,6 +42,7 @@ static int snd_hwdep_free(struct snd_hwd
static int snd_hwdep_free(struct snd_hwdep *hwdep);
static int snd_hwdep_dev_free(struct snd_device *device);
static int snd_hwdep_dev_register(struct snd_device *device);
+static int snd_hwdep_dev_disconnect(struct snd_device *device);
static int snd_hwdep_dev_unregister(struct snd_device *device);
@@ -353,6 +354,7 @@ int snd_hwdep_new(struct snd_card *card,
static struct snd_device_ops ops = {
.dev_free = snd_hwdep_dev_free,
.dev_register = snd_hwdep_dev_register,
+ .dev_disconnect = snd_hwdep_dev_disconnect,
.dev_unregister = snd_hwdep_dev_unregister
};
@@ -439,7 +441,7 @@ static int snd_hwdep_dev_register(struct
return 0;
}
-static int snd_hwdep_dev_unregister(struct snd_device *device)
+static int snd_hwdep_dev_disconnect(struct snd_device *device)
{
struct snd_hwdep *hwdep = device->device_data;
@@ -454,8 +456,16 @@ static int snd_hwdep_dev_unregister(stru
snd_unregister_oss_device(hwdep->oss_type, hwdep->card, hwdep->device);
#endif
snd_unregister_device(SNDRV_DEVICE_TYPE_HWDEP, hwdep->card, hwdep->device);
- list_del(&hwdep->list);
+ list_del_init(&hwdep->list);
mutex_unlock(®ister_mutex);
+ return 0;
+}
+
+static int snd_hwdep_dev_unregister(struct snd_device *device)
+{
+ struct snd_hwdep *hwdep = device->device_data;
+ snd_assert(hwdep != NULL, return -ENXIO);
+ (void) snd_hwdep_dev_disconnect(device);
return snd_hwdep_free(hwdep);
}
diff -r 08577d0b45ef core/info.c
--- a/core/info.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/info.c Thu Jun 15 03:02:53 2006 -0700
@@ -820,6 +820,21 @@ struct snd_info_entry *snd_info_create_c
EXPORT_SYMBOL(snd_info_create_card_entry);
+static int snd_info_disconnect(struct snd_info_entry *entry)
+{
+ struct proc_dir_entry *root;
+ if (!entry->disconnected) {
+ snd_assert(entry->p != NULL, return -ENXIO);
+ root = entry->parent == NULL ? snd_proc_root : entry->parent->p;
+ snd_assert(root, return -ENXIO);
+ mutex_lock(&info_mutex);
+ snd_remove_proc_entry(root, entry->p);
+ mutex_unlock(&info_mutex);
+ entry->disconnected = 1;
+ }
+ return 0;
+}
+
static int snd_info_dev_free_entry(struct snd_device *device)
{
struct snd_info_entry *entry = device->device_data;
@@ -836,8 +851,7 @@ static int snd_info_dev_disconnect_entry
static int snd_info_dev_disconnect_entry(struct snd_device *device)
{
struct snd_info_entry *entry = device->device_data;
- entry->disconnected = 1;
- return 0;
+ return snd_info_disconnect(entry);
}
static int snd_info_dev_unregister_entry(struct snd_device *device)
@@ -952,16 +966,9 @@ EXPORT_SYMBOL(snd_info_register);
*/
int snd_info_unregister(struct snd_info_entry * entry)
{
- struct proc_dir_entry *root;
-
if (! entry)
return 0;
- snd_assert(entry->p != NULL, return -ENXIO);
- root = entry->parent == NULL ? snd_proc_root : entry->parent->p;
- snd_assert(root, return -ENXIO);
- mutex_lock(&info_mutex);
- snd_remove_proc_entry(root, entry->p);
- mutex_unlock(&info_mutex);
+ (void) snd_info_disconnect(entry);
snd_info_free_entry(entry);
return 0;
}
diff -r 08577d0b45ef core/init.c
--- a/core/init.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/init.c Thu Jun 15 03:02:53 2006 -0700
@@ -310,70 +310,104 @@ int snd_card_disconnect(struct snd_card
if (err < 0)
snd_printk(KERN_ERR "not all devices for card %i can be disconnected\n", card->number);
- return 0;
-}
-
-EXPORT_SYMBOL(snd_card_disconnect);
-
-/**
- * snd_card_free - frees given soundcard structure
- * @card: soundcard structure
- *
- * This function releases the soundcard structure and the all assigned
- * devices automatically. That is, you don't have to release the devices
- * by yourself.
- *
- * Returns zero. Frees all associated devices and frees the control
- * interface associated to given soundcard.
- */
-int snd_card_free(struct snd_card *card)
-{
- struct snd_shutdown_f_ops *s_f_ops;
-
- if (card == NULL)
- return -EINVAL;
- mutex_lock(&snd_card_mutex);
- snd_cards[card->number] = NULL;
- mutex_unlock(&snd_card_mutex);
-
-#ifdef CONFIG_PM
- wake_up(&card->power_sleep);
-#endif
- /* wait, until all devices are ready for the free operation */
- wait_event(card->shutdown_sleep, card->files == NULL);
-
-#if defined(CONFIG_SND_MIXER_OSS) || defined(CONFIG_SND_MIXER_OSS_MODULE)
- if (snd_mixer_oss_notify_callback)
- snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE);
-#endif
- if (snd_device_free_all(card, SNDRV_DEV_CMD_PRE) < 0) {
- snd_printk(KERN_ERR "unable to free all devices (pre)\n");
- /* Fatal, but this situation should never occur */
- }
- if (snd_device_free_all(card, SNDRV_DEV_CMD_NORMAL) < 0) {
- snd_printk(KERN_ERR "unable to free all devices (normal)\n");
- /* Fatal, but this situation should never occur */
- }
- if (snd_device_free_all(card, SNDRV_DEV_CMD_POST) < 0) {
- snd_printk(KERN_ERR "unable to free all devices (post)\n");
- /* Fatal, but this situation should never occur */
- }
- if (card->private_free)
- card->private_free(card);
snd_info_unregister(card->proc_id);
if (snd_info_card_free(card) < 0) {
snd_printk(KERN_WARNING "unable to free card info\n");
/* Not fatal error */
}
+
+ return 0;
+}
+
+EXPORT_SYMBOL(snd_card_disconnect);
+
+/**
+ * snd_card_free - frees given soundcard structure
+ * @card: soundcard structure
+ *
+ * This function releases the soundcard structure and the all assigned
+ * devices automatically. That is, you don't have to release the devices
+ * by yourself.
+ *
+ * Returns zero. Frees all associated devices and frees the control
+ * interface associated to given soundcard.
+ */
+static int snd_card_do_free(struct snd_card *card)
+{
+ struct snd_shutdown_f_ops *s_f_ops;
+
+#if defined(CONFIG_SND_MIXER_OSS) || defined(CONFIG_SND_MIXER_OSS_MODULE)
+ if (snd_mixer_oss_notify_callback)
+ snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE);
+#endif
+ if (snd_device_free_all(card, SNDRV_DEV_CMD_PRE) < 0) {
+ snd_printk(KERN_ERR "unable to free all devices (pre)\n");
+ /* Fatal, but this situation should never occur */
+ }
+ if (snd_device_free_all(card, SNDRV_DEV_CMD_NORMAL) < 0) {
+ snd_printk(KERN_ERR "unable to free all devices (normal)\n");
+ /* Fatal, but this situation should never occur */
+ }
+ if (snd_device_free_all(card, SNDRV_DEV_CMD_POST) < 0) {
+ snd_printk(KERN_ERR "unable to free all devices (post)\n");
+ /* Fatal, but this situation should never occur */
+ }
+ if (card->private_free)
+ card->private_free(card);
while (card->s_f_ops) {
s_f_ops = card->s_f_ops;
card->s_f_ops = s_f_ops->next;
kfree(s_f_ops);
}
+ kfree(card);
+ return 0;
+}
+
+static int snd_card_free_prepare(struct snd_card *card)
+{
+ if (card == NULL)
+ return -EINVAL;
+ (void) snd_card_disconnect(card);
mutex_lock(&snd_card_mutex);
+ snd_cards[card->number] = NULL;
snd_cards_lock &= ~(1 << card->number);
mutex_unlock(&snd_card_mutex);
- kfree(card);
+#ifdef CONFIG_PM
+ wake_up(&card->power_sleep);
+#endif
+ return 0;
+}
+
+int snd_card_free_when_closed(struct snd_card *card)
+{
+ int free_now = 0;
+ int ret = snd_card_free_prepare(card);
+ if (ret)
+ return ret;
+
+ spin_lock(&card->files_lock);
+ if (card->files == NULL)
+ free_now = 1;
+ else
+ card->free_on_last_close = 1;
+ spin_unlock(&card->files_lock);
+
+ if (free_now)
+ snd_card_do_free(card);
+ return 0;
+}
+
+EXPORT_SYMBOL(snd_card_free_when_closed);
+
+int snd_card_free(struct snd_card *card)
+{
+ int ret = snd_card_free_prepare(card);
+ if (ret)
+ return ret;
+
+ /* wait, until all devices are ready for the free operation */
+ wait_event(card->shutdown_sleep, card->files == NULL);
+ snd_card_do_free(card);
return 0;
}
@@ -717,6 +751,7 @@ int snd_card_file_remove(struct snd_card
int snd_card_file_remove(struct snd_card *card, struct file *file)
{
struct snd_monitor_file *mfile, *pfile = NULL;
+ int last_close = 0;
spin_lock(&card->files_lock);
mfile = card->files;
@@ -731,9 +766,14 @@ int snd_card_file_remove(struct snd_card
pfile = mfile;
mfile = mfile->next;
}
+ if (card->files == NULL)
+ last_close = 1;
spin_unlock(&card->files_lock);
- if (card->files == NULL)
+ if (last_close) {
wake_up(&card->shutdown_sleep);
+ if (card->free_on_last_close)
+ snd_card_do_free(card);
+ }
if (!mfile) {
snd_printk(KERN_ERR "ALSA card file remove problem (%p)\n", file);
return -ENOENT;
diff -r 08577d0b45ef core/oss/mixer_oss.c
--- a/core/oss/mixer_oss.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/oss/mixer_oss.c Thu Jun 15 03:02:53 2006 -0700
@@ -1313,22 +1313,20 @@ static int snd_mixer_oss_notify_handler(
card->mixer_oss = mixer;
snd_mixer_oss_build(mixer);
snd_mixer_oss_proc_init(mixer);
- } else if (cmd == SND_MIXER_OSS_NOTIFY_DISCONNECT) {
- mixer = card->mixer_oss;
- if (mixer == NULL || !mixer->oss_dev_alloc)
- return 0;
- snd_unregister_oss_device(SNDRV_OSS_DEVICE_TYPE_MIXER, mixer->card, 0);
- mixer->oss_dev_alloc = 0;
- } else { /* free */
+ } else {
mixer = card->mixer_oss;
if (mixer == NULL)
return 0;
+ if (mixer->oss_dev_alloc) {
#ifdef SNDRV_OSS_INFO_DEV_MIXERS
- snd_oss_info_unregister(SNDRV_OSS_INFO_DEV_MIXERS, mixer->card->number);
+ snd_oss_info_unregister(SNDRV_OSS_INFO_DEV_MIXERS, mixer->card->number);
#endif
- if (mixer->oss_dev_alloc)
snd_unregister_oss_device(SNDRV_OSS_DEVICE_TYPE_MIXER, mixer->card, 0);
- snd_mixer_oss_proc_done(mixer);
+ snd_mixer_oss_proc_done(mixer);
+ mixer->oss_dev_alloc = 0;
+ }
+ if (cmd == SND_MIXER_OSS_NOTIFY_DISCONNECT)
+ return 0;
return snd_mixer_oss_free1(mixer);
}
return 0;
diff -r 08577d0b45ef core/oss/pcm_oss.c
--- a/core/oss/pcm_oss.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/oss/pcm_oss.c Thu Jun 15 03:02:53 2006 -0700
@@ -2929,14 +2929,6 @@ static int snd_pcm_oss_disconnect_minor(
snd_unregister_oss_device(SNDRV_OSS_DEVICE_TYPE_PCM,
pcm->card, 1);
}
- }
- return 0;
-}
-
-static int snd_pcm_oss_unregister_minor(struct snd_pcm *pcm)
-{
- snd_pcm_oss_disconnect_minor(pcm);
- if (pcm->oss.reg) {
if (dsp_map[pcm->card->number] == (int)pcm->device) {
#ifdef SNDRV_OSS_INFO_DEV_AUDIO
snd_oss_info_unregister(SNDRV_OSS_INFO_DEV_AUDIO, pcm->card->number);
@@ -2945,6 +2937,12 @@ static int snd_pcm_oss_unregister_minor(
pcm->oss.reg = 0;
snd_pcm_oss_proc_done(pcm);
}
+ return 0;
+}
+
+static int snd_pcm_oss_unregister_minor(struct snd_pcm *pcm)
+{
+ snd_pcm_oss_disconnect_minor(pcm);
return 0;
}
diff -r 08577d0b45ef core/pcm.c
--- a/core/pcm.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/pcm.c Thu Jun 15 03:02:53 2006 -0700
@@ -968,39 +968,29 @@ static int snd_pcm_dev_register(struct s
return 0;
}
-static int snd_pcm_dev_disconnect(struct snd_device *device)
-{
- struct snd_pcm *pcm = device->device_data;
+static int snd_pcm_do_disconnect(struct snd_pcm *pcm, int notify)
+{
struct list_head *list;
struct snd_pcm_substream *substream;
- int cidx;
-
- mutex_lock(®ister_mutex);
+ int cidx, devtype;
+
+ if (list_empty(&pcm->list))
+ return 0;
+
list_del_init(&pcm->list);
for (cidx = 0; cidx < 2; cidx++)
for (substream = pcm->streams[cidx].substream; substream; substream = substream->next)
if (substream->runtime)
substream->runtime->status->state = SNDRV_PCM_STATE_DISCONNECTED;
- list_for_each(list, &snd_pcm_notify_list) {
- struct snd_pcm_notify *notify;
- notify = list_entry(list, struct snd_pcm_notify, list);
- notify->n_disconnect(pcm);
- }
- mutex_unlock(®ister_mutex);
- return 0;
-}
-
-static int snd_pcm_dev_unregister(struct snd_device *device)
-{
- int cidx, devtype;
- struct snd_pcm_substream *substream;
- struct list_head *list;
- struct snd_pcm *pcm = device->device_data;
-
- snd_assert(pcm != NULL, return -ENXIO);
- mutex_lock(®ister_mutex);
- list_del(&pcm->list);
+ if (notify) {
+ list_for_each(list, &snd_pcm_notify_list) {
+ struct snd_pcm_notify *notify;
+ notify = list_entry(list, struct snd_pcm_notify, list);
+ notify->n_disconnect(pcm);
+ }
+ }
for (cidx = 0; cidx < 2; cidx++) {
+ int err;
devtype = -1;
switch (cidx) {
case SNDRV_PCM_STREAM_PLAYBACK:
@@ -1010,10 +1000,38 @@ static int snd_pcm_dev_unregister(struct
devtype = SNDRV_DEVICE_TYPE_PCM_CAPTURE;
break;
}
- snd_unregister_device(devtype, pcm->card, pcm->device);
+ err = snd_unregister_device(devtype, pcm->card, pcm->device);
+ if (err < 0)
+ snd_printk(KERN_ERR "snd_unregister_device %d/%d = %i\n",
+ pcm->card->number, devtype, err);
+ }
+ return 0;
+}
+
+static int snd_pcm_dev_disconnect(struct snd_device *device)
+{
+ struct snd_pcm *pcm = device->device_data;
+ int err = 0;
+
+ mutex_lock(®ister_mutex);
+ err = snd_pcm_do_disconnect(pcm, 1);
+ mutex_unlock(®ister_mutex);
+ return err;
+}
+
+static int snd_pcm_dev_unregister(struct snd_device *device)
+{
+ int cidx;
+ struct snd_pcm_substream *substream;
+ struct list_head *list;
+ struct snd_pcm *pcm = device->device_data;
+
+ snd_assert(pcm != NULL, return -ENXIO);
+ mutex_lock(®ister_mutex);
+ snd_pcm_do_disconnect(pcm, 0);
+ for (cidx = 0; cidx < 2; cidx++)
for (substream = pcm->streams[cidx].substream; substream; substream = substream->next)
snd_pcm_timer_done(substream);
- }
list_for_each(list, &snd_pcm_notify_list) {
struct snd_pcm_notify *notify;
notify = list_entry(list, struct snd_pcm_notify, list);
diff -r 08577d0b45ef core/timer.c
--- a/core/timer.c Tue Jun 13 12:01:14 2006 +0200
+++ b/core/timer.c Thu Jun 15 03:02:53 2006 -0700
@@ -88,6 +88,7 @@ static int snd_timer_free(struct snd_tim
static int snd_timer_free(struct snd_timer *timer);
static int snd_timer_dev_free(struct snd_device *device);
static int snd_timer_dev_register(struct snd_device *device);
+static int snd_timer_dev_disconnect(struct snd_device *device);
static int snd_timer_dev_unregister(struct snd_device *device);
static void snd_timer_reschedule(struct snd_timer * timer, unsigned long ticks_left);
@@ -772,6 +773,7 @@ int snd_timer_new(struct snd_card *card,
static struct snd_device_ops ops = {
.dev_free = snd_timer_dev_free,
.dev_register = snd_timer_dev_register,
+ .dev_disconnect = snd_timer_dev_disconnect,
.dev_unregister = snd_timer_dev_unregister
};
@@ -884,6 +886,15 @@ static int snd_timer_unregister(struct s
list_del(&timer->device_list);
mutex_unlock(®ister_mutex);
return snd_timer_free(timer);
+}
+
+static int snd_timer_dev_disconnect(struct snd_device *device)
+{
+ struct snd_timer *timer = device->device_data;
+ mutex_lock(®ister_mutex);
+ list_del_init(&timer->device_list);
+ mutex_unlock(®ister_mutex);
+ return 0;
}
static int snd_timer_dev_unregister(struct snd_device *device)
diff -r 08577d0b45ef include/core.h
--- a/include/core.h Tue Jun 13 12:01:14 2006 +0200
+++ b/include/core.h Thu Jun 15 03:02:53 2006 -0700
@@ -131,6 +131,7 @@ struct snd_card {
state */
spinlock_t files_lock; /* lock the files for this card */
int shutdown; /* this card is going down */
+ int free_on_last_close; /* free in context of file_release */
wait_queue_head_t shutdown_sleep;
struct work_struct free_workq; /* for free in workqueue */
struct device *dev;
@@ -246,6 +247,7 @@ struct snd_card *snd_card_new(int idx, c
struct module *module, int extra_size);
int snd_card_disconnect(struct snd_card *card);
int snd_card_free(struct snd_card *card);
+int snd_card_free_when_closed(struct snd_card *card);
int snd_card_free_in_thread(struct snd_card *card);
int snd_card_register(struct snd_card *card);
int snd_card_info_init(void);
diff -r 08577d0b45ef usb/usbaudio.c
--- a/usb/usbaudio.c Tue Jun 13 12:01:14 2006 +0200
+++ b/usb/usbaudio.c Thu Jun 15 03:02:53 2006 -0700
@@ -3469,7 +3469,7 @@ static void snd_usb_audio_disconnect(str
}
usb_chip[chip->index] = NULL;
mutex_unlock(®ister_mutex);
- snd_card_free(card);
+ snd_card_free_when_closed(card);
} else {
mutex_unlock(®ister_mutex);
}
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
