From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Abd-El-Malek Subject: Re: Vanilla Linux and has_foreign_mapping Date: Fri, 25 Apr 2008 14:31:20 -0400 Message-ID: <48122378.2090802@cmu.edu> References: <48112345.5000503@goop.org> <481210C0.6070109@cmu.edu> <48122153.1070007@goop.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <48122153.1070007@goop.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jeremy Fitzhardinge Cc: Mark McLoughlin , xen-devel , Andrea Arcangeli , Eduardo Habkost , Keir Fraser , Christoph Lameter List-Id: xen-devel@lists.xenproject.org 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