* snd-ctxfi @ 2009-05-30 3:22 The Source 2009-05-30 7:00 ` snd-ctxfi Takashi Iwai 0 siblings, 1 reply; 8+ messages in thread From: The Source @ 2009-05-30 3:22 UTC (permalink / raw) To: alsa-devel Hello. I tested the snd-ctxfi driver and found no problems (except no support for 5.1 and external console, but original ctxfi does not support them either). A minor problem: kernel oops reporter periodically pops up with message about using a timer in a wrong place. I'll post here this message when I get it next time. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-05-30 3:22 snd-ctxfi The Source @ 2009-05-30 7:00 ` Takashi Iwai 2009-05-30 16:22 ` snd-ctxfi The Source 0 siblings, 1 reply; 8+ messages in thread From: Takashi Iwai @ 2009-05-30 7:00 UTC (permalink / raw) To: The Source; +Cc: alsa-devel At Sat, 30 May 2009 07:22:43 +0400, The Source wrote: > > Hello. I tested the snd-ctxfi driver and found no problems (except no > support for 5.1 and external console, but original ctxfi does not > support them either). A minor problem: kernel oops reporter periodically > pops up with message about using a timer in a wrong place. Oh, that's bad. > I'll post > here this message when I get it next time. Yes, please, that'll be helpful. thanks, Takashi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-05-30 7:00 ` snd-ctxfi Takashi Iwai @ 2009-05-30 16:22 ` The Source 2009-06-01 9:10 ` snd-ctxfi Takashi Iwai 0 siblings, 1 reply; 8+ messages in thread From: The Source @ 2009-05-30 16:22 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On 30.05.2009 11:00, Takashi Iwai wrote: > At Sat, 30 May 2009 07:22:43 +0400, > The Source wrote: > >> Hello. I tested the snd-ctxfi driver and found no problems (except no >> support for 5.1 and external console, but original ctxfi does not >> support them either). A minor problem: kernel oops reporter periodically >> pops up with message about using a timer in a wrong place. >> > Oh, that's bad. > > >> I'll post >> here this message when I get it next time. >> > Yes, please, that'll be helpful. > > > thanks, > > Takashi > > Here's the message: BUG: sleeping function called from invalid context at mm/slub.c:1599 in_atomic(): 0, irqs_disabled(): 1, pid: 32065, name: xine Pid: 32065, comm: xine Tainted: P 2.6.29.4-75.fc10.x86_64 #1 Call Trace: [<ffffffff81040685>] __might_sleep+0x105/0x10a [<ffffffff810c9fae>] kmem_cache_alloc+0x32/0xe2 [<ffffffffa08e3110>] ct_vm_map+0xfa/0x19e [snd_ctxfi] [<ffffffffa08e1a07>] ct_map_audio_buffer+0x4c/0x76 [snd_ctxfi] [<ffffffffa08e2aa5>] atc_pcm_playback_prepare+0x1d7/0x2a8 [snd_ctxfi] [<ffffffff8105ef3f>] ? up_read+0x9/0xb [<ffffffff81186b61>] ? __up_read+0x7c/0x87 [<ffffffffa08e36a6>] ct_pcm_playback_prepare+0x39/0x60 [snd_ctxfi] [<ffffffffa0886bcb>] snd_pcm_do_prepare+0x16/0x28 [snd_pcm] [<ffffffffa08867c7>] snd_pcm_action_single+0x2d/0x5b [snd_pcm] [<ffffffffa08881f3>] snd_pcm_action_nonatomic+0x52/0x6a [snd_pcm] [<ffffffffa088a723>] snd_pcm_common_ioctl1+0x404/0xc79 [snd_pcm] [<ffffffff810c52c8>] ? alloc_pages_current+0xb9/0xc2 [<ffffffff810c9402>] ? new_slab+0x1a5/0x1cb [<ffffffff810ab9ea>] ? vma_prio_tree_insert+0x23/0xc1 [<ffffffffa088b411>] snd_pcm_playback_ioctl1+0x213/0x230 [snd_pcm] [<ffffffff810b6c20>] ? mmap_region+0x397/0x4c9 [<ffffffffa088bd9b>] snd_pcm_playback_ioctl+0x2e/0x36 [snd_pcm] [<ffffffff810ddc64>] vfs_ioctl+0x2a/0x78 [<ffffffff810de130>] do_vfs_ioctl+0x462/0x4a2 [<ffffffff81029cef>] ? default_spin_lock_flags+0x9/0xe [<ffffffff81374647>] ? trace_hardirqs_off_thunk+0x3a/0x6c [<ffffffff810de1c5>] sys_ioctl+0x55/0x77 [<ffffffff8101133a>] system_call_fastpath+0x16/0x1b ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-05-30 16:22 ` snd-ctxfi The Source @ 2009-06-01 9:10 ` Takashi Iwai 2009-06-02 1:47 ` snd-ctxfi Lee Revell 0 siblings, 1 reply; 8+ messages in thread From: Takashi Iwai @ 2009-06-01 9:10 UTC (permalink / raw) To: The Source; +Cc: alsa-devel At Sat, 30 May 2009 20:22:46 +0400, The Source wrote: > > On 30.05.2009 11:00, Takashi Iwai wrote: > > At Sat, 30 May 2009 07:22:43 +0400, > > The Source wrote: > > > >> Hello. I tested the snd-ctxfi driver and found no problems (except no > >> support for 5.1 and external console, but original ctxfi does not > >> support them either). A minor problem: kernel oops reporter periodically > >> pops up with message about using a timer in a wrong place. > >> > > Oh, that's bad. > > > > > >> I'll post > >> here this message when I get it next time. > >> > > Yes, please, that'll be helpful. > > > > > > thanks, > > > > Takashi > > > > > Here's the message: > > BUG: sleeping function called from invalid context at mm/slub.c:1599 > in_atomic(): 0, irqs_disabled(): 1, pid: 32065, name: xine > Pid: 32065, comm: xine Tainted: P 2.6.29.4-75.fc10.x86_64 #1 > Call Trace: > [<ffffffff81040685>] __might_sleep+0x105/0x10a > [<ffffffff810c9fae>] kmem_cache_alloc+0x32/0xe2 > [<ffffffffa08e3110>] ct_vm_map+0xfa/0x19e [snd_ctxfi] > [<ffffffffa08e1a07>] ct_map_audio_buffer+0x4c/0x76 [snd_ctxfi] > [<ffffffffa08e2aa5>] atc_pcm_playback_prepare+0x1d7/0x2a8 [snd_ctxfi] > [<ffffffff8105ef3f>] ? up_read+0x9/0xb > [<ffffffff81186b61>] ? __up_read+0x7c/0x87 > [<ffffffffa08e36a6>] ct_pcm_playback_prepare+0x39/0x60 [snd_ctxfi] > [<ffffffffa0886bcb>] snd_pcm_do_prepare+0x16/0x28 [snd_pcm] > [<ffffffffa08867c7>] snd_pcm_action_single+0x2d/0x5b [snd_pcm] > [<ffffffffa08881f3>] snd_pcm_action_nonatomic+0x52/0x6a [snd_pcm] > [<ffffffffa088a723>] snd_pcm_common_ioctl1+0x404/0xc79 [snd_pcm] > [<ffffffff810c52c8>] ? alloc_pages_current+0xb9/0xc2 > [<ffffffff810c9402>] ? new_slab+0x1a5/0x1cb > [<ffffffff810ab9ea>] ? vma_prio_tree_insert+0x23/0xc1 > [<ffffffffa088b411>] snd_pcm_playback_ioctl1+0x213/0x230 [snd_pcm] > [<ffffffff810b6c20>] ? mmap_region+0x397/0x4c9 > [<ffffffffa088bd9b>] snd_pcm_playback_ioctl+0x2e/0x36 [snd_pcm] > [<ffffffff810ddc64>] vfs_ioctl+0x2a/0x78 > [<ffffffff810de130>] do_vfs_ioctl+0x462/0x4a2 > [<ffffffff81029cef>] ? default_spin_lock_flags+0x9/0xe > [<ffffffff81374647>] ? trace_hardirqs_off_thunk+0x3a/0x6c > [<ffffffff810de1c5>] sys_ioctl+0x55/0x77 > [<ffffffff8101133a>] system_call_fastpath+0x16/0x1b Thanks. So, something wrong in the mmap handling. I'll take a look later in this week. Takashi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-06-01 9:10 ` snd-ctxfi Takashi Iwai @ 2009-06-02 1:47 ` Lee Revell 2009-06-02 6:09 ` snd-ctxfi Takashi Iwai 0 siblings, 1 reply; 8+ messages in thread From: Lee Revell @ 2009-06-02 1:47 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, The Source On Mon, Jun 1, 2009 at 5:10 AM, Takashi Iwai <tiwai@suse.de> wrote: > Thanks. So, something wrong in the mmap handling. > I'll take a look later in this week. Looks like ct_map_audio_buffer calls ct_vm_map under spinlock; ct_vm_map calls get_vm_block which calls kzalloc with GFP_KERNEL. Lee ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-06-02 1:47 ` snd-ctxfi Lee Revell @ 2009-06-02 6:09 ` Takashi Iwai 2009-06-04 3:48 ` snd-ctxfi The Source 0 siblings, 1 reply; 8+ messages in thread From: Takashi Iwai @ 2009-06-02 6:09 UTC (permalink / raw) To: Lee Revell; +Cc: alsa-devel, The Source At Mon, 1 Jun 2009 21:47:02 -0400, Lee Revell wrote: > > On Mon, Jun 1, 2009 at 5:10 AM, Takashi Iwai <tiwai@suse.de> wrote: > > Thanks. So, something wrong in the mmap handling. > > I'll take a look later in this week. > > Looks like ct_map_audio_buffer calls ct_vm_map under spinlock; > ct_vm_map calls get_vm_block which calls kzalloc with GFP_KERNEL. Indeed. Does the patch below fix the problem? thanks, Takashi --- diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c index ead104e..1a4bb35 100644 --- a/sound/pci/ctxfi/ctatc.c +++ b/sound/pci/ctxfi/ctatc.c @@ -119,7 +119,6 @@ atc_pcm_release_resources(struct ct_atc *atc, struct ct_atc_pcm *apcm); static int ct_map_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm *apcm) { - unsigned long flags; struct snd_pcm_runtime *runtime; struct ct_vm *vm; @@ -129,9 +128,7 @@ static int ct_map_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm *apcm) runtime = apcm->substream->runtime; vm = atc->vm; - spin_lock_irqsave(&atc->vm_lock, flags); apcm->vm_block = vm->map(vm, runtime->dma_area, runtime->dma_bytes); - spin_unlock_irqrestore(&atc->vm_lock, flags); if (NULL == apcm->vm_block) return -ENOENT; @@ -141,7 +138,6 @@ static int ct_map_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm *apcm) static void ct_unmap_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm *apcm) { - unsigned long flags; struct ct_vm *vm; if (NULL == apcm->vm_block) @@ -149,9 +145,7 @@ static void ct_unmap_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm *apcm) vm = atc->vm; - spin_lock_irqsave(&atc->vm_lock, flags); vm->unmap(vm, apcm->vm_block); - spin_unlock_irqrestore(&atc->vm_lock, flags); apcm->vm_block = NULL; } @@ -161,9 +155,7 @@ static unsigned long atc_get_ptp_phys(struct ct_atc *atc, int index) struct ct_vm *vm; void *kvirt_addr; unsigned long phys_addr; - unsigned long flags; - spin_lock_irqsave(&atc->vm_lock, flags); vm = atc->vm; kvirt_addr = vm->get_ptp_virt(vm, index); if (kvirt_addr == NULL) @@ -171,8 +163,6 @@ static unsigned long atc_get_ptp_phys(struct ct_atc *atc, int index) else phys_addr = virt_to_phys(kvirt_addr); - spin_unlock_irqrestore(&atc->vm_lock, flags); - return phys_addr; } @@ -1562,7 +1552,6 @@ int ct_atc_create(struct snd_card *card, struct pci_dev *pci, atc_set_ops(atc); spin_lock_init(&atc->atc_lock); - spin_lock_init(&atc->vm_lock); /* Find card model */ err = atc_identify_card(atc); diff --git a/sound/pci/ctxfi/ctatc.h b/sound/pci/ctxfi/ctatc.h index 286c993..a7b0ec2 100644 --- a/sound/pci/ctxfi/ctatc.h +++ b/sound/pci/ctxfi/ctatc.h @@ -101,7 +101,6 @@ struct ct_atc { unsigned long (*get_ptp_phys)(struct ct_atc *atc, int index); spinlock_t atc_lock; - spinlock_t vm_lock; int (*pcm_playback_prepare)(struct ct_atc *atc, struct ct_atc_pcm *apcm); diff --git a/sound/pci/ctxfi/ctvmem.c b/sound/pci/ctxfi/ctvmem.c index cecf77e..363b67e 100644 --- a/sound/pci/ctxfi/ctvmem.c +++ b/sound/pci/ctxfi/ctvmem.c @@ -35,25 +35,27 @@ get_vm_block(struct ct_vm *vm, unsigned int size) struct ct_vm_block *block = NULL, *entry = NULL; struct list_head *pos = NULL; + mutex_lock(&vm->lock); list_for_each(pos, &vm->unused) { entry = list_entry(pos, struct ct_vm_block, list); if (entry->size >= size) break; /* found a block that is big enough */ } if (pos == &vm->unused) - return NULL; + goto out; if (entry->size == size) { /* Move the vm node from unused list to used list directly */ list_del(&entry->list); list_add(&entry->list, &vm->used); vm->size -= size; - return entry; + block = entry; + goto out; } block = kzalloc(sizeof(*block), GFP_KERNEL); if (NULL == block) - return NULL; + goto out; block->addr = entry->addr; block->size = size; @@ -62,6 +64,8 @@ get_vm_block(struct ct_vm *vm, unsigned int size) entry->size -= size; vm->size -= size; + out: + mutex_unlock(&vm->lock); return block; } @@ -70,6 +74,7 @@ static void put_vm_block(struct ct_vm *vm, struct ct_vm_block *block) struct ct_vm_block *entry = NULL, *pre_ent = NULL; struct list_head *pos = NULL, *pre = NULL; + mutex_lock(&vm->lock); list_del(&block->list); vm->size += block->size; @@ -106,6 +111,7 @@ static void put_vm_block(struct ct_vm *vm, struct ct_vm_block *block) pos = pre; pre = pos->prev; } + mutex_unlock(&vm->lock); } /* Map host addr (kmalloced/vmalloced) to device logical addr. */ @@ -191,6 +197,8 @@ int ct_vm_create(struct ct_vm **rvm) if (NULL == vm) return -ENOMEM; + mutex_init(&vm->lock); + /* Allocate page table pages */ for (i = 0; i < CT_PTP_NUM; i++) { vm->ptp[i] = kmalloc(PAGE_SIZE, GFP_KERNEL); diff --git a/sound/pci/ctxfi/ctvmem.h b/sound/pci/ctxfi/ctvmem.h index 4eb5bdd..618952e 100644 --- a/sound/pci/ctxfi/ctvmem.h +++ b/sound/pci/ctxfi/ctvmem.h @@ -20,7 +20,7 @@ #define CT_PTP_NUM 1 /* num of device page table pages */ -#include <linux/spinlock.h> +#include <linux/mutex.h> #include <linux/list.h> struct ct_vm_block { @@ -35,6 +35,7 @@ struct ct_vm { unsigned int size; /* Available addr space in bytes */ struct list_head unused; /* List of unused blocks */ struct list_head used; /* List of used blocks */ + struct mutex lock; /* Map host addr (kmalloced/vmalloced) to device logical addr. */ struct ct_vm_block *(*map)(struct ct_vm *, void *host_addr, int size); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-06-02 6:09 ` snd-ctxfi Takashi Iwai @ 2009-06-04 3:48 ` The Source 2009-06-04 5:29 ` snd-ctxfi Takashi Iwai 0 siblings, 1 reply; 8+ messages in thread From: The Source @ 2009-06-04 3:48 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel 2009/6/2 Takashi Iwai <tiwai@suse.de> > At Mon, 1 Jun 2009 21:47:02 -0400, > Lee Revell wrote: > > > > On Mon, Jun 1, 2009 at 5:10 AM, Takashi Iwai <tiwai@suse.de> wrote: > > > Thanks. So, something wrong in the mmap handling. > > > I'll take a look later in this week. > > > > Looks like ct_map_audio_buffer calls ct_vm_map under spinlock; > > ct_vm_map calls get_vm_block which calls kzalloc with GFP_KERNEL. > > Indeed. Does the patch below fix the problem? > > > thanks, > > Takashi > > --- > diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c > index ead104e..1a4bb35 100644 > --- a/sound/pci/ctxfi/ctatc.c > +++ b/sound/pci/ctxfi/ctatc.c > @@ -119,7 +119,6 @@ atc_pcm_release_resources(struct ct_atc *atc, struct > ct_atc_pcm *apcm); > > static int ct_map_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm > *apcm) > { > - unsigned long flags; > struct snd_pcm_runtime *runtime; > struct ct_vm *vm; > > @@ -129,9 +128,7 @@ static int ct_map_audio_buffer(struct ct_atc *atc, > struct ct_atc_pcm *apcm) > runtime = apcm->substream->runtime; > vm = atc->vm; > > - spin_lock_irqsave(&atc->vm_lock, flags); > apcm->vm_block = vm->map(vm, runtime->dma_area, runtime->dma_bytes); > - spin_unlock_irqrestore(&atc->vm_lock, flags); > > if (NULL == apcm->vm_block) > return -ENOENT; > @@ -141,7 +138,6 @@ static int ct_map_audio_buffer(struct ct_atc *atc, > struct ct_atc_pcm *apcm) > > static void ct_unmap_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm > *apcm) > { > - unsigned long flags; > struct ct_vm *vm; > > if (NULL == apcm->vm_block) > @@ -149,9 +145,7 @@ static void ct_unmap_audio_buffer(struct ct_atc *atc, > struct ct_atc_pcm *apcm) > > vm = atc->vm; > > - spin_lock_irqsave(&atc->vm_lock, flags); > vm->unmap(vm, apcm->vm_block); > - spin_unlock_irqrestore(&atc->vm_lock, flags); > > apcm->vm_block = NULL; > } > @@ -161,9 +155,7 @@ static unsigned long atc_get_ptp_phys(struct ct_atc > *atc, int index) > struct ct_vm *vm; > void *kvirt_addr; > unsigned long phys_addr; > - unsigned long flags; > > - spin_lock_irqsave(&atc->vm_lock, flags); > vm = atc->vm; > kvirt_addr = vm->get_ptp_virt(vm, index); > if (kvirt_addr == NULL) > @@ -171,8 +163,6 @@ static unsigned long atc_get_ptp_phys(struct ct_atc > *atc, int index) > else > phys_addr = virt_to_phys(kvirt_addr); > > - spin_unlock_irqrestore(&atc->vm_lock, flags); > - > return phys_addr; > } > > @@ -1562,7 +1552,6 @@ int ct_atc_create(struct snd_card *card, struct > pci_dev *pci, > atc_set_ops(atc); > > spin_lock_init(&atc->atc_lock); > - spin_lock_init(&atc->vm_lock); > > /* Find card model */ > err = atc_identify_card(atc); > diff --git a/sound/pci/ctxfi/ctatc.h b/sound/pci/ctxfi/ctatc.h > index 286c993..a7b0ec2 100644 > --- a/sound/pci/ctxfi/ctatc.h > +++ b/sound/pci/ctxfi/ctatc.h > @@ -101,7 +101,6 @@ struct ct_atc { > unsigned long (*get_ptp_phys)(struct ct_atc *atc, int index); > > spinlock_t atc_lock; > - spinlock_t vm_lock; > > int (*pcm_playback_prepare)(struct ct_atc *atc, > struct ct_atc_pcm *apcm); > diff --git a/sound/pci/ctxfi/ctvmem.c b/sound/pci/ctxfi/ctvmem.c > index cecf77e..363b67e 100644 > --- a/sound/pci/ctxfi/ctvmem.c > +++ b/sound/pci/ctxfi/ctvmem.c > @@ -35,25 +35,27 @@ get_vm_block(struct ct_vm *vm, unsigned int size) > struct ct_vm_block *block = NULL, *entry = NULL; > struct list_head *pos = NULL; > > + mutex_lock(&vm->lock); > list_for_each(pos, &vm->unused) { > entry = list_entry(pos, struct ct_vm_block, list); > if (entry->size >= size) > break; /* found a block that is big enough */ > } > if (pos == &vm->unused) > - return NULL; > + goto out; > > if (entry->size == size) { > /* Move the vm node from unused list to used list directly > */ > list_del(&entry->list); > list_add(&entry->list, &vm->used); > vm->size -= size; > - return entry; > + block = entry; > + goto out; > } > > block = kzalloc(sizeof(*block), GFP_KERNEL); > if (NULL == block) > - return NULL; > + goto out; > > block->addr = entry->addr; > block->size = size; > @@ -62,6 +64,8 @@ get_vm_block(struct ct_vm *vm, unsigned int size) > entry->size -= size; > vm->size -= size; > > + out: > + mutex_unlock(&vm->lock); > return block; > } > > @@ -70,6 +74,7 @@ static void put_vm_block(struct ct_vm *vm, struct > ct_vm_block *block) > struct ct_vm_block *entry = NULL, *pre_ent = NULL; > struct list_head *pos = NULL, *pre = NULL; > > + mutex_lock(&vm->lock); > list_del(&block->list); > vm->size += block->size; > > @@ -106,6 +111,7 @@ static void put_vm_block(struct ct_vm *vm, struct > ct_vm_block *block) > pos = pre; > pre = pos->prev; > } > + mutex_unlock(&vm->lock); > } > > /* Map host addr (kmalloced/vmalloced) to device logical addr. */ > @@ -191,6 +197,8 @@ int ct_vm_create(struct ct_vm **rvm) > if (NULL == vm) > return -ENOMEM; > > + mutex_init(&vm->lock); > + > /* Allocate page table pages */ > for (i = 0; i < CT_PTP_NUM; i++) { > vm->ptp[i] = kmalloc(PAGE_SIZE, GFP_KERNEL); > diff --git a/sound/pci/ctxfi/ctvmem.h b/sound/pci/ctxfi/ctvmem.h > index 4eb5bdd..618952e 100644 > --- a/sound/pci/ctxfi/ctvmem.h > +++ b/sound/pci/ctxfi/ctvmem.h > @@ -20,7 +20,7 @@ > > #define CT_PTP_NUM 1 /* num of device page table pages */ > > -#include <linux/spinlock.h> > +#include <linux/mutex.h> > #include <linux/list.h> > > struct ct_vm_block { > @@ -35,6 +35,7 @@ struct ct_vm { > unsigned int size; /* Available addr space in bytes */ > struct list_head unused; /* List of unused blocks */ > struct list_head used; /* List of used blocks */ > + struct mutex lock; > > /* Map host addr (kmalloced/vmalloced) to device logical addr. */ > struct ct_vm_block *(*map)(struct ct_vm *, void *host_addr, int > size); > I tried this patch (actually I downloaded the snapshot yesterday and patch said it was already applied there), but my system can not boot at all with this driver. It prints 'starting udev' at that's it. I removed the driver and my system is back to normal. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: snd-ctxfi 2009-06-04 3:48 ` snd-ctxfi The Source @ 2009-06-04 5:29 ` Takashi Iwai 0 siblings, 0 replies; 8+ messages in thread From: Takashi Iwai @ 2009-06-04 5:29 UTC (permalink / raw) To: The Source; +Cc: alsa-devel At Thu, 4 Jun 2009 07:48:47 +0400, The Source wrote: > > 2009/6/2 Takashi Iwai <tiwai@suse.de> > > At Mon, 1 Jun 2009 21:47:02 -0400, > Lee Revell wrote: > > > > On Mon, Jun 1, 2009 at 5:10 AM, Takashi Iwai <tiwai@suse.de> wrote: > > > Thanks. So, something wrong in the mmap handling. > > > I'll take a look later in this week. > > > > Looks like ct_map_audio_buffer calls ct_vm_map under spinlock; > > ct_vm_map calls get_vm_block which calls kzalloc with GFP_KERNEL. > > Indeed. Does the patch below fix the problem? > > thanks, > > Takashi > > --- > diff --git a/sound/pci/ctxfi/ctatc.c b/sound/pci/ctxfi/ctatc.c > index ead104e..1a4bb35 100644 > --- a/sound/pci/ctxfi/ctatc.c > +++ b/sound/pci/ctxfi/ctatc.c > @@ -119,7 +119,6 @@ atc_pcm_release_resources(struct ct_atc *atc, struct > ct_atc_pcm *apcm); > > static int ct_map_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm > *apcm) > { > - unsigned long flags; > struct snd_pcm_runtime *runtime; > struct ct_vm *vm; > > @@ -129,9 +128,7 @@ static int ct_map_audio_buffer(struct ct_atc *atc, > struct ct_atc_pcm *apcm) > runtime = apcm->substream->runtime; > vm = atc->vm; > > - spin_lock_irqsave(&atc->vm_lock, flags); > apcm->vm_block = vm->map(vm, runtime->dma_area, runtime-> > dma_bytes); > - spin_unlock_irqrestore(&atc->vm_lock, flags); > > if (NULL == apcm->vm_block) > return -ENOENT; > @@ -141,7 +138,6 @@ static int ct_map_audio_buffer(struct ct_atc *atc, > struct ct_atc_pcm *apcm) > > static void ct_unmap_audio_buffer(struct ct_atc *atc, struct ct_atc_pcm > *apcm) > { > - unsigned long flags; > struct ct_vm *vm; > > if (NULL == apcm->vm_block) > @@ -149,9 +145,7 @@ static void ct_unmap_audio_buffer(struct ct_atc *atc, > struct ct_atc_pcm *apcm) > > vm = atc->vm; > > - spin_lock_irqsave(&atc->vm_lock, flags); > vm->unmap(vm, apcm->vm_block); > - spin_unlock_irqrestore(&atc->vm_lock, flags); > > apcm->vm_block = NULL; > } > @@ -161,9 +155,7 @@ static unsigned long atc_get_ptp_phys(struct ct_atc > *atc, int index) > struct ct_vm *vm; > void *kvirt_addr; > unsigned long phys_addr; > - unsigned long flags; > > - spin_lock_irqsave(&atc->vm_lock, flags); > vm = atc->vm; > kvirt_addr = vm->get_ptp_virt(vm, index); > if (kvirt_addr == NULL) > @@ -171,8 +163,6 @@ static unsigned long atc_get_ptp_phys(struct ct_atc > *atc, int index) > else > phys_addr = virt_to_phys(kvirt_addr); > > - spin_unlock_irqrestore(&atc->vm_lock, flags); > - > return phys_addr; > } > > @@ -1562,7 +1552,6 @@ int ct_atc_create(struct snd_card *card, struct > pci_dev *pci, > atc_set_ops(atc); > > spin_lock_init(&atc->atc_lock); > - spin_lock_init(&atc->vm_lock); > > /* Find card model */ > err = atc_identify_card(atc); > diff --git a/sound/pci/ctxfi/ctatc.h b/sound/pci/ctxfi/ctatc.h > index 286c993..a7b0ec2 100644 > --- a/sound/pci/ctxfi/ctatc.h > +++ b/sound/pci/ctxfi/ctatc.h > @@ -101,7 +101,6 @@ struct ct_atc { > unsigned long (*get_ptp_phys)(struct ct_atc *atc, int index); > > spinlock_t atc_lock; > - spinlock_t vm_lock; > > int (*pcm_playback_prepare)(struct ct_atc *atc, > struct ct_atc_pcm *apcm); > diff --git a/sound/pci/ctxfi/ctvmem.c b/sound/pci/ctxfi/ctvmem.c > index cecf77e..363b67e 100644 > --- a/sound/pci/ctxfi/ctvmem.c > +++ b/sound/pci/ctxfi/ctvmem.c > @@ -35,25 +35,27 @@ get_vm_block(struct ct_vm *vm, unsigned int size) > struct ct_vm_block *block = NULL, *entry = NULL; > struct list_head *pos = NULL; > > + mutex_lock(&vm->lock); > list_for_each(pos, &vm->unused) { > entry = list_entry(pos, struct ct_vm_block, list); > if (entry->size >= size) > break; /* found a block that is big enough */ > } > if (pos == &vm->unused) > - return NULL; > + goto out; > > if (entry->size == size) { > /* Move the vm node from unused list to used list directly > */ > list_del(&entry->list); > list_add(&entry->list, &vm->used); > vm->size -= size; > - return entry; > + block = entry; > + goto out; > } > > block = kzalloc(sizeof(*block), GFP_KERNEL); > if (NULL == block) > - return NULL; > + goto out; > > block->addr = entry->addr; > block->size = size; > @@ -62,6 +64,8 @@ get_vm_block(struct ct_vm *vm, unsigned int size) > entry->size -= size; > vm->size -= size; > > + out: > + mutex_unlock(&vm->lock); > return block; > } > > @@ -70,6 +74,7 @@ static void put_vm_block(struct ct_vm *vm, struct > ct_vm_block *block) > struct ct_vm_block *entry = NULL, *pre_ent = NULL; > struct list_head *pos = NULL, *pre = NULL; > > + mutex_lock(&vm->lock); > list_del(&block->list); > vm->size += block->size; > > @@ -106,6 +111,7 @@ static void put_vm_block(struct ct_vm *vm, struct > ct_vm_block *block) > pos = pre; > pre = pos->prev; > } > + mutex_unlock(&vm->lock); > } > > /* Map host addr (kmalloced/vmalloced) to device logical addr. */ > @@ -191,6 +197,8 @@ int ct_vm_create(struct ct_vm **rvm) > if (NULL == vm) > return -ENOMEM; > > + mutex_init(&vm->lock); > + > /* Allocate page table pages */ > for (i = 0; i < CT_PTP_NUM; i++) { > vm->ptp[i] = kmalloc(PAGE_SIZE, GFP_KERNEL); > diff --git a/sound/pci/ctxfi/ctvmem.h b/sound/pci/ctxfi/ctvmem.h > index 4eb5bdd..618952e 100644 > --- a/sound/pci/ctxfi/ctvmem.h > +++ b/sound/pci/ctxfi/ctvmem.h > @@ -20,7 +20,7 @@ > > #define CT_PTP_NUM 1 /* num of device page table pages */ > > -#include <linux/spinlock.h> > +#include <linux/mutex.h> > #include <linux/list.h> > > struct ct_vm_block { > @@ -35,6 +35,7 @@ struct ct_vm { > unsigned int size; /* Available addr space in bytes */ > struct list_head unused; /* List of unused blocks */ > struct list_head used; /* List of used blocks */ > + struct mutex lock; > > /* Map host addr (kmalloced/vmalloced) to device logical addr. */ > struct ct_vm_block *(*map)(struct ct_vm *, void *host_addr, int > size); > > I tried this patch (actually I downloaded the snapshot yesterday and patch > said it was already applied there), but my system can not boot at all with > this driver. It prints 'starting udev' at that's it. I removed the driver and > my system is back to normal. Yes, yesterday's version had a severe bug in the core code. It's already fixed, so please use the later one. Note that alsa-driver-snapshot.tar.gz is *always* the latest version, so pick this up instead of the daily snapshots for testing something. thanks, Takashi ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-06-04 5:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-30 3:22 snd-ctxfi The Source 2009-05-30 7:00 ` snd-ctxfi Takashi Iwai 2009-05-30 16:22 ` snd-ctxfi The Source 2009-06-01 9:10 ` snd-ctxfi Takashi Iwai 2009-06-02 1:47 ` snd-ctxfi Lee Revell 2009-06-02 6:09 ` snd-ctxfi Takashi Iwai 2009-06-04 3:48 ` snd-ctxfi The Source 2009-06-04 5:29 ` snd-ctxfi Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.