Re: kernel analyser to detect sleep under spinlock

From: Jens Axboe
Date: Sun Nov 14 2004 - 10:02:43 EST


On Sat, Nov 13 2004, Peter T. Breuer wrote:
> Peter T. Breuer <ptb@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>
> * 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/