The function __flush_dcache_page (in arch/mips/mm/cache.c) was simply
returning if the struct page* argument it was given indicated we had a
page in high memory, so the dcache was never being flushed. This is an
obvious Bad Thing.
Sort of. It could be argued that the flushing of highmem pages should be
done on kunmap but I haven't researched that into depth.
I find it hard to get my brain around caching, but my expectation is that you
would not normally need to flush the dcache because, with physical tagging, this
all happens automatically. The case where you do need to flush the cache is
where you may do DMA on the memory you mapped, which is only part of why you use
kmap_atomic. So, most of the flushes in kunmap_atomic would be unnecessary.
People expect to have to flush the dcache before doing DMA, so they'd call
flush_dcache_page/__flush_dcache_page.
We then have the following:
addr = (unsigned long) page_address(page);
flush_data_cache_page(addr);
Additional Modification #2: If the page is in high memory, it may not
have a kernel mapping, in which case page_address() will return NULL.
So, I've modified the code to only call flush_data_cache_page() if the
page_address() doesn't return NULL.
This assumes that kunmap and kunmap_atomic flush the cache.
I think we've already taken care of the kunmap_atomic case above. For the kunmap
case, if you call __flush_dcache_page and page_address returns NULL, then don't
you have to be working with a user-mapped page? If you do DMA from a user-mapped
page, then I don't know how you come up with a virtual address to use to flush
the cache.
copy_user_highpage, copy_to_user_page and copy_from_user_page could use some review for correctness for the highmem case.
Thanks! I'll look at these, too.
Ralf
To me, this is one of the hairiest corners of the the kernel. I appreciate the
time you're taking to respond!
--
David VomLehn, dvomlehn@xxxxxxxxx
The opinions expressed herein are likely mine, but might not be my employer's...
