lkml.org 
[lkml]   [2006]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/3] Fix COW D-cache aliasing on fork
On Sat, Oct 21, 2006 at 12:39:40AM +1000, Nick Piggin wrote:

> >>That would require changing the order of cache flush and tlb flush.
> >>To keep certain architectures that require a valid translation in
> >>the TLB the cacheflush has to be done first. Not sure if those
> >>architectures need a writeable mapping for dirty cachelines - I
> >>think hypersparc was one of them.
> >
> >
> >There just has to be "a mapping" in the TLB so that the L2 cache can
> >translate the virtual address to a physical one for the writeback to
> >main memory.
>
> So moving the flush_cache_mm below the copy_page_range, to just
> before the flush_tlb_mm, would work then? This would make the
> race much smaller than with this patchset.

90% of this changeset are MIPS-specific code. Of that in turn much is
just infrastructure which is already being used anyway.

> But doesn't that still leave a race?

Both calls would have to be done under the mmap_sem to close any races.

> What if another thread writes to cache after we have flushed it
> but before flushing the TLBs? Although we've marked the the ptes
> readonly, the CPU won't trap if the TLB is valid? There must be
> some special way for the arch to handle this, but I can't see it.

There isn't really. Reordering with a patch like:

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/kernel/fork.c b/kernel/fork.c
index 29ebb30..28e51e0 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -202,7 +202,6 @@ static inline int dup_mmap(struct mm_str
struct mempolicy *pol;

down_write(&oldmm->mmap_sem);
- flush_cache_mm(oldmm);
/*
* Not linked in yet - no deadlock potential:
*/
@@ -287,8 +286,9 @@ static inline int dup_mmap(struct mm_str
}
retval = 0;
out:
- up_write(&mm->mmap_sem);
flush_tlb_mm(oldmm);
+ flush_cache_mm(oldmm);
+ up_write(&mm->mmap_sem);
up_write(&oldmm->mmap_sem);
return retval;
fail_nomem_policy:
should close the hole for all effected architectures. I say should
because this patch would need another round of linux-arch reviewing and I
haven't tested it this patch yet myself.

But even so that doesn't change that I would really like to make
copy_user_highpage() an arch interface replacing copy_to_user_page.

The current way of doing things enforces a cacheflush on MIPS which itself
is pricy - 1,000 cycles when it's cheap but could be several times as
expensive. And as a side effect of the cacheflush the process breaking
a COW page will start with a cold page.

Or if an architecture wants to be clever about aliasing and uses the
vto argument of copy_user_page to create a non-conflicting mapping it
means the mapping setup by copy_user_highpage will be unused ...

Ralf
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-10-20 18:07    [W:0.106 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site