Re: 2.6.24-git: kmap_atomic() WARN_ON()

Björn Steinbrink
2008-02-25T20:35:53Z

On 2008.02.25 15:08:35 -0500, Jeff Garzik wrote:
> Björn Steinbrink wrote:
>> Hm, do you have lockdep enabled? If not, does lockdep make this go away?
>> Because lockdep will set IRQF_DISABLED for all interrupt handlers, and
>> unless that flag is set, handle_IRQ_event will reenable interrupts while
>> the handler is running. And ahci_interrupt only uses a plain spin_lock,
>> so interrupts keep being enabled. The patch below should help with that.
>>
>> Hmhm, maybe that also solves the deadlock you saw? Dunno...
>>
>> I can't come up with an useful commit message right now, but I'll resend
>> in suitable form for submission if that thing actually works.
>>
>> Björn
>>
>>
>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>> index 1db93b6..ae3dbc8 100644
>> --- a/drivers/ata/ahci.c
>> +++ b/drivers/ata/ahci.c
>> @@ -1739,6 +1739,7 @@ static irqreturn_t ahci_interrupt(int irq, void *dev_instance)
>> unsigned int i, handled = 0;
>> void __iomem *mmio;
>> u32 irq_stat, irq_ack = 0;
>> + unsigned long flags;
>> VPRINTK("ENTER\n");
>> @@ -1751,7 +1752,7 @@ static irqreturn_t ahci_interrupt(int irq, void
>> *dev_instance)
>> if (!irq_stat)
>> return IRQ_NONE;
>> - spin_lock(&host->lock);
>> + spin_lock_irqsave(&host->lock, flags);
>> for (i = 0; i < host->n_ports; i++) {
>> struct ata_port *ap;
>> @@ -1778,7 +1779,7 @@ static irqreturn_t ahci_interrupt(int irq, void *dev_instance)
>> handled = 1;
>> }
>> - spin_unlock(&host->lock);
>> + spin_unlock_irqrestore(&host->lock, flags);
>
> If this truly fixes the problem, then lockdep is definitely the problem
> source.

Hm, lockdep keeps the interrupts _disabled_. The code that reenables the
interrupts was already in the first revision of Linus' git tree. So
lockdep would actually just hide the problem, not cause it.

Björn