[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH v2] Use ballooned pages for gntdev
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. >>> As a secondary concern, assuming we don't crash, does the balloon >>> process correctly sleep until explicitly kicked when ballooned_pages >>> becomes empty? Or does it keep needlessly waking up on the jiffy timer? >>> IOW does credit in balloon_process become 0 when this situation occurs? >>> >>> Or does adjusting the balloon stats ensure this all works out alright? >> >> Adjusting the statistics should make this work out properly. Since only >> pages actually in the list will be counted in balloon_low+balloon_high, >> which is the lower bound for credit, we will always be able to satisfy >> the reservation changes requested by balloon_process. The jiffy timer is >> used only when Linux or Xen cannot satisfy the memory allocations required >> to adjust the balloon size. > > OK. > >> >>> An interesting experiment would probably be to allocate a bunch of >>> address space via gntdev and then manually balloon the guest above it's >>> current allocation, back below the lower limit due to the gntdev >>> allocation, back up to starting point etc etc. >>> >>> Ian. >>> >> >> That does sound like a useful test case. >> > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |