At Tue, 3 Dec 2002 06:57:45 +0100, Duncan Sands wrote: > > On Tuesday 03 December 2002 14:56, Takashi Iwai wrote: > > At Tue, 3 Dec 2002 04:55:15 +0100, > > > > Duncan Sands wrote: > > > On Tuesday 03 December 2002 13:01, Takashi Iwai wrote: > > > > At Tue, 3 Dec 2002 04:07:52 +0100, > > > > > > > > Duncan Sands wrote: > > > > > Got this with today 2.5 BK tree: > > > > > > > > > > Debug: sleeping function called from illegal context at > > > > > include/asm/semaphore.h:119 Call Trace: > > > > > [<c0113f1a>] __might_sleep+0x52/0x58 > > > > > [<c024291a>] snd_cs46xx_iec958_put+0x36/0xf8 > > > > > [<c0217f28>] snd_ctl_elem_write+0xe0/0x1a4 > > > > > [<c0218360>] snd_ctl_ioctl+0x184/0x2c8 > > > > > [<c01462e6>] sys_ioctl+0x1fa/0x244 > > > > > [<c01088f7>] syscall_call+0x7/0xb > > > > > > > > ouch, we are using rwlock in the control ioctls. > > > > > > > > mutex is necessary for many controls, so we cannot suppress the use of > > > > mutex in control callbacks. > > > > but temporary unlocking looks ad-hoc, too... > > > > > > If I understand right, the problem is that snd_ctl_elem_write > > > acquires control_rwlock, which is a rw spinlock. It then calls > > > snd_cs46xx_iec958_put which acquires chip->spos_mutex, > > > which is a semaphore. Thus the message. Now I deduce > > > from the fact that you don't use read_lock_irqsave that the > > > data structure is not read from interrupt context. That means > > > you are only protecting against other CPUs. So why not use > > > a semaphore instead of a spinlock? > > > > hmm, there are some places calling with irq lock. > > for example, snd_ctl_notify() can be called from the interrupt handler > > for some interrupts like h/w volume change. > > Uh oh! > > > however, it seems that a single rwlock is used for the management of > > two different lists. and snd_ctl_elem_write() uses card->controls > > only, whereas snd_ctl_notify() uses card->ctl_files only. > > hence, we can merge two locks, rwsem for card->controls and rwlock for > > card->ctl_files. i'll give a try. > > I guess another way of dealing with this kind of problem is to use a > semaphore rather than a spinlock, and a workqueue: when the interrupt > comes in, the call to snd_ctl_notify is put on the queue, where it will later > be run in process context, and can safely take the semaphore.
yes, basically what snd_ctl_notify() does is the same. it queues an event and wakes up the sleepers. thus, it's ok to separate the stuff from the semaphore. the attached is a patch to rewrite the locks with rwsem. please check whether it works for you. thanks, Takashi
control-lock-fix.dif
Description: Binary data
