On Thu, Dec 10, 2009 at 11:59:16AM -0600, James Bottomley wrote: > On Thu, 2009-12-10 at 17:48 +0000, Russell King wrote: > > 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). The above example code comes from non-aliasing VIPT - where for the vast majority of cases, unmapping without flush is fine provided we haven't written data. However, unmapping with flush is required to ensure coherency with the instruction cache. > So I really think in kunmap(_atomic) we need to check to see if the page > was modified (using the pte flags), That's fine _if_ you have such flags. Not everything has - in which case, going down the route you're proposing means that every single kunmap_atomic() ends up having to flush the whole page whether it's actually needed on an architecture "just because" - with no technical reason to actually do so. We need the two cases separated for hardware which is not PARISC. -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: -- 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
