On Wed, Mar 05, 2008 at 02:55:48PM -0800, David VomLehn wrote:
> We've made significant progress in getting HIGHMEM to work on our 24K
> processor, but things do not completely work yet. Since I don't yet have
> confidence that we know everything that's going on, I"m not ready to submit
> a full-blown patch, but here's what we've done so far. Please send
> comments/suggestions...
Even a work in progress patch would be useful.
> 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.
> Our first modification was to expand the check for high memory. If the page
> had a temporary mapping, i.e. it was mapped through kmap_atomic(), we call
> flush_data_cache_page(). We then immediately return:
>
> if (PageHighMem(page)) {
> addr = (unsigned long)kmap_atomic_to_vaddr(page);
> if (addr != 0) {
> flush_data_cache_page(addr);
> }
> return;
> }
>
> (kmap_atomic_to_vaddr() returns the virtual address if the page is mapped
> with kmap_atomic(), otherwise it returns NULL). This change by itself is
> enough to be able to boot with NFS most of the time. I think it is not
> sufficient for permanently mapped kernel pages (those mapped with
> kmap_high()). So, I made two other modifications.
>
> Additional Modification #1: To me, it looks like the return should be moved
> to right after the call to flush_data_cache_page() so that we only return
> immediately for temporary kernel mappings.
>
> The next section of code, which I think already works correctly with high
> memory, is:
>
> if (mapping && !mapping_mapped(mapping)) {
> SetPageDcacheDirty(page);
> return;
> }
>
> 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.
> With the two additional modifications above, thing are still not completely
> reliable. So, two questions:
>
> 1. Does what we've done so far make sense?
> 2. Since the behavior is still somewhat flaky, I'm still missing
> something. Any suggestions?
copy_user_highpage, copy_to_user_page and copy_from_user_page could use
some review for correctness for the highmem case.
Ralf
