
From: Manfred Spraul <manfred@colorfullife.com>

The mwave driver uses a user space daemon for some modem operations. The 
user space daemon calls ioctl(,IOCTL_MW_GET_IPC), and the driver returns 
after an interrupt arrived. The actual wait used 
interruptible_sleep_on(), which can lead to lost wakeups. A local 
spinlock on the stack is used to close that race, but this is broken on 
SMP, perhaps even with preempt.

The attached patch fixes that by switching to the normal 
add_wait_queue/test_if_race_occured/schedule/remove_wait_queue sequence.



 drivers/char/mwave/mwavedd.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff -puN drivers/char/mwave/mwavedd.c~mwave-locking-fixes drivers/char/mwave/mwavedd.c
--- 25/drivers/char/mwave/mwavedd.c~mwave-locking-fixes	2003-09-07 11:55:59.000000000 -0700
+++ 25-akpm/drivers/char/mwave/mwavedd.c	2003-09-07 11:55:59.000000000 -0700
@@ -293,8 +293,6 @@ static int mwave_ioctl(struct inode *ino
 	
 		case IOCTL_MW_GET_IPC: {
 			unsigned int ipcnum = (unsigned int) ioarg;
-			spinlock_t ipc_lock = SPIN_LOCK_UNLOCKED;
-			unsigned long flags;
 	
 			PRINTK_3(TRACE_MWAVE,
 				"mwavedd::mwave_ioctl IOCTL_MW_GET_IPC"
@@ -310,32 +308,29 @@ static int mwave_ioctl(struct inode *ino
 			}
 	
 			if (pDrvData->IPCs[ipcnum].bIsEnabled == TRUE) {
+				DECLARE_WAITQUEUE(wait, current);
+
 				PRINTK_2(TRACE_MWAVE,
 					"mwavedd::mwave_ioctl, thread for"
 					" ipc %x going to sleep\n",
 					ipcnum);
-	
-				spin_lock_irqsave(&ipc_lock, flags);
+				add_wait_queue(&pDrvData->IPCs[ipcnum].ipc_wait_queue, &wait);
+				pDrvData->IPCs[ipcnum].bIsHere = TRUE;
+				set_current_state(TASK_INTERRUPTIBLE);
 				/* check whether an event was signalled by */
 				/* the interrupt handler while we were gone */
 				if (pDrvData->IPCs[ipcnum].usIntCount == 1) {	/* first int has occurred (race condition) */
 					pDrvData->IPCs[ipcnum].usIntCount = 2;	/* first int has been handled */
-					spin_unlock_irqrestore(&ipc_lock, flags);
 					PRINTK_2(TRACE_MWAVE,
 						"mwavedd::mwave_ioctl"
 						" IOCTL_MW_GET_IPC ipcnum %x"
 						" handling first int\n",
 						ipcnum);
 				} else {	/* either 1st int has not yet occurred, or we have already handled the first int */
-					pDrvData->IPCs[ipcnum].bIsHere = TRUE;
-#warning "Sleeping on spinlock"
-					interruptible_sleep_on(&pDrvData->IPCs[ipcnum].ipc_wait_queue);
-					pDrvData->IPCs[ipcnum].bIsHere = FALSE;
+					schedule();
 					if (pDrvData->IPCs[ipcnum].usIntCount == 1) {
-						pDrvData->IPCs[ipcnum].
-						usIntCount = 2;
+						pDrvData->IPCs[ipcnum].usIntCount = 2;
 					}
-					spin_unlock_irqrestore(&ipc_lock, flags);
 					PRINTK_2(TRACE_MWAVE,
 						"mwavedd::mwave_ioctl"
 						" IOCTL_MW_GET_IPC ipcnum %x"
@@ -343,6 +338,9 @@ static int mwave_ioctl(struct inode *ino
 						" application\n",
 						ipcnum);
 				}
+				pDrvData->IPCs[ipcnum].bIsHere = FALSE;
+				remove_wait_queue(&pDrvData->IPCs[ipcnum].ipc_wait_queue, &wait);
+				set_current_state(TASK_RUNNING);
 				PRINTK_2(TRACE_MWAVE,
 					"mwavedd::mwave_ioctl IOCTL_MW_GET_IPC,"
 					" returning thread for ipc %x"

_
