From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Andres Lagar-Cavilla" <andres@lagarcavilla.org>
Subject: Re: [PATCH 2 of 3] x86/mem_sharing: modularize reverse
 map for shared frames
Date: Wed, 18 Apr 2012 07:19:57 -0700
Message-ID: <f73ef505cf67b715cbf37b6f63a4db3e.squirrel@webmail.lagarcavilla.org>
References: <patchbomb.1334240171@xdev.gridcentric.ca>
	<1730bff8fccfb08cee3f.1334240173@xdev.gridcentric.ca>
	<20120418140550.GH7013@ocelot.phlegethon.org>
Reply-To: andres@lagarcavilla.org
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20120418140550.GH7013@ocelot.phlegethon.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Tim Deegan <tim@xen.org>
Cc: adin@gridcentric.ca, andres@gridcentric.ca, keir.xen@gmail.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

> At 10:16 -0400 on 12 Apr (1334225773), Andres Lagar-Cavilla wrote:
>>      /* Do the accounting first. If anything fails below, we have bigger
>>       * bigger fish to fry. First, remove the gfn from the list. */
>> -    last_gfn = list_has_one_entry(&page->sharing->gfns);
>> +    last_gfn = rmap_has_one_entry(page);
>>      if ( last_gfn )
>>      {
>> -        /* Clean up shared state */
>> -        audit_del_list(page);
>> +        /* Clean up shared state. Get rid of the <domid, gfn> tuple
>> +         * before destroying the rmap. */
>> +        mem_sharing_gfn_destroy(page, d, gfn_info);
>
> Moving this mem_sharing_gfn_destroy() around seems like it's unrelated
> to the rest of this patch, which is basically code motion.  If so, can
> you put it in a patch of its own?

Sure. I'll split it up but wait for feedback on 3.3 before re-submit.
Andres

>
> Otherwise, pending patch 3/3 being OK too,
>
> Acked-by: Tim Deegan <tim@xen.org>
>
>> +        page_sharing_dispose(page);
>>          page->sharing = NULL;
>>          atomic_dec(&nr_shared_mfns);
>>      }
>>      else
>>          atomic_dec(&nr_saved_mfns);
>> +
>>      /* If the GFN is getting destroyed drop the references to MFN
>>       * (possibly freeing the page), and exit early */
>>      if ( flags & MEM_SHARING_DESTROY_GFN )
>>      {
>> -        mem_sharing_gfn_destroy(d, gfn_info);
>> +        if ( !last_gfn )
>> +            mem_sharing_gfn_destroy(page, d, gfn_info);
>>          put_page_and_type(page);
>>          mem_sharing_page_unlock(page);
>>          if ( last_gfn &&
>> @@ -987,7 +1052,6 @@ gfn_found:
>>
>>      if ( last_gfn )
>>      {
>> -        mem_sharing_gfn_destroy(d, gfn_info);
>>          /* Making a page private atomically unlocks it */
>>          BUG_ON(page_make_private(d, page) != 0);
>>          goto private_page_found;
>> @@ -1011,7 +1075,7 @@ gfn_found:
>>      unmap_domain_page(t);
>>
>>      BUG_ON(set_shared_p2m_entry(d, gfn, page_to_mfn(page)) == 0);
>> -    mem_sharing_gfn_destroy(d, gfn_info);
>> +    mem_sharing_gfn_destroy(old_page, d, gfn_info);
>>      mem_sharing_page_unlock(old_page);
>>      put_page_and_type(old_page);
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>