From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page Date: Wed, 18 Dec 2013 13:30:54 +0000 Message-ID: <52B1A38E.2020500@linaro.org> References: <1387297678-17762-1-git-send-email-julien.grall@linaro.org> <1387297678-17762-9-git-send-email-julien.grall@linaro.org> <1387298217.1025.5.camel@dagon.hellion.org.uk> <52B082B0.8030903@linaro.org> <1387299773.1025.21.camel@dagon.hellion.org.uk> <1387372866.28680.14.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VtHDH-00058p-5Q for xen-devel@lists.xenproject.org; Wed, 18 Dec 2013 13:30:59 +0000 Received: by mail-wi0-f181.google.com with SMTP id hq4so611426wib.14 for ; Wed, 18 Dec 2013 05:30:57 -0800 (PST) In-Reply-To: <1387372866.28680.14.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, Tim Deegan , ian.jackson@eu.citrix.com, Stefano Stabellini , patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 12/18/2013 01:21 PM, Ian Campbell wrote: > (the cc line here is a bit odd, why Ian J? But not Tim or Stefano? I've > added those two) On Tue, 2013-12-17 at 17:02 +0000, Ian Campbell wrote: >>> For the last item, I think it's a bit stupid to create table if we are >>> removing/relinquish mapping. But I think it's an improvement for later. >>> There are lots of improvement to do in this function (eg: flushing). >> >> Agreed. > > Actually, there is an efficiency concern here. > > If we were to skip non-present first and second levels then we would > skip vast swathes of the address space very quickly. As it stands we > actively spend time filling it in just so we can recurse over it. > > This effectively turns this back into a loop over the entire gfn space, > which is what we wanted to avoid. > > The use of next_gfn_to_relinquish..max_mapped_gfn mitigates this > somewhat, but max_mapped_gfn is guest controlled and can trivially be > made huge by the guest. > > How hard would it be to skip missing entries for remove and relinquish > walks? Should just be roughtly: > > int populate = (op == INSERT || op == ALLOCATE) > > for ( addr = ... ; addr < ... ; addr ... ) > { > [...] > > if ( !first[...].valid ) > { > if ( !populate ) { > addr += skip size of a first supermapping, > continue; > } > rc = p2m_create(...) > [..error..] > > [same for second etc] > } > } > > "size of a first" needs care vs the += PAGE_SIZE in the for loop, I'd be > inclined to turn this into a while loop and move the += PAGE_SIZE to the > end. Your solution should be fine. I will add a patch in the series. -- Julien Grall