Re: [PATCH v5 1/4] xen: introduce a helper to allocate non-contiguous memory

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 08/05/15 15:34, Roger Pau Monne wrote:
> The allocator uses independent calls to alloc_domheap_pages in order to get
> the desired amount of memory and then maps all the independent physical
> addresses into a contiguous virtual address space.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Tested-by: Julien Grall <julien.grall@citrix.com> (ARM)
> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes since v4:
>  - Remove the red-black tree, the same can be achieved by doing a VA -> MFN
>    translation.
>  - Provide a vzalloc that zeroes the allocated memory area.
>  - Allocate domheap annonymous pages.
> ---
>  xen/common/vmap.c          |  2 +-
>  xen/common/xmalloc_tlsf.c  | 62 ++++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/mm.h   |  2 ++
>  xen/include/asm-x86/page.h |  2 ++
>  xen/include/xen/vmap.h     |  1 +
>  xen/include/xen/xmalloc.h  |  4 +++
>  6 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
> index 739d468..248ceec 100644
> --- a/xen/common/vmap.c
> +++ b/xen/common/vmap.c
> @@ -146,7 +146,7 @@ static unsigned int vm_index(const void *va)
>             test_bit(idx, vm_bitmap) ? idx : 0;
>  }
>  
> -static unsigned int vm_size(const void *va)
> +unsigned int vm_size(const void *va)
>  {
>      unsigned int start = vm_index(va), end;
>  
> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> index b13317e..fe3032e 100644
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -27,6 +27,7 @@
>  #include <xen/irq.h>
>  #include <xen/mm.h>
>  #include <xen/pfn.h>
> +#include <xen/vmap.h>
>  #include <asm/time.h>
>  
>  #define MAX_POOL_NAME_LEN       16
> @@ -613,6 +614,13 @@ void *_xzalloc(unsigned long size, unsigned long align)
>      return p ? memset(p, 0, size) : p;
>  }
>  
> +void *vzalloc(unsigned long size)
> +{
> +    void *p = vmalloc(size);
> +
> +    return p ? memset(p, 0, size) : p;
> +}
> +
>  void xfree(void *p)
>  {
>      struct bhdr *b;
> @@ -651,3 +659,57 @@ void xfree(void *p)
>  
>      xmem_pool_free(p, xenpool);
>  }
> +
> +void *vmalloc(unsigned long size)
> +{
> +    unsigned long *mfn;
> +    unsigned long pages, i;
> +    struct page_info *pg;
> +    void *va = NULL;
> +
> +    ASSERT(!in_irq());
> +
> +    if ( size == 0 )
> +        return ZERO_BLOCK_PTR;
> +
> +    pages = DIV_ROUND_UP(size, PAGE_SIZE);
> +    mfn = xzalloc_array(unsigned long, pages);

This can avoid the zeroing if...

> +    if ( mfn == NULL )
> +        return NULL;
> +
> +    for ( i = 0; i < pages; i++ )
> +    {
> +        pg = alloc_domheap_pages(NULL, 1, 0);
> +        if ( pg == NULL )
> +            goto error;
> +        mfn[i] = page_to_mfn(pg);
> +    }
> +
> +    va = vmap(mfn, pages);
> +    if ( va == NULL )
> +        goto error;
> +
> +    xfree(mfn);
> +    return va;
> +
> + error:
> +    vunmap(va);
> +    for ( i = 0; i < pages; i++ )
> +        if ( mfn[i] != 0 )
> +            free_domheap_pages(mfn_to_page(mfn[i]), 1);

this loop turns into

while ( i )
    free_domheap_pages(mfn_to_page(mfn[--i], 1);

and even avoids the conditional.

What is the expected max value of pages?  Given that our hypervisor
stacks are quite large, it might be prudent to use C99 variable length
arrays (with an appropriate BUG_ON()) to avoid the xalloc() altogether.

> +    xfree(mfn);
> +    return NULL;
> +}
> +
> +void vfree(void *va)
> +{
> +    unsigned int i, pages = vm_size(va);
> +
> +    if ( pages == 0 )
> +        return;
> +
> +    for ( i = 0; i < pages; i++ )
> +        free_domheap_pages(vmap_to_page(va + i * PAGE_SIZE), 1);
> +
> +    vunmap(va);
> +}
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index d25e485..c0afcec 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -208,6 +208,8 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len)
>  #define pfn_to_paddr(pfn) ((paddr_t)(pfn) << PAGE_SHIFT)
>  #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
>  #define paddr_to_pdx(pa)    pfn_to_pdx(paddr_to_pfn(pa))
> +#define vmap_to_mfn(va)     paddr_to_pfn(virt_to_maddr((vaddr_t)va))
> +#define vmap_to_page(va)    mfn_to_page(vmap_to_mfn(va))
>  
>  /* Page-align address and convert to frame number format */
>  #define paddr_to_pfn_aligned(paddr)    paddr_to_pfn(PAGE_ALIGN(paddr))
> diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
> index a8bc999..8977a76 100644
> --- a/xen/include/asm-x86/page.h
> +++ b/xen/include/asm-x86/page.h
> @@ -262,6 +262,8 @@ void copy_page_sse2(void *, const void *);
>  #define pfn_to_paddr(pfn)   __pfn_to_paddr(pfn)
>  #define paddr_to_pfn(pa)    __paddr_to_pfn(pa)
>  #define paddr_to_pdx(pa)    pfn_to_pdx(paddr_to_pfn(pa))
> +#define vmap_to_mfn(va)     l1e_get_pfn(*virt_to_xen_l1e((unsigned long)(va)))
> +#define vmap_to_page(va)    mfn_to_page(vmap_to_mfn(va))
>  
>  #endif /* !defined(__ASSEMBLY__) */
>  
> diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h
> index b1923dd..bc5c862 100644
> --- a/xen/include/xen/vmap.h
> +++ b/xen/include/xen/vmap.h
> @@ -11,6 +11,7 @@ void *__vmap(const unsigned long *mfn, unsigned int granularity,
>               unsigned int nr, unsigned int align, unsigned int flags);
>  void *vmap(const unsigned long *mfn, unsigned int nr);
>  void vunmap(const void *);
> +unsigned int vm_size(const void *va);
>  
>  void __iomem *ioremap(paddr_t, size_t);
>  
> diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
> index 24a99ac..be157bf 100644
> --- a/xen/include/xen/xmalloc.h
> +++ b/xen/include/xen/xmalloc.h
> @@ -30,6 +30,10 @@ extern void xfree(void *);
>  extern void *_xmalloc(unsigned long size, unsigned long align);
>  extern void *_xzalloc(unsigned long size, unsigned long align);
>  
> +void *vmalloc(unsigned long size);
> +void *vzalloc(unsigned long size);

I would suggest size_t to for closer consistency with C.

~Andrew

> +void vfree(void *va);
> +
>  static inline void *_xmalloc_array(
>      unsigned long size, unsigned long align, unsigned long num)
>  {


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel