Re: Does HIGHMEM work on 32-bit MIPS ports?

[Ralf Baechle <ralf@linux-mips.org>]

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