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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/03/2010 12:20 PM, Andrew Morton wrote:
On Wed, 03 Feb 2010 12:15:46 -0500 Jeff Garzik<jgarzik@xxxxxxxxx>  wrote:

On 02/03/2010 12:06 PM, Andrew Morton wrote:
On Wed, 03 Feb 2010 12:00:58 -0500 Jeff Garzik<jgarzik@xxxxxxxxx>   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?

libata may have to transfer data in response to an interrupt. That is normal interrupt-driven PIO -- although it should be noted that the code supports polled PIO as well.


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.

I don't object to the patch... as long as the arch people are happy. Arch people seem to be the ones complaining, though.

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux