On Thu, 2009-12-10 at 17:48 +0000, Russell King wrote: > On Thu, Dec 10, 2009 at 11:07:31AM -0600, James Bottomley wrote: > > For PIO we have this kmap/operate/flush_kernel_dcache_page()/kunmap > > complexity (see for example drivers/mmc/mmc_spi.c). However, it could > > all be taken care of in a similar way to the DMA API via an > > abstraction ... although I suspect the best abstraction is to pull the > > flush into kunmap. > > I assume you actually mean kmap_atomic() / kunmap_atomic(), since that > would be used in interrupt driven PIO drivers rather than kmap()/kunmap(). Yes, just using shorthand. > Well, it would mean every kunmap_atomic() gains an expensive cache flush > no matter what it's doing. That would be very bad for things like > copy_user_highpage(), where we kmap the source and destination pages, > and then kunmap both. > > However, there are cases where we do want to flush on kunmap_atomic() - > again, taking the copy_user_highpage() example, we want to ensure that > data written to the page is going to be visible in some way. IOW, we > already have this: > > kfrom = kmap_atomic(from, KM_USER0); > kto = kmap_atomic(to, KM_USER1); > copy_page(kto, kfrom); > #ifdef CONFIG_HIGHMEM > /* > * kmap_atomic() doesn't set the page virtual address, and > * kunmap_atomic() takes care of cache flushing already. > */ > if (page_address(to) != NULL) > #endif > __cpuc_flush_dcache_page(kto); > kunmap_atomic(kto, KM_USER1); > kunmap_atomic(kfrom, KM_USER0); > > would become something like: > > ... > kunmap_atomic_flush(kto, KM_USER1); > kunmap_atomic(kfrom, KM_USER0); > > So I think what we want to add is kunmap_atomic_flush() for the cases > where we need the additional coherency, or maybe a kunmap_atomic_noflush() > for those which we don't. So if you think about it on a VI architecture, assuming we modified some data in the kmap page at the returned address, why would we ever want to unmap without flushing? The only case I can think of is when we *know* the kmap address and the other addresses are all congruent (so we have no aliases). So I really think in kunmap(_atomic) we need to check to see if the page was modified (using the pte flags), and if it was, and it's not congruent, we flush ... otherwise we've left a dirty cache line above an unmapped area (which is an illegal condition, at least on parisc). The problems, as you rightly point out, come because most of our uses of kmap/kunmap already have a flush built into them if they modify the mapped page. However, doesn't it make sense semantically to pull all of these flushes into the unmap (as in modify a lot of code to remove the explicit flush)? What this does is have kmap/kunmap automatically do the architecturally right thing, and the user never needs to worry about flushing. James -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
