Messages in this thread | | | Date | Sun, 14 Nov 2004 16:01:31 +0100 | From | Jens Axboe <> | Subject | Re: kernel analyser to detect sleep under spinlock |
| |
On Sat, Nov 13 2004, Peter T. Breuer wrote: > Peter T. Breuer <ptb@inv.it.uc3m.es> wrote: > > I'll undertake a survey of the current kernel. > > Just for kicks, I started with the DAC960.c driver (alphabet ..), and > it registered 6 alarms! > > Linux Driver for Mylex DAC960/AcceleRAID/eXtremeRAID PCI RAID Controllers > > Copyright 1998-2001 by Leonard N. Zubkoff <lnz@dandelion.com> > > * function line calls (locks) > * - /usr/local/src/linux-2.6.3/drivers/block/DAC960.c > !! DAC960_BA_InterruptHandler 5219 DAC960_V2_ProcessCompletedCommand (1) > !! DAC960_LP_InterruptHandler 5262 DAC960_V2_ProcessCompletedCommand (1) > !! DAC960_V1_ExecuteUserCommand 5869 DAC960_WaitForCommand (1) > !! DAC960_V2_ExecuteUserCommand 6132 DAC960_WaitForCommand (1) > !! DAC960_gam_ioctl 6663 DAC960_WaitForCommand (1) > !! DAC960_gam_ioctl 6688 DAC960_WaitForCommand (1) > > The ProcessCompletedCommand thing really is called under spinlock, but > it appears to be detected as sleepy because it calls kmalloc (and > kfree), however it calls kmalloc with GFP_ATOMIC, so it's not sleepy > and that's a false alarm. Ho hum ... I'll have to detect that. > > The WaitForCommand is also definitely called under spinlock ... and is > thought to be sleepy because it calls schedule! Well, it calls > __wait_event(Controller->CommandWaitQueue, Controller->FreeCommands); > Is that going to schedule? I suppose logically it should. > > Anyway, that looks a legitimate complaint: > > spin_lock_irqsave(&Controller->queue_lock, flags); > while ((Command = DAC960_AllocateCommand(Controller)) == NULL) > DAC960_WaitForCommand(Controller); > spin_unlock_irqrestore(&Controller->queue_lock, flags); > > Looks like it waits under spinlock to me!
static void DAC960_WaitForCommand(DAC960_Controller_T *Controller) { spin_unlock_irq(&Controller->queue_lock); __wait_event(Controller->CommandWaitQueue, Controller->FreeCommands); spin_lock_irq(&Controller->queue_lock); }
Looks alright to me, I don't understand your confusion. One thing you could say is that either the path to DAC960_WaitForCommand should not save interrupt flags, _or_ it's a bug to use spin_unlock_irq() if interrutps could already be disabled at that point.
-- Jens Axboe
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |