Re: Rawmidi and CONFIG_DEBUG_SPINLOCK_SLEEP

[Thomas Charbonnel <thomas@undata.org>]

[-- Attachment #1: Type: text/plain, Size: 2678 bytes --]

Thomas Charbonnel wrote :
> Jaroslav Kysela a écrit :
> 
>> On Thu, 29 Jan 2004, Thomas Charbonnel wrote:
>>
>>
>>> Hi,
>>>
>>> I noticed this problem today using midi on my setup (alsa 1.0.1, 
>>> kernel 2.6.1 with CONFIG_DEBUG_SPINLOCK_SLEEP) :
>>>
>>> Debug: sleeping function called from invalid context at 
>>> include/asm/uaccess.h:473
>>> in_atomic():1, irqs_disabled():1
>>> Call Trace:
>>>  [<c012139c>] __might_sleep+0xac/0xe0
>>>  [<c010b170>] handle_signal+0xd0/0x140
>>>  [<c03955ef>] snd_rawmidi_kernel_read1+0x10f/0x170
>>>  [<c03957e7>] snd_rawmidi_read+0x147/0x1b0
>>>  [<c01131cf>] restore_i387_fxsave+0x8f/0xa0
>>>  [<c015b2f3>] vfs_read+0xd3/0x140
>>>  [<c015b59f>] sys_read+0x3f/0x60
>>>  [<c010b4bb>] syscall_call+0x7/0xb
>>>
>>> I'm sorry I didn't check if this was fixed in 1.0.2, but though it 
>>> was worth mentioning. This caused regular (~ 1/second) dropouts in pd 
>>> while using midi.
>>
>>
>>
>> I'm not sure, but it looks like kernel problem. We don't call
>> handle_signal() function from snd_rawmidi_kernel_read1() - you may check
>> this function - scheduling is not involved. I wonder why this function is
>> called from in the spinlock context.
>>
>> Please, report this problem to LKML or try the latest 2.6 kernel.
>>
>>                         Jaroslav
>>
> 
> Hi Jaroslav,
> 
> Doing some more tests I got also this message :
> 
> Debug: sleeping function called from invalid context at
> include/asm/uaccess.h:498
> in_atomic():1, irqs_disabled():1
> Call Trace:
>  [<c012139c>] __might_sleep+0xac/0xe0
>  [<c0395c2d>] snd_rawmidi_kernel_write1+0x16d/0x200
>  [<c0395ea4>] snd_rawmidi_write+0x1b4/0x300
>  [<c010b2c1>] do_signal+0xe1/0xf0
>  [<c01131cf>] restore_i387_fxsave+0x8f/0xa0
>  [<c015b4f3>] vfs_write+0xd3/0x140
>  [<c015b5ff>] sys_write+0x3f/0x60
>  [<c010b4bb>] syscall_call+0x7/0xb
> 
> The kernel functions pointed to in include/asm/usaccess.h are
> copy_to_user and copy_from_user. They are indeed called from a spinlock
> context from both snd_rawmidi_kernel_write1 and
> snd_rawmidi_kernel_read1, and are flagged might_sleep. The strange thing
> is that they were already flagged might_sleep in 2.6.0, which I believe
> I compiled with CONFIG_DEBUG_SPINLOCK_SLEEP also, and I can't remember
> having the problem. On the other hand I only checked dmesg here because
> I could hear audio dropouts. Maybe the might_sleep detection code has
> been changed and is responsible for the dropouts ?
> 
> Thomas
> 

Hi Jaroslav,

The attached patch fixes the problem for me, but I took the obvious and 
easy way - I may have missed something.

Thomas

[-- Attachment #2: rawmidi.patch --]
[-- Type: text/plain, Size: 1076 bytes --]

--- rawmidi.c.old	2003-10-23 16:34:52.000000000 +0200
+++ rawmidi.c	2004-02-01 15:04:15.000000000 +0100
@@ -909,10 +909,11 @@
 		if (kernel) {
 			memcpy(buf + result, runtime->buffer + runtime->appl_ptr, count1);
 		} else {
+			spin_unlock_irqrestore(&runtime->lock, flags);
 			if (copy_to_user(buf + result, runtime->buffer + runtime->appl_ptr, count1)) {
-				spin_unlock_irqrestore(&runtime->lock, flags);
 				return result > 0 ? result : -EFAULT;
 			}
+			spin_lock_irqsave(&runtime->lock, flags);
 		}
 		runtime->appl_ptr += count1;
 		runtime->appl_ptr %= runtime->buffer_size;
@@ -1133,10 +1134,13 @@
 		if (kernel) {
 			memcpy(runtime->buffer + runtime->appl_ptr, buf, count1);
 		} else {
+			spin_unlock_irqrestore(&runtime->lock, flags);
 			if (copy_from_user(runtime->buffer + runtime->appl_ptr, buf, count1)) {
+				spin_lock_irqsave(&runtime->lock, flags);
 				result = result > 0 ? result : -EFAULT;
 				goto __end;
 			}
+			spin_lock_irqsave(&runtime->lock, flags);
 		}
 		runtime->appl_ptr += count1;
 		runtime->appl_ptr %= runtime->buffer_size;