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