Re: [PATCH 6/6] mm: remove some update_mmu_cache() calls

[Hugh Dickins]

On Mon, 19 Jun 2006, Peter Zijlstra wrote:
> From: Christoph Lameter <clameter@xxxxxxx>
> 
> This may be a bit controversial but it does not seem to
> make sense to use the update_mmu_cache macro when we reuse
> the page. We are only fiddling around with the protections,
> the dirty and accessed bits.

Controversial indeed.  Just because Christoph tentatively puts forward
such a patch is no reason to include it in your dirty page series.

Probably my fault for drawing attention to the ill-defined relationship
between update_mmu_cache and more recently added lazy_mmu_prot_update,
and wondering aloud on what's different between do_wp_page and mprotect.

If you want to pursue this patch, you should address it to someone
experienced in update_mmu_cache, someone familiar with those
architectures which implement it: ideally to Dave Miller (CCed),
who designed and wrote most of Documentation/cachetlb.txt.

The answer I expect is that update_mmu_cache is essential there in
do_wp_page (reuse case) and handle_pte_fault, on at least some if
not all of those arches which implement it.  That without those
lines, they'll fault and refault endlessly, since the "MMU cache"
has not been updated with the write permission.

But omitted from mprotect, since that's dealing with a batch of
pages, perhaps none of which will be faulted in the near future:
a waste of resources to update for all those entries.

But now I wonder, why does do_wp_page reuse case flush_cache_page?

Hugh

> With the call to update_mmu_cache the way of using the macros
> would be different from mprotect() and page_mkclean(). I'd
> rather have everything work the same way. If this breaks on some
> arches then also mprotect and page_mkclean() are broken.
> The use of mprotect() is rare, we may have breakage in some
> arches that we just have not seen yet.
> 
> Signed-off-by: Christoph Lameter <clameter@xxxxxxx>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> ---
>  mm/memory.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> Index: 2.6-mm/mm/memory.c
> ===================================================================
> --- 2.6-mm.orig/mm/memory.c	2006-06-19 16:21:16.000000000 +0200
> +++ 2.6-mm/mm/memory.c	2006-06-19 16:21:25.000000000 +0200
> @@ -1514,7 +1514,6 @@ static int do_wp_page(struct mm_struct *
>  		entry = pte_mkyoung(orig_pte);
>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>  		ptep_set_access_flags(vma, address, page_table, entry, 1);
> -		update_mmu_cache(vma, address, entry);
>  		lazy_mmu_prot_update(entry);
>  		ret |= VM_FAULT_WRITE;
>  		goto unlock;
> @@ -2317,7 +2316,6 @@ static inline int handle_pte_fault(struc
>  	entry = pte_mkyoung(entry);
>  	if (!pte_same(old_entry, entry)) {
>  		ptep_set_access_flags(vma, address, pte, entry, write_access);
> -		update_mmu_cache(vma, address, entry);
>  		lazy_mmu_prot_update(entry);
>  	} else {
>  		/*
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/