Re: [Alsa-devel] Re: ALSA regression: oops on shutdown

From: Takashi Iwai
Date: Mon Apr 24 2006 - 12:34:40 EST


At Mon, 24 Apr 2006 19:30:33 +0400,
Alexey Dobriyan wrote:
>
> On Mon, Apr 24, 2006 at 04:26:10PM +0200, Takashi Iwai wrote:
> > At Mon, 24 Apr 2006 15:34:30 +0200,
> > I wrote:
> > >
> > > At Mon, 24 Apr 2006 07:51:44 +0400,
> > > Alexey Dobriyan wrote:
> > > >
> > > > On Mon, Apr 24, 2006 at 07:11:42AM +0400, Alexey Dobriyan wrote:
> > > > > On Sun, Apr 23, 2006 at 10:39:19PM -0400, Lee Revell wrote:
> > > > >
> > > > > Andrew asked about precise steps of reproduction:
> > > > >
> > > > > rmmod snd_pcm_oss
> > > > >
> > > > > which is part of Gentoo install scripts (/etc/init.d/alsasound:134-140, to
> > > > > be precise) (basically lsmod | grep snd | xargs rmmod).
> > > > >
> > > > > BUG: unable to handle kernel paging request at virtual address 6b6b6b6b
> > > > > printing eip:
> > > > > c016c6a4
> > > > > *pde = 00000000
> > > > > Oops: 0000 [#1]
> > > > > PREEMPT
> > > > > Modules linked in: snd_pcm_oss snd_mixer_oss snd_intel8x0 snd_ac97_codec snd_ac97_bus snd_pcm snd_timer snd soundcore snd_page_alloc ehci_hcd uhci_hcd usbcore
> > > > > CPU: 0
> > > > > EIP: 0060:[<c016c6a4>] Not tainted VLI
> > > > > EFLAGS: 00010286 (2.6.17-rc2-6b426e785cb81e53dc2fc4dcf997661472b470ef #1)
> > > > > EIP is at remove_proc_entry+0x2c/0x11c
> > > > > eax: 00000000 ebx: deb48b80 ecx: ffffffff edx: de11ed24
> > > > > esi: de11ed24 edi: 6b6b6b6b ebp: de4cb000 esp: de4cbf28
> > > > > ds: 007b es: 007b ss: 0068
> > > > > Process rmmod (pid: 7735, threadinfo=de4cb000 task=c1487a90)
> > > > > Stack: <0>de11ed24 6b6b6b6b deb48b80 de11ed24 bf8590cc df93603b de4f0e78 00000000
> > > > > df965fc8 de4f0d84 df967e74 df966184 de4f0d88 df95292d df967f00 00000000
> > > > > c0125f7a 00000000 5f646e73 5f6d6370 0073736f de85d4a4 b7f27000 c013789d
> > > > > Call Trace:
> > > > > [<df93603b>] snd_info_unregister+0x2e/0x44 [snd]
> > > > > [<df965fc8>] snd_pcm_oss_proc_done+0x19/0x30 [snd_pcm_oss]
> > > > > [<df966184>] snd_pcm_oss_unregister_minor+0x3b/0x3f [snd_pcm_oss]
> > > > > [<df95292d>] snd_pcm_notify+0x4a/0x91 [snd_pcm]
> > > > > [<c0125f7a>] sys_delete_module+0x113/0x138
> > > > > [<c013789d>] do_munmap+0xe2/0xef
> > > > > [<c010259b>] syscall_call+0x7/0xb
> > > > > Code: 85 d2 56 53 51 51 89 14 24 89 44 24 04 75 13 8d 4c 24 04 89 e2 e8 dd f7 ff ff 85 c0 0f 85 f2 00 00 00 8b 7c 24 04 31 c0 83 c9 ff <f2> ae f7 d1 49 b0 01 89 ce e8 fa 13 fa ff 8b 04 24 8d 58 38 83
> > > > > EIP: [<c016c6a4>] remove_proc_entry+0x2c/0x11c SS:ESP 0068:de4cbf28
> > > > > BUG: rmmod/7735, lock held at task exit time!
> > > > > [df960920] {register_mutex}
> > > > > .. held by: rmmod: 7735 [c1487a90, 118]
> > > > > ... acquired at: snd_pcm_notify+0x10/0x91 [snd_pcm]
> > > >
> > > > Other symptoms are 6 (six) "prealloc" files, 3 "oss" files and 1 "oss"
> > > > directory (according to highlighting):
> > > >
> > > > $ ls /proc/asound/
> > > > card0 devices modules oss oss prealloc prealloc prealloc timers
> > > > ^^^
> > > > directory
> > > >
> > > > cards ICH5 oss oss pcm prealloc prealloc prealloc version
> > > >
> > > > "ls -l" thinks all 4 "oss" files are files (again, according to
> > > > highlighting).
> > >
> > > That's odd. It implies that the proc file paths are flatten.
> >
> > But, not all entries seem flatten (e.g. pcm0p, sub0, etc).
> > Could you show the output of "tree /proc/asound" ?
>
> 2.6.16 (good) 2.6.17-rc2-whatever (bad)
> ---------------------------------------------------------
> /proc/asound/ /proc/asound/
> |-- ICH5 -> card0 |-- ICH5 -> card0
> |-- card0 |-- card0
> | |-- codec97#0 | |-- codec97#0
> | | |-- ac97#0-0 | | |-- ac97#0-0
> | | `-- ac97#0-0+regs | | `-- ac97#0-0+regs
> | |-- id | |-- id
> | |-- intel8x0 | |-- intel8x0
> | |-- oss_mixer | `-- oss_mixer
> | |-- pcm0c |-- cards
> | | |-- info |-- devices
> | | |-- oss |-- modules
> | | `-- sub0 |-- oss
> | | |-- hw_params |-- oss
> | | |-- info |-- oss
> | | |-- prealloc |-- oss
> | | |-- status |-- pcm
> | | `-- sw_params |-- prealloc
> | |-- pcm0p |-- prealloc
> | | |-- info |-- prealloc
> | | |-- oss |-- prealloc
> | | `-- sub0 |-- prealloc
> | | |-- hw_params |-- prealloc
> | | |-- info |-- timers
> | | |-- prealloc `-- version
> | | |-- status
> | | `-- sw_params 3 directories, 21 files
> | |-- pcm1c
> | | |-- info
> | | |-- oss
> | | `-- sub0
> | | |-- hw_params
> | | |-- info
> | | |-- prealloc
> | | |-- status
> | | `-- sw_params
> | |-- pcm2c
> | | |-- info
> | | `-- sub0
> | | |-- hw_params
> | | |-- info
> | | |-- prealloc
> | | |-- status
> | | `-- sw_params
> | |-- pcm3c
> | | |-- info
> | | `-- sub0
> | | |-- hw_params
> | | |-- info
> | | |-- prealloc
> | | |-- status
> | | `-- sw_params
> | `-- pcm4p
> | |-- info
> | `-- sub0
> | |-- hw_params
> | |-- info
> | |-- prealloc
> | |-- status
> | `-- sw_params
> |-- cards
> |-- devices
> |-- modules
> |-- oss
> | |-- devices
> | `-- sndstat
> |-- pcm
> |-- timers
> `-- version
>
> 16 directories, 52 files

Thanks. I think I got the culprit. It's a patch introducing
CONFIG_SND_VERBOSE_PROCFS option.

Try the patch below.


Takashi

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index df70e75..9796bc0 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -374,12 +374,14 @@ struct snd_pcm_substream {
/* -- OSS things -- */
struct snd_pcm_oss_substream oss;
#endif
+#ifdef CONFIG_SND_VERBOSE_PROCFS
struct snd_info_entry *proc_root;
struct snd_info_entry *proc_info_entry;
struct snd_info_entry *proc_hw_params_entry;
struct snd_info_entry *proc_sw_params_entry;
struct snd_info_entry *proc_status_entry;
struct snd_info_entry *proc_prealloc_entry;
+#endif
/* misc flags */
unsigned int no_mmap_ctrl: 1;
unsigned int hw_opened: 1;
@@ -400,12 +402,14 @@ struct snd_pcm_str {
struct snd_pcm_oss_stream oss;
#endif
struct snd_pcm_file *files;
+#ifdef CONFIG_SND_VERBOSE_PROCFS
struct snd_info_entry *proc_root;
struct snd_info_entry *proc_info_entry;
#ifdef CONFIG_SND_DEBUG
unsigned int xrun_debug; /* 0 = disabled, 1 = verbose, 2 = stacktrace */
struct snd_info_entry *proc_xrun_debug_entry;
#endif
+#endif
};

struct snd_pcm {
diff --git a/include/sound/pcm_oss.h b/include/sound/pcm_oss.h
index 39df2ba..c854647 100644
--- a/include/sound/pcm_oss.h
+++ b/include/sound/pcm_oss.h
@@ -75,7 +75,9 @@ struct snd_pcm_oss_substream {
struct snd_pcm_oss_stream {
struct snd_pcm_oss_setup *setup_list; /* setup list */
struct mutex setup_mutex;
+#ifdef CONFIG_SND_VERBOSE_PROCFS
struct snd_info_entry *proc_entry;
+#endif
};

struct snd_pcm_oss {
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 8efc1b1..44015a8 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -142,7 +142,7 @@ config SND_SUPPORT_OLD_API

config SND_VERBOSE_PROCFS
bool "Verbose procfs contents"
- depends on SND
+ depends on SND && PROC_FS
default y
help
Say Y here to include code for verbose procfs contents (provides
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index c5978d6..817b727 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2212,7 +2212,7 @@ static int snd_pcm_oss_mmap(struct file
return 0;
}

-#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_SND_VERBOSE_PROCFS
/*
* /proc interface
*/
@@ -2366,10 +2366,10 @@ static void snd_pcm_oss_proc_done(struct
}
}
}
-#else /* !CONFIG_PROC_FS */
+#else /* !CONFIG_SND_VERBOSE_PROCFS */
#define snd_pcm_oss_proc_init(pcm)
#define snd_pcm_oss_proc_done(pcm)
-#endif /* CONFIG_PROC_FS */
+#endif /* CONFIG_SND_VERBOSE_PROCFS */

/*
* ENTRY functions
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 122e10a..d8c6d7f 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -142,7 +142,7 @@ static int snd_pcm_control_ioctl(struct
return -ENOIOCTLCMD;
}

-#if defined(CONFIG_PROC_FS) && defined(CONFIG_SND_VERBOSE_PROCFS)
+#ifdef CONFIG_SND_VERBOSE_PROCFS

#define STATE(v) [SNDRV_PCM_STATE_##v] = #v
#define STREAM(v) [SNDRV_PCM_STREAM_##v] = #v
@@ -599,12 +599,12 @@ static int snd_pcm_substream_proc_done(s
}
return 0;
}
-#else /* !CONFIG_PROC_FS */
+#else /* !CONFIG_SND_VERBOSE_PROCFS */
static inline int snd_pcm_stream_proc_init(struct snd_pcm_str *pstr) { return 0; }
static inline int snd_pcm_stream_proc_done(struct snd_pcm_str *pstr) { return 0; }
static inline int snd_pcm_substream_proc_init(struct snd_pcm_substream *substream) { return 0; }
static inline int snd_pcm_substream_proc_done(struct snd_pcm_substream *substream) { return 0; }
-#endif /* CONFIG_PROC_FS */
+#endif /* CONFIG_SND_VERBOSE_PROCFS */

/**
* snd_pcm_new_stream - create a new PCM stream
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 230a940..91fe5d0 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -130,7 +130,7 @@ void snd_pcm_playback_silence(struct snd
static void xrun(struct snd_pcm_substream *substream)
{
snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
-#ifdef CONFIG_SND_DEBUG
+#if defined(CONFIG_SND_DEBUG) && defined(CONFIG_SND_VERBOSE_PROCFS)
if (substream->pstr->xrun_debug) {
snd_printd(KERN_DEBUG "XRUN: pcmC%dD%d%c\n",
substream->pcm->card->number,
@@ -204,7 +204,7 @@ static inline int snd_pcm_update_hw_ptr_
delta = hw_ptr_interrupt - new_hw_ptr;
if (delta > 0) {
if ((snd_pcm_uframes_t)delta < runtime->buffer_size / 2) {
-#ifdef CONFIG_SND_DEBUG
+#if defined(CONFIG_SND_DEBUG) && defined(CONFIG_SND_VERBOSE_PROCFS)
if (runtime->periods > 1 && substream->pstr->xrun_debug) {
snd_printd(KERN_ERR "Unexpected hw_pointer value [1] (stream = %i, delta: -%ld, max jitter = %ld): wrong interrupt acknowledge?\n", substream->stream, (long) delta, runtime->buffer_size / 2);
if (substream->pstr->xrun_debug > 1)
@@ -249,7 +249,7 @@ int snd_pcm_update_hw_ptr(struct snd_pcm
delta = old_hw_ptr - new_hw_ptr;
if (delta > 0) {
if ((snd_pcm_uframes_t)delta < runtime->buffer_size / 2) {
-#ifdef CONFIG_SND_DEBUG
+#if defined(CONFIG_SND_DEBUG) && defined(CONFIG_SND_VERBOSE_PROCFS)
if (runtime->periods > 2 && substream->pstr->xrun_debug) {
snd_printd(KERN_ERR "Unexpected hw_pointer value [2] (stream = %i, delta: -%ld, max jitter = %ld): wrong interrupt acknowledge?\n", substream->stream, (long) delta, runtime->buffer_size / 2);
if (substream->pstr->xrun_debug > 1)
diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index a0119ae..428f8c1 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -100,8 +100,10 @@ static void snd_pcm_lib_preallocate_dma_
int snd_pcm_lib_preallocate_free(struct snd_pcm_substream *substream)
{
snd_pcm_lib_preallocate_dma_free(substream);
+#ifdef CONFIG_SND_VERBOSE_PROCFS
snd_info_unregister(substream->proc_prealloc_entry);
substream->proc_prealloc_entry = NULL;
+#endif
return 0;
}

@@ -124,7 +126,7 @@ int snd_pcm_lib_preallocate_free_for_all
return 0;
}

-#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_SND_VERBOSE_PROCFS
/*
* read callback for prealloc proc file
*
@@ -203,9 +205,9 @@ static inline void preallocate_info_init
substream->proc_prealloc_entry = entry;
}

-#else /* !CONFIG_PROC_FS */
+#else /* !CONFIG_SND_VERBOSE_PROCFS */
#define preallocate_info_init(s)
-#endif
+#endif /* CONFIG_SND_VERBOSE_PROCFS */

/*
* pre-allocate the buffer and create a proc file for the substream
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/