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

Jeff Garzik
2008-02-26T23:00:38Z

Andrew Morton wrote:
> On Tue, 26 Feb 2008 21:49:43 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>> i.e. to introduce a kmap_atomic_irqsave(...,flags) and
>> kunmap_atomic_irqrestore() API variant. _That_ then could be mapped by
>> -rt to a non-irq disabling thing.

> Sure. But iirc we haven't had a need for this before. Which is a bit odd.

We have such a need -- we just always paper over it with
spin_lock_irqsave() or local_irq_save() because those are "the rules."
grep around :)

See ata_pio_sector() in libata-core.c, where we do:

> if (PageHighMem(page)) {
> unsigned long flags;
>
> /* FIXME: use a bounce buffer */
> local_irq_save(flags);
> buf = kmap_atomic(page, KM_IRQ0);
>
> /* do the actual data transfer */
> ap->ops->data_xfer(qc->dev, buf + offset, qc->sect_size, do_write);
>
> kunmap_atomic(buf, KM_IRQ0);
> local_irq_restore(flags);
> } else {
> buf = page_address(page);
> ap->ops->data_xfer(qc->dev, buf + offset, qc->sect_size, do_write);
> }

This is a core non-DMA data transfer routine. Given the high cost of
disabling interrupts during the relatively-slow PIO data transfer, we
added extra logic to only disable interrupts for the highmem case.

The code in libata-scsi generally only deals with a single buffer, on
average less than 128 bytes in length. So the memcpy() isn't really
that expensive in the case being discussed -- unlike the
ata_pio_sector() case, where the cost is very, very high.

Aside: The "FIXME: use bounce buffer" comment above indicates the more
optimal PIO data xfer approach of

local_irq_save()
kmap_atomic()
memcpy into bounce buffer
kunmap_atomic()
local_irq_restore()

/* do slow PIO bitbanging data transfer */
ap->ops->data_xfer(...)

Regards,

Jeff