Re: kernel analyser to detect sleep under spinlock

[Jens Axboe <axboe@suse.de>]

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