Re: [patch for 2.6.33? 1/1] ata: call flush_dcache_page() around PIO data transfers in libata-aff.c

[Jeff Garzik <jgarzik@pobox.com>]

On 02/03/2010 12:06 PM, Andrew Morton wrote:
> On Wed, 03 Feb 2010 12:00:58 -0500 Jeff Garzik<jgarzik@pobox.com>  wrote:
>
>> On 02/03/2010 11:40 AM, James Bottomley wrote:
>>> The fix to libata looks to be just that it should kmap all the time
>>> rather than trying to fiddle with the page to see if its higmem.  For
>>> kmap on a normal page, we should just return the offset map address and
>>> do all the flushing.
>>
>> libata tests PageHighMem() because it was measurably faster to do things
>> the current way (which includes local_irq_save/restore, only for
>> highmem) on boxes where it actually matters.
>>
>> It seems more efficient to add a flush where necessary, than
>> unconditionally punish everyone...
>
> kmap_atomic() tests PageHighMem() too - it's pretty lightweight for
> lowmem pages.

Note the lack of local_irq_save/restore in our code, though...  These 
PIO xfers are __slow__, from the perspective of a CPU manufactured in 
the past decade; you are definitely disabling local interrupts for a 
long time.  I suppose we could do

	if (high mem)
		local irq save
		kmap
		xfer
		kunmap
		local irq restore
	else
		kmap
		xfer
		kunmap

does that solve the problem for ARM, for 2.6.33?


> Anyway, I'd draw your attention to this claim in the changelog: "This
> patch allows the ARM boards to use a rootfs on CompactFlash with the
> PATA platform driver." Immediate-term, we should be looking for a small
> fix for this issue which is acceptable for 2.6.33 and 2.6.32 and earlier.

Sure...  see above.  hopefully one that does not punish -everybody- 
though.  It would be sad to unconditionally slow down millions of volume 
platform (read: x86) users for some embedded boards.

	Jeff