On Thu, 2010-02-04 at 15:39 +0000, Catalin Marinas wrote: > On Thu, 2010-02-04 at 15:01 +0000, James Bottomley wrote: > > On Thu, 2010-02-04 at 14:33 +0000, Catalin Marinas wrote: > > > On Wed, 2010-02-03 at 17:39 +0000, James Bottomley wrote: > > > > On Wed, 2010-02-03 at 12:29 -0500, Jeff Garzik wrote: > > > > > 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: > > > > > >>> 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 > > > > > > > > Apart from the contents, which is looking like sprinkle mainline with > > > > random flushes, I'm unhappy that something which could be better > > > > implemented by thinking about the problem is now being rammed through as > > > > a must have bug fix. > > > > > > > > We got this piece of crap in the same way: > > > > > > > > commit 2d4dc890b5c8fabd818a8586607e6843c4375e62 > > > > Author: Ilya Loginov <isloginov@xxxxxxxxx> > > > > Date: Thu Nov 26 09:16:19 2009 +0100 > > > > > > > > block: add helpers to run flush_dcache_page() against a bio and a > > > > request's pages > > > > > > > > Which is another race to flush everywhere until my coherence problem > > > > goes away. > > > > > > > > This scattershot approach to coherency is really damaging in the long > > > > term because all these random flushes are going to mask real problems we > > > > may or may not have in the arch APIs ... and worse, they'll mask > > > > mostly ... there'll likely be times when the masking is incomplete and > > > > we're left with incredibly hard to debug data loss or binary crashes. > > > > > > I agree that this could be solved in a better way and I'm in favour of > > > API improvements. The current API and description in cachetlb.txt > > > suggest that flush_dcache_page() is the best approach when modifying > > > page cache pages. > > > > > > AFAICT, there are two main use cases of flush_dcache_page() (could be > > > more but that's what affects ARM): (1) filesystems modifying page cache > > > pages (NFS, cramfs etc.) and (2) drivers doing PIO (block, HCD) that may > > > write directly to page cache pages. Point (2) is a newer addition (as > > > the one above) to fix coherency problems on Harvard architectures. > > > > > > As long as you use filesystems like cramfs that call > > > flush_dcache_page(), the PIO block driver doesn't need to do any > > > flushing. That's one of the reasons why MTD didn't need any for a long > > > time. Once you start using filesystems like ext2, there is no > > > flush_dcache_page() called by the VFS layer, so we thought about moving > > > this into the driver (for a long time I had a dirty hack in > > > mpage_end_io_read() to call flush_dcache_page()). > > > > > > If we state that flush_dcache_page() should not be used in driver code, > > > than we need additional API for this (like pio_map_page etc. to be in > > > line with the dma_map_* functions). This would allow architectures to > > > implement them however they like. > > > > > > I can go on linux-arch with a proposed patch if this sounds acceptable. > > > > I think I'd prefer some type of kmap_ variant. The reason is that all > > the semantics are going to be the same as kmap ... down to having the > > slots for atomic. All we need is the extra flag giving the direction of > > transfer and, of course, what you get back is a virtual address pointer > > (all the dma operations return physical address handles). > > Would a kmap_pio API always require a page structure as argument? This seems a reasonable requirement. > There is a similar situation for HCD drivers and USB mass storage which > I raised recently. The idea that the USB mass storage code should do the > cache flushing was rejected as this has to be pushed down to the HCD > driver. However, the HCD driver doesn't get page information, only a > urb->transfer_buffer pointer to which it may do either DMA or PIO (both > may actually happen in the same driver as an optimisation). You'd want to do dma_map or kmap at a point after the DMA or PIO decision is made. > If we enforce the use of a kmap_pio API in the USB mass storage, it > wouldn't be optimal since this code is not aware of whether the HCD > driver would do PIO or DMA. So, if the decision isn't made until the HCD driver, then surely this belongs there. > Looking at the HCD drivers, there are more similarities to the DMA API > like pio_map_page, pio_map_single etc. I personally don't care much > about how the API is named but rather where such API should be called > from. the API usage is similar to the dma one. The point I was making is the semantics are similar to kmap because of the slot and atomic/non atomic requirements. James -- 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
