Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032084AbdDZWpn (ORCPT ); Wed, 26 Apr 2017 18:45:43 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:47164 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1032064AbdDZWpf (ORCPT ); Wed, 26 Apr 2017 18:45:35 -0400 Subject: Re: xen_exit_mmap() questions To: Andy Lutomirski , "xen-devel@lists.xenproject.org" , Juergen Gross References: Cc: X86 ML , Borislav Petkov , "linux-kernel@vger.kernel.org" From: Boris Ostrovsky Message-ID: Date: Wed, 26 Apr 2017 18:45:18 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1290 Lines: 36 On 04/26/2017 04:52 PM, Andy Lutomirski wrote: > I was trying to understand xen_drop_mm_ref() to update it for some > changes I'm working on, and I'm wondering whether we need > xen_exit_mmap() at all. > > AFAICS the intent is to force all CPUs to drop their lazy uses of the > mm being destroyed so it can be unpinned before tearing down the page > tables, thus making it faster to tear down the page tables. This > seems like it'll speed up xen_set_pud() and xen_set_pmd(), but this > seems like it may be of rather limited value. Why do you think it's of limited value? Without it we will end up with a hypercall for each update. Or is your point that the number of those update is relatively small when we are tearing down? > Could we get away with > deleting it? > > Also, this code in drop_other_mm_ref() looks dubious to me: > > /* If this cpu still has a stale cr3 reference, then make sure > it has been flushed. */ > if (this_cpu_read(xen_current_cr3) == __pa(mm->pgd)) > load_cr3(swapper_pg_dir); > > If cr3 hasn't been flushed to the hypervisor because we're in a lazy > mode, why would load_cr3() help? Shouldn't this be xen_mc_flush() > instead? load_cr3() actually ends with xen_mc_flush() by way of xen_write_cr3() -> xen_mc_issue(). -boris