Re: 2.6.17-rc4-mm1

[Takashi Iwai <tiwai@suse.de>]

At Mon, 15 May 2006 19:30:24 +0200,
I wrote:
> 
> At Mon, 15 May 2006 10:04:29 -0700,
> Andrew Morton wrote:
> > 
> > "Michal Piotrowski" <michal.k.k.piotrowski@gmail.com> wrote:
> > >
> > > Hi,
> > > 
> > > On 15/05/06, Andrew Morton <akpm@osdl.org> wrote:
> > > >
> > > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17-rc4/2.6.17-rc4-mm1/
> > > >
> > > > - This tree contains a large number of new bugs^H^H^H^Hpatches.
> > > [snip]
> > > >  git-alsa.patch
> > > 
> > > BUG: sleeping function called from invalid context at
> > > /usr/src/linux-mm/sound/core/info.c:117
> > > in_atomic():1, irqs_disabled():0
> > >  <c1003ef9> show_trace+0xd/0xf   <c100440c> dump_stack+0x17/0x19
> > >   <c10178ce> __might_sleep+0x93/0x9d   <f988eeb5> snd_iprintf+0x1b/0x84 [snd]
> > >   <f988d808> snd_card_module_info_read+0x34/0x4e [snd]   <f988f197>
> > > snd_info_entry_open+0x20f/0x2cc [snd]
> > >  <c1067a17> __dentry_open+0x133/0x260   <c1067bb7> nameidata_to_filp+0x1c/0x2e
> > >  <c1067bf7> do_filp_open+0x2e/0x35   <c1068bf2> do_sys_open+0x54/0xd7
> > >  <c1068ca1> sys_open+0x16/0x18   <c11dab67> sysenter_past_esp+0x54/0x75
> > > Non-volatile memory driver v1.2
> > > 
> > > Here is dmesg http://www.stardust.webpages.pl/files/mm/2.6.17-rc4-mm1/mm-dmesg
> > > Here is config http://www.stardust.webpages.pl/files/mm/2.6.17-rc4-mm1/mm-config
> > 
> > heh, yes, Takashi baited his hook and pulled in a big one.
> > 
> > snd_card_module_info_read() calls snd_iprintf() under read_lock()..
> 
> Ouch, I checked only spinlocks but forgot rwlocks.  Will fix them.

The patch below should fix them (already merged in ALSA HG repo).


[ALSA] Fix rwlock around snd_iprintf() in sound core

Fixed rwlock around snd_iprintf() in sound core part.
Replaced with mutex.

Also, make mutex and flags static variables with addition of
snd_card_locked() function (just for sound.c).

Signed-off-by: Takashi Iwai <tiwai@suse.de>

---
commit f4e4b6a7b3b404edb1acd72ae3cb26e6246b04bb
tree 6e529ed077718299bf7dff400cedc62767d7bdbe
parent 3a5be5e3bc2d702b9f8428465729895b691c9059
author Takashi Iwai <tiwai@suse.de> Mon, 15 May 2006 00:00:00 +0200
committer Takashi Iwai <tiwai@suse.de> Mon, 15 May 2006 19:51:18 +0200

 include/sound/core.h |    3 +--
 sound/core/init.c    |   51 ++++++++++++++++++++++++++++++--------------------
 sound/core/sound.c   |    7 +------
 3 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index 5135147..5d184be 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -233,9 +233,8 @@ int copy_from_user_toio(volatile void __
 
 /* init.c */
 
-extern unsigned int snd_cards_lock;
 extern struct snd_card *snd_cards[SNDRV_CARDS];
-extern rwlock_t snd_card_rwlock;
+int snd_card_locked(int card);
 #if defined(CONFIG_SND_MIXER_OSS) || defined(CONFIG_SND_MIXER_OSS_MODULE)
 #define SND_MIXER_OSS_NOTIFY_REGISTER	0
 #define SND_MIXER_OSS_NOTIFY_DISCONNECT	1
diff --git a/sound/core/init.c b/sound/core/init.c
index 2ff0e5e..38b2d4a 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -38,11 +38,11 @@ struct snd_shutdown_f_ops {
 	struct snd_shutdown_f_ops *next;
 };
 
-unsigned int snd_cards_lock = 0;	/* locked for registering/using */
+static unsigned int snd_cards_lock = 0;	/* locked for registering/using */
 struct snd_card *snd_cards[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = NULL};
 EXPORT_SYMBOL(snd_cards);
 
-DEFINE_RWLOCK(snd_card_rwlock);
+static DEFINE_MUTEX(snd_card_mutex);
 
 #if defined(CONFIG_SND_MIXER_OSS) || defined(CONFIG_SND_MIXER_OSS_MODULE)
 int (*snd_mixer_oss_notify_callback)(struct snd_card *card, int free_flag);
@@ -112,7 +112,7 @@ struct snd_card *snd_card_new(int idx, c
 		strlcpy(card->id, xid, sizeof(card->id));
 	}
 	err = 0;
-	write_lock(&snd_card_rwlock);
+	mutex_lock(&snd_card_mutex);
 	if (idx < 0) {
 		int idx2;
 		for (idx2 = 0; idx2 < SNDRV_CARDS; idx2++)
@@ -130,12 +130,12 @@ struct snd_card *snd_card_new(int idx, c
 	else
 		err = -ENODEV;
 	if (idx < 0 || err < 0) {
-		write_unlock(&snd_card_rwlock);
+		mutex_unlock(&snd_card_mutex);
 		snd_printk(KERN_ERR "cannot find the slot for index %d (range 0-%i)\n", idx, snd_ecards_limit - 1);
 		goto __error;
 	}
 	snd_cards_lock |= 1 << idx;		/* lock it */
-	write_unlock(&snd_card_rwlock);
+	mutex_unlock(&snd_card_mutex);
 	card->number = idx;
 	card->module = module;
 	INIT_LIST_HEAD(&card->devices);
@@ -173,6 +173,17 @@ struct snd_card *snd_card_new(int idx, c
 
 EXPORT_SYMBOL(snd_card_new);
 
+/* return non-zero if a card is already locked */
+int snd_card_locked(int card)
+{
+	int locked;
+
+	mutex_lock(&snd_card_mutex);
+	locked = snd_cards_lock & (1 << card);
+	mutex_unlock(&snd_card_mutex);
+	return locked;
+}
+
 static loff_t snd_disconnect_llseek(struct file *file, loff_t offset, int orig)
 {
 	return -ENODEV;
@@ -240,9 +251,9 @@ int snd_card_disconnect(struct snd_card 
 	spin_unlock(&card->files_lock);
 
 	/* phase 1: disable fops (user space) operations for ALSA API */
-	write_lock(&snd_card_rwlock);
+	mutex_lock(&snd_card_mutex);
 	snd_cards[card->number] = NULL;
-	write_unlock(&snd_card_rwlock);
+	mutex_unlock(&snd_card_mutex);
 	
 	/* phase 2: replace file->f_op with special dummy operations */
 	
@@ -321,9 +332,9 @@ int snd_card_free(struct snd_card *card)
 
 	if (card == NULL)
 		return -EINVAL;
-	write_lock(&snd_card_rwlock);
+	mutex_lock(&snd_card_mutex);
 	snd_cards[card->number] = NULL;
-	write_unlock(&snd_card_rwlock);
+	mutex_unlock(&snd_card_mutex);
 
 #ifdef CONFIG_PM
 	wake_up(&card->power_sleep);
@@ -359,9 +370,9 @@ int snd_card_free(struct snd_card *card)
 		card->s_f_ops = s_f_ops->next;
 		kfree(s_f_ops);
 	}
-	write_lock(&snd_card_rwlock);
+	mutex_lock(&snd_card_mutex);
 	snd_cards_lock &= ~(1 << card->number);
-	write_unlock(&snd_card_rwlock);
+	mutex_unlock(&snd_card_mutex);
 	kfree(card);
 	return 0;
 }
@@ -497,16 +508,16 @@ int snd_card_register(struct snd_card *c
 	snd_assert(card != NULL, return -EINVAL);
 	if ((err = snd_device_register_all(card)) < 0)
 		return err;
-	write_lock(&snd_card_rwlock);
+	mutex_lock(&snd_card_mutex);
 	if (snd_cards[card->number]) {
 		/* already registered */
-		write_unlock(&snd_card_rwlock);
+		mutex_unlock(&snd_card_mutex);
 		return 0;
 	}
 	if (card->id[0] == '\0')
 		choose_default_id(card);
 	snd_cards[card->number] = card;
-	write_unlock(&snd_card_rwlock);
+	mutex_unlock(&snd_card_mutex);
 	init_info_for_card(card);
 #if defined(CONFIG_SND_MIXER_OSS) || defined(CONFIG_SND_MIXER_OSS_MODULE)
 	if (snd_mixer_oss_notify_callback)
@@ -527,7 +538,7 @@ static void snd_card_info_read(struct sn
 	struct snd_card *card;
 
 	for (idx = count = 0; idx < SNDRV_CARDS; idx++) {
-		read_lock(&snd_card_rwlock);
+		mutex_lock(&snd_card_mutex);
 		if ((card = snd_cards[idx]) != NULL) {
 			count++;
 			snd_iprintf(buffer, "%2i [%-15s]: %s - %s\n",
@@ -538,7 +549,7 @@ static void snd_card_info_read(struct sn
 			snd_iprintf(buffer, "                      %s\n",
 					card->longname);
 		}
-		read_unlock(&snd_card_rwlock);
+		mutex_unlock(&snd_card_mutex);
 	}
 	if (!count)
 		snd_iprintf(buffer, "--- no soundcards ---\n");
@@ -552,12 +563,12 @@ void snd_card_info_read_oss(struct snd_i
 	struct snd_card *card;
 
 	for (idx = count = 0; idx < SNDRV_CARDS; idx++) {
-		read_lock(&snd_card_rwlock);
+		mutex_lock(&snd_card_mutex);
 		if ((card = snd_cards[idx]) != NULL) {
 			count++;
 			snd_iprintf(buffer, "%s\n", card->longname);
 		}
-		read_unlock(&snd_card_rwlock);
+		mutex_unlock(&snd_card_mutex);
 	}
 	if (!count) {
 		snd_iprintf(buffer, "--- no soundcards ---\n");
@@ -575,11 +586,11 @@ static void snd_card_module_info_read(st
 	struct snd_card *card;
 
 	for (idx = 0; idx < SNDRV_CARDS; idx++) {
-		read_lock(&snd_card_rwlock);
+		mutex_lock(&snd_card_mutex);
 		if ((card = snd_cards[idx]) != NULL)
 			snd_iprintf(buffer, "%2i %s\n",
 				    idx, card->module->name);
-		read_unlock(&snd_card_rwlock);
+		mutex_unlock(&snd_card_mutex);
 	}
 }
 #endif
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 8313f97..02c8cc4 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -81,14 +81,9 @@ extern struct class *sound_class;
  */
 void snd_request_card(int card)
 {
-	int locked;
-
 	if (! current->fs->root)
 		return;
-	read_lock(&snd_card_rwlock);
-	locked = snd_cards_lock & (1 << card);
-	read_unlock(&snd_card_rwlock);
-	if (locked)
+	if (snd_card_locked(card))
 		return;
 	if (card < 0 || card >= cards_limit)
 		return;