busdma dflt_lock on amd64 > 4 GB

[Søren Schmidt]

On 26/10/2005, at 16:09, Scott Long wrote:
> Jacques Caron wrote:
>> Hi all,
>> Continuing on this story... [I took the liberty of CC'ing Scott  
>> and Soren], pr is amd64/87977 though it finally isn't amd64- 
>> specific but  >4GB-specific.
>> There is really a big problem somewhere between ata and bus_dma  
>> for boxes with more than 4 GB RAM and more than 2 ata disks:
>> * bounce buffers will be needed
>> * ata will have bus_dma allocate bounce buffers:
>> hw.busdma.zone1.total_bpages: 32
>> hw.busdma.zone1.free_bpages: 32
>> hw.busdma.zone1.reserved_bpages: 0
>> hw.busdma.zone1.active_bpages: 0
>> hw.busdma.zone1.total_bounced: 27718
>> hw.busdma.zone1.total_deferred: 0
>> hw.busdma.zone1.lowaddr: 0xffffffff
>> hw.busdma.zone1.alignment: 2
>> hw.busdma.zone1.boundary: 65536
>> * if I do a dd with a bs=256000, 16 bounce pages will be used  
>> (most of the time). As long as I stay on the same disk, no more  
>> pages will be used.
>> * as soon as I access another disk (e.g. with another dd with the  
>> same bs=256000), another set of 16 pages will be used (bus_dma  
>> tags and maps are allocated on a per-channel basis), and all 32  
>> bounce pages will be used (most of the time)
>> * and if I try to access a third disk, more bounce pages are  
>> needed and:
>> - one of ata_dmaalloc calls to bus_dma_tag_create has ALLOCNOW set
>> - busdma_machdep will not allocate more bounce pages in that case  
>> (the limit is imposed by maxsize in that situation, which has  
>> already been reached)
>> - ata_dmaalloc will fail
>> - but some other bus_dma_tag_create call without ALLOCNOW set will  
>> still cause bounce pages to be allocated, but deferred, and the  
>> non-existent lockfunc to be called, and panic.
>> Adding the standard lockfunc will (probably) solve the panic  
>> issue, but there will still be a problem with DMA in ata.
>>
>
> Actually, it won't.  It'll result in silent data corruption.  What is
> happening is that bus_dmamap_load() is returning EINPROGRESS, but the
> ATA driver ignores it and assumes that the load failed.  Later on the
> busdma subsystem tries to run the callback but trips over the  
> intentional assertion.  If the standard lock was used, then the  
> callback
> would succeed and start spamming memory that either had been freed or
> is in the process of being used by other ATA commands.

Ehm, according to the man page the load should succed for at least  
one map when the ALLOCNOW flag is set. ATA only use one map so there  
is no way that spamming can happen.
The bug i ATA is that the sg_tag and the work_tag is not created with  
the ALLOCNOW flag so if all resources are used before they are called  
things get messy. The below patch takes care of that problem.
> So, the panic is doing exactly what it is supposed to do.  It's  
> guarding
> against bugs in the driver.  The workaround for this is to use the  
> NOWAIT flag in all instances of bus_dmamap_load() where deferals can
> happen.  This, however, means that using bounce pages still remains  
> fragile and that the driver is still likely to return ENOMEM to the  
> upper layers.  C'est la vie, I guess.  At one time I had patches that
> made ATA use the busdma API correctly (it is one of the few remaining
> that does not), but they rotted over time.

As long as ATA doesn't do tags there is no gain by changing this at  
all except spamming the code with all the callback crap thats not  
needed.
According to the man page bus_dmamap_load takes no flags, so thats  
why thats not done. Besides its not needed as shown above.

Søren Schmidt
sos at FreeBSD.org