[parisc-linux] Re: RFC: mmap patch

Matthew Wilcox willy@debian.org
Thu, 6 Mar 2003 14:31:49 +0000


On Thu, Mar 06, 2003 at 07:14:53AM -0700, John Marvin wrote:
> In my opinion this patch is a hack workaround for a real bug. parisc is
> not the only architecture that has virtual tagged caches.  Some mips
> machines, sparc and ultrasparc machines have virtual tagged caches,
> although none of them have virtual tagged caches as large as parisc (the
> other architectures typically have a larger physical cache at a higher
> level in the cache hierarchy).
> 
> Dave Miller designed the cache flushing strategy to have hooks in the
> machine independent code to support virtual tagged caches.  Probably there
> is simply a cache flush that is missing that doesn't show itself as a bug
> as easily on the smaller virtually tagged caches of the other
> architectures. At most this particular scenario wasn't considered
> for virtual tagged caches (maintaining coherence between fd writes and
> mmap'ed regions) and will require a design change to fix.
> 
> The fix for this bug should be made in machine independent code, not in
> our machine dependent code.

Unfortunately, the flush is in the right place according to the
definition.  Here's how it looks in 2.4 (2.5 is more complex but has
essentially the same flush in it):

generic_file_write(struct file *file,const char *buf,size_t count, loff_t *ppos)
{
[...]
                kaddr = kmap(page);
                status = mapping->a_ops->prepare_write(file, page, offset, offse
t+bytes);
                if (status)
                        goto sync_failure;
                page_fault = __copy_from_user(kaddr+offset, buf, bytes);
                flush_dcache_page(page);
                status = mapping->a_ops->commit_write(file, page, offset, offset
+bytes);

So our flush_dcache_page() flushes the kernel's view of that page,
no problem.  Memory now has the right contents.  But the dcache still
has the stale data in it for the user's mapping of the same page.

There's a few ways to fix it.

1) This patch tausq did.  Now we flush both user & kernel mappings for the
data.

2) Split flush_dcache_page into flush_dcache_page_user_mapping() and
flush_dcache_page_kernel_mapping().  Hopefully with better names.

3) (ab)use kmap to hand back an address which virtually aliases the user
mapping.  flush_dcache_page() would only have to writeback to memory if
the page was not mapped.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk