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

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 03 Feb 2010 12:15:46 -0500 Jeff Garzik <jgarzik@pobox.com> wrote:

> 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?
> 

It's unclear (to me) why that code is using KM_IRQ0 at all.  Can't it
use a non-irq kmap slot?

> > 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.

Well. 
ata-call-flush_dcache_page-around-pio-data-transfers-in-libata-affc.patch
is a no-op on x86.  It only has an effect on architectures which
implement flush_dcache_page().  And I expect flush_dcache_page() is
pretty light on those architectures, when compared with a PIO-mode
transfer.