WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Vanilla Linux and has_foreign_mapping

To: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: Re: [Xen-devel] Vanilla Linux and has_foreign_mapping
From: Michael Abd-El-Malek <mabdelmalek@xxxxxxx>
Date: Fri, 25 Apr 2008 14:31:20 -0400
Cc: Mark McLoughlin <markmc@xxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Andrea Arcangeli <andrea@xxxxxxxxxxxx>, Eduardo Habkost <ehabkost@xxxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Christoph Lameter <clameter@xxxxxxx>
Delivery-date: Fri, 25 Apr 2008 11:33:48 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <48122153.1070007@xxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C4329BC3.16E37%keir.fraser@xxxxxxxxxxxxx> <48112345.5000503@xxxxxxxx> <481210C0.6070109@xxxxxxx> <48122153.1070007@xxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.12 (Windows/20080213)
Jeremy Fitzhardinge wrote:
Michael Abd-El-Malek wrote:


Jeremy Fitzhardinge wrote:
Keir Fraser wrote:
I'm not really familiar with the pv_ops code I'm afraid. But thinking about
this some more I've realised there's no way really to avoid making the
early-unpin logic aware of gntdev mappings. This is because if we do pin pte
pages, and require them to remain pinned across early-unpin, then
pgd_unpin() must not attempt to make those pte pages writable. That will fail, because the pages are still pinned! You'd either need to handle the failure to make the page writable, or have a per-page flag to indicate which pte pages contain gntdev mappings. Frankly you may as well stick with the
per-mm-context has_foreign_mappings flag.

So the issue is that a pte page containing a _PAGE_IO pte must remain pinned while it contains that mapping? Would shooting down the mapping allow it to be unpinned, or does that need to be deferred until some later point (if so, when?)?

I guess the downside is that we'd need to scan the pte looking for _PAGE_IO mappings, which is a bit of a pain. Skipping that would mean hiding a flag somewhere...

Is it a pain to add a pv_ops-subtype-specific flag to mm_context? If so you could maintain a set datastructure instead, indicating which mm_contexts
contain foreign mappings.

So, in 2.6.18-xen mm->has_foreign_mapping makes it skip early-unpin, but puts it off until pgd_free(). Presumably that works because all the vma's all been unmapped by then...
The following patch was sufficient for me. I delayed the arch_exit_mmap (which eventually calls into xen) until after unmap_vmas is called, which calls zap_pte (where I unmap the grant). Presumably, there is a performance overhead to always doing this delay, and hence 2.6.18 only did the delay if has_foreign_mappings is set. For macrobenchmarks like compilation, I couldn't find a difference.

Cheers,
Mike

diff --git a/mm/mmap.c b/mm/mmap.c
index a32d28c..c118b54 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2036,15 +2036,14 @@ void exit_mmap(struct mm_struct *mm)
     unsigned long nr_accounted = 0;
     unsigned long end;

-    /* mm's last user has gone, and its about to be pulled down */
-    arch_exit_mmap(mm);
-
     lru_add_drain();
     flush_cache_mm(mm);
     tlb = tlb_gather_mmu(mm, 1);
     /* Don't update_hiwater_rss(mm) here, do_exit already did */
     /* Use -1 here to ensure all VMAs in the mm are unmapped */
     end = unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);
+    /* mm's last user has gone, and its about to be pulled down */
+    arch_exit_mmap(mm);

Yeah, that pretty much removes the point of "early unpin". The problem is that unmap_vmas tears down the pagetable, and if its pinned that means lots of hypercalls. If it has already been unpinned it can be much more efficient.

It mainly effects exec/exit performance, and I'm not sure a kernel compile is all that execy/exity - most of the time is just spent in gcc crunching. Something with more shellscripts might show more of a difference. (I have to admit I haven't benchmarked this myself.)

   J

How about we do the following:

arch_exit_mmap_pre(mm);

lru_add_drain();
flush_cache_mm(mm);
tlb = tlb_gather_mmu(mm, 1);
/* Don't update_hiwater_rss(mm) here, do_exit already did */
/* Use -1 here to ensure all VMAs in the mm are unmapped */
end = unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);

arch_exit_mmap_post(mm);

We'll reintroduce has_foreign_mappings. If has_foreign_mappings is _not_ set, then arch_exit_mmap_pre can early unpin the page tables and arch_exit_mmap_post will do nothing. If has_foreign_mappings is set, then arch_exist_mmap_pre won't do anything, and arch_exit_mmap_post will do the actual xen_exit_mmap call.

What do you think?

Mike

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel