WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH v2] Use ballooned pages for gntdev

To: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH v2] Use ballooned pages for gntdev
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Thu, 10 Mar 2011 13:24:39 -0500
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Delivery-date: Thu, 10 Mar 2011 10:25:13 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1299774148.17339.815.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: National Security Agency
References: <1299712054-18104-1-git-send-email-dgdegra@xxxxxxxxxxxxx> <1299748693.17339.754.camel@xxxxxxxxxxxxxxxxxxxxxx> <4D78E6AB.7030503@xxxxxxxxxxxxx> <1299772650.17339.808.camel@xxxxxxxxxxxxxxxxxxxxxx> <4D78F810.8040607@xxxxxxxxxxxxx> <1299774148.17339.815.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc13 Thunderbird/3.1.7
On 03/10/2011 11:22 AM, Ian Campbell wrote:
> On Thu, 2011-03-10 at 16:10 +0000, Daniel De Graaf wrote:
>> On 03/10/2011 10:57 AM, Ian Campbell wrote:
>>> On Thu, 2011-03-10 at 14:56 +0000, Daniel De Graaf wrote:
>>>> On 03/10/2011 04:18 AM, Ian Campbell wrote:
>>>>> On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
>>>>>> Split up the balloon module, similar to how grants and events are
>>>>>> handled. This allow gntdev to use ballooned pages without adding a
>>>>>> dependency on the balloon module.
>>>>>>
>>>>>> The interface is slightly different from that exposed in 2.6.32; the
>>>>>> page vector is allocated by the caller, not by the balloon driver. This
>>>>>> allows more freedom in how the returned pages are used, since they do
>>>>>> not all have to be returned to the balloon driver at the same time. It
>>>>>> also tries to reuse already ballooned pages rather than always
>>>>>> ballooning new pages to ensure they are in low memory as the 2.6.32
>>>>>> version did.
>>>>>>
>>>>>> Changes since v1:
>>>>>>  * Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
>>>>>>  * Better function names
>>>>>>  * Adjust balloon stats for requested pages
>>>>>>
>>>>>> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
>>>>>> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
>>>>>> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
>>>>>
>>>>> Does this introduces a new potential failure case? When a normal balloon
>>>>> operation takes place is it is now possible for the ballooned_pages list
>>>>> to be empty (because gntdev has stolen stuff from it) where before we
>>>>> knew the list was non-empty at that point?
>>>>>
>>>>> e.g. increase_reservation includes:
>>>>>           page = balloon_retrieve();
>>>>>           BUG_ON(page == NULL);
>>>>> as well as:
>>>>>   page = balloon_first_page();
>>>>>   for (i = 0; i < nr_pages; i++) {
>>>>>           BUG_ON(page == NULL);
>>>>>           frame_list[i] = page_to_pfn(page);
>>>>>           page = balloon_next_page(page);
>>>>>   }
>>>>>
>>>>> Are we sure we aren't about to exercise those BUG_ONs?
>>>>
>>>> This is safe because increase_reservation is protected by the balloon
>>>> mutex. There is a loop just before the hypercall that verifies that
>>>> there are enough pages in the list.
>>>
>>> Which loop? I don't see anything like that in either
>>> increase_reservation or the caller (balloon process).
>>>
>>
>> In increase_reservation:
>>         page = balloon_first_page();
>>         for (i = 0; i < nr_pages; i++) {
>>                 if (!page) {
>>                         nr_pages = i;
>>                         break;
>>                 }
>>                 frame_list[i] = page_to_pfn(page);
>>                 page = balloon_next_page(page);
>>         }
>>
>> This ensures that the balloon size is at least nr_pages.
> 
> I must be missing a patch, on my version this is exactly the loop with
> the BUG_ON in it that I was referring to:
>       page = balloon_first_page();
>       for (i = 0; i < nr_pages; i++) {
>               BUG_ON(page == NULL);
>               frame_list[i] = page_to_pfn(page);
>               page = balloon_next_page(page);
>       }
> 
> Ian
> 

Either way, increase_reservation is only called from balloon_process, with
nr_pages = current_target() - balloon_stats.current_pages, which is at
most balloon_stats.balloon_low + balloon_stats.balloon_high due to how
current_target() is calculated. Since all of these calculations are done
under the balloon mutex, it's impossible to trigger the BUG_ON in your
version or the break in konrad's #devel/balloon-hotplug branch.

-- 
Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel