Re: swiotlb_alloc_coherent: allocated memory is out of range for device

[Takashi Iwai <tiwai@suse.de>]

At Sun, 19 Oct 2008 12:09:32 +0200,
Sven Schnelle wrote:
> 
> Hi List,
> 
> my kernel dies while probing parport with the following last words:
> 
> [    3.672199] parport_pc 00:0b: reported by Plug and Play ACPI
> [    3.677969] parport0: PC-style at 0x378 (0x778), irq 7, dma 3 [PCSPP,TRISTATE,COMPAT,EPP,ECP,DMA]
> [    3.687691] hwdev DMA mask = 0x0000000000ffffff, dev_addr = 0x0000000020000000
> [    3.694916] Kernel panic - not syncing: swiotlb_alloc_coherent: allocated memory is out of range for device
> 
> I haven't started a bisection yet, but this seems to be introduced
> somewhere between 2.6.26 and 2.6.27, at least 2.6.26 was working without
> problems. The dmesg log + config was obtained from a kernel compiled
> from git on 10/16/2008.

This bug hits me, too.  Looks like swiotlb assumes that the alloc caller
must set GFP_DMA appropriately by itself since GFP_DMA hack was
removed.  The patch below should fix this particular case.

HOWEVER: the fundamental problem appears to be in swiotlb itself.
It assumes that iotlb pages are in DMA area.  But, in this case, the
driver sets 24bit DMA (as of PnP) while iotlb pages are allocated 
under 32bit DMA via alloc_bootmem_low_pages().  This doesn't work, of
course.

So, even adding GFP_DMA works mostly, it has still potentially
breakage when you can't get the page and fall back to iotlb pages,
just like the panic above.

Also, the removal of GFP_DMA hack is a bad idea.  For example, if a
device requires 28bit DMA mask, it doesn't set always GFP_DMA for
allocation because pages in ZONE_NORMAL may be inside that DMA mask.
Normal allocators allow this behavior but swiotlb allocator doesn't.
(Correct me if I'm wrong here -- I haven't followed much the recent
 changes.)

Last but not least, I think panic() in allocation error path is too
strict.  Usually returning NULL isn't always fatal error, so give some
more chance to debug, e.g. by calling WARN() (or whatever) instead of
panic().


thanks,

Takashi

diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 8a846ad..4fbec16 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -2347,7 +2347,7 @@ struct parport *parport_pc_probe_port (unsigned long int base,
 				  dma_alloc_coherent(dev,
 						       PAGE_SIZE,
 						       &priv->dma_handle,
-						       GFP_KERNEL);
+						       GFP_KERNEL | GFP_DMA);
 				if (! priv->dma_buf) {
 					printk (KERN_WARNING "%s: "
 						"cannot get buffer for DMA, "