From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] libxc: refactor memory allocation functions Date: Tue, 1 Dec 2015 12:45:55 +0100 Message-ID: <565D8873.4010908@suse.com> References: <1448969956-26310-1-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1a3jNf-000184-1w for xen-devel@lists.xenproject.org; Tue, 01 Dec 2015 11:45:59 +0000 In-Reply-To: <1448969956-26310-1-git-send-email-wei.liu2@citrix.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: Wei Liu , Xen-devel Cc: Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 01/12/15 12:39, Wei Liu wrote: > There were some problems with the original memory allocation functions: > 1. xc_dom_alloc_segment and xc_dom_alloc_pad ended up calling > xc_dom_chk_alloc_pages while xc_dom_alloc_page open-coded everything. > 2. xc_dom_alloc_pad didn't call dom->allocate. > > Refactor the code so that: > 1. xc_dom_alloc_{segment,pad,page} end up calling > xc_dom_chk_alloc_pages. > 2. xc_dom_chk_alloc_pages calls dom->allocate. > > This way we avoid scattering dom->allocate over multiple locations and > open-coding. > > Also change the return type of xc_dom_alloc_page to xen_pfn_t and return > an invalid pfn when xc_dom_chk_alloc_pages fails. > > Signed-off-by: Wei Liu Reviewed-by: Juergen Gross > --- > Cc: Ian Campbell > Cc: Ian Jackson > Cc: Juergen Gross > > We don't have INVALID_PFN, maybe we need one? > > Tested with Jessie with 64 bit pvgrub, Wheezy with 64 bit pvgrub, Wheezy pv > guest, Wheezy HVM guest, Wheezy HVM with qemu stubdom. > > tools/libxc/include/xc_dom.h | 2 +- > tools/libxc/xc_dom_core.c | 16 +++++++--------- > 2 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h > index 2176216..2d0de8c 100644 > --- a/tools/libxc/include/xc_dom.h > +++ b/tools/libxc/include/xc_dom.h > @@ -350,7 +350,7 @@ char *xc_dom_strdup(struct xc_dom_image *dom, const char *str); > > /* --- alloc memory pool ------------------------------------------- */ > > -int xc_dom_alloc_page(struct xc_dom_image *dom, char *name); > +xen_pfn_t xc_dom_alloc_page(struct xc_dom_image *dom, char *name); > int xc_dom_alloc_segment(struct xc_dom_image *dom, > struct xc_dom_seg *seg, char *name, > xen_vaddr_t start, xen_vaddr_t size); > diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c > index 5d6c3ba..8967970 100644 > --- a/tools/libxc/xc_dom_core.c > +++ b/tools/libxc/xc_dom_core.c > @@ -555,6 +555,9 @@ static int xc_dom_chk_alloc_pages(struct xc_dom_image *dom, char *name, > dom->pfn_alloc_end += pages; > dom->virt_alloc_end += pages * page_size; > > + if ( dom->allocate ) > + dom->allocate(dom); > + > return 0; > } > > @@ -602,9 +605,6 @@ int xc_dom_alloc_segment(struct xc_dom_image *dom, > if ( xc_dom_chk_alloc_pages(dom, name, pages) ) > return -1; > > - if (dom->allocate) > - dom->allocate(dom); > - > /* map and clear pages */ > ptr = xc_dom_seg_to_ptr(dom, seg); > if ( ptr == NULL ) > @@ -621,18 +621,16 @@ int xc_dom_alloc_segment(struct xc_dom_image *dom, > return 0; > } > > -int xc_dom_alloc_page(struct xc_dom_image *dom, char *name) > +xen_pfn_t xc_dom_alloc_page(struct xc_dom_image *dom, char *name) > { > - unsigned int page_size = XC_DOM_PAGE_SIZE(dom); > xen_vaddr_t start; > xen_pfn_t pfn; > > start = dom->virt_alloc_end; > pfn = dom->pfn_alloc_end - dom->rambase_pfn; > - dom->virt_alloc_end += page_size; > - dom->pfn_alloc_end++; > - if ( dom->allocate ) > - dom->allocate(dom); > + > + if ( xc_dom_chk_alloc_pages(dom, name, 1) ) > + return (xen_pfn_t)-1; > > DOMPRINTF("%-20s: %-12s : 0x%" PRIx64 " (pfn 0x%" PRIpfn ")", > __FUNCTION__, name, start, pfn); >