Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-29 Thread Juergen Gross
On 25/05/18 17:33, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Memory {increase|decrease}_reservation and VA mappings update/reset
> code used in balloon driver can be made common, so other drivers can
> also re-use the same functionality without open-coding.
> Create a dedicated module for the shared code and export corresponding
> symbols for other kernel modules.
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  drivers/xen/Makefile  |   1 +
>  drivers/xen/balloon.c |  71 ++
>  drivers/xen/mem-reservation.c | 134 ++
>  include/xen/mem_reservation.h |  29 
>  4 files changed, 170 insertions(+), 65 deletions(-)
>  create mode 100644 drivers/xen/mem-reservation.c
>  create mode 100644 include/xen/mem_reservation.h

Can you please name this include/xen/mem-reservation.h ?

> 
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 451e833f5931..3c87b0c3aca6 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_HOTPLUG_CPU)+= cpu_hotplug.o
>  obj-$(CONFIG_X86)+= fallback.o
>  obj-y+= grant-table.o features.o balloon.o manage.o preempt.o time.o
> +obj-y+= mem-reservation.o
>  obj-y+= events/
>  obj-y+= xenbus/
>  
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 065f0b607373..57b482d67a3a 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -71,6 +71,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  static int xen_hotplug_unpopulated;
>  
> @@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, 
> balloon_process);
>  #define GFP_BALLOON \
>   (GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)
>  
> -static void scrub_page(struct page *page)
> -{
> -#ifdef CONFIG_XEN_SCRUB_PAGES
> - clear_highpage(page);
> -#endif
> -}
> -
>  /* balloon_append: add the given page to the balloon. */
>  static void __balloon_append(struct page *page)
>  {
> @@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long 
> nr_pages)
>   int rc;
>   unsigned long i;
>   struct page   *page;
> - struct xen_memory_reservation reservation = {
> - .address_bits = 0,
> - .extent_order = EXTENT_ORDER,
> - .domid= DOMID_SELF
> - };
>  
>   if (nr_pages > ARRAY_SIZE(frame_list))
>   nr_pages = ARRAY_SIZE(frame_list);
> @@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long 
> nr_pages)
>   page = balloon_next_page(page);
>   }
>  
> - set_xen_guest_handle(reservation.extent_start, frame_list);
> - reservation.nr_extents = nr_pages;
> - rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, );
> + rc = xenmem_reservation_increase(nr_pages, frame_list);
>   if (rc <= 0)
>   return BP_EAGAIN;
>  
> @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long 
> nr_pages)
>   page = balloon_retrieve(false);
>   BUG_ON(page == NULL);
>  
> -#ifdef CONFIG_XEN_HAVE_PVMMU
> - /*
> -  * We don't support PV MMU when Linux and Xen is using
> -  * different page granularity.
> -  */
> - BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
> -
> - if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> - unsigned long pfn = page_to_pfn(page);
> -
> - set_phys_to_machine(pfn, frame_list[i]);
> -
> - /* Link back into the page tables if not highmem. */
> - if (!PageHighMem(page)) {
> - int ret;
> - ret = HYPERVISOR_update_va_mapping(
> - (unsigned long)__va(pfn << 
> PAGE_SHIFT),
> - mfn_pte(frame_list[i], 
> PAGE_KERNEL),
> - 0);
> - BUG_ON(ret);
> - }
> - }
> -#endif
> + xenmem_reservation_va_mapping_update(1, , _list[i]);
>  
>   /* Relinquish the page back to the allocator. */
>   free_reserved_page(page);
> @@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long 
> nr_pages, gfp_t gfp)
>   unsigned long i;
>   struct page *page, *tmp;
>   int ret;
> - struct xen_memory_reservation reservation = {
> - .address_bits = 0,
> - .extent_order = EXTENT_ORDER,
> - .domid= DOMID_SELF
> - };
>   LIST_HEAD(pages);
>  
>   if (nr_pages > ARRAY_SIZE(frame_list))
> @@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long 
> nr_pages, gfp_t gfp)
>   break;
>   }
>   adjust_managed_page_count(page, 

Re: [PATCH 1/8] xen/grant-table: Make set/clear page private code shared

2018-05-29 Thread Juergen Gross
On 25/05/18 17:33, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Make set/clear page private code shared and accessible to
> other kernel modules which can re-use these instead of open-coding.
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  drivers/xen/grant-table.c | 54 +--
>  include/xen/grant_table.h |  3 +++
>  2 files changed, 38 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 27be107d6480..d7488226e1f2 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
>  }
>  EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
>  
> -/**
> - * gnttab_alloc_pages - alloc pages suitable for grant mapping into
> - * @nr_pages: number of pages to alloc
> - * @pages: returns the pages
> - */
> -int gnttab_alloc_pages(int nr_pages, struct page **pages)
> +int gnttab_pages_set_private(int nr_pages, struct page **pages)
>  {
>   int i;
> - int ret;
> -
> - ret = alloc_xenballooned_pages(nr_pages, pages);
> - if (ret < 0)
> - return ret;
>  
>   for (i = 0; i < nr_pages; i++) {
>  #if BITS_PER_LONG < 64
>   struct xen_page_foreign *foreign;
>  
>   foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
> - if (!foreign) {
> - gnttab_free_pages(nr_pages, pages);
> + if (!foreign)
>   return -ENOMEM;
> - }
> +
>   set_page_private(pages[i], (unsigned long)foreign);
>  #endif
>   SetPagePrivate(pages[i]);
> @@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page 
> **pages)
>  
>   return 0;
>  }
> -EXPORT_SYMBOL(gnttab_alloc_pages);
> +EXPORT_SYMBOL(gnttab_pages_set_private);

EXPORT_SYMBOL_GPL()

>  
>  /**
> - * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
> - * @nr_pages; number of pages to free
> - * @pages: the pages
> + * gnttab_alloc_pages - alloc pages suitable for grant mapping into
> + * @nr_pages: number of pages to alloc
> + * @pages: returns the pages
>   */
> -void gnttab_free_pages(int nr_pages, struct page **pages)
> +int gnttab_alloc_pages(int nr_pages, struct page **pages)
> +{
> + int ret;
> +
> + ret = alloc_xenballooned_pages(nr_pages, pages);
> + if (ret < 0)
> + return ret;
> +
> + ret = gnttab_pages_set_private(nr_pages, pages);
> + if (ret < 0)
> + gnttab_free_pages(nr_pages, pages);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(gnttab_alloc_pages);
> +
> +void gnttab_pages_clear_private(int nr_pages, struct page **pages)
>  {
>   int i;
>  
> @@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
>   ClearPagePrivate(pages[i]);
>   }
>   }
> +}
> +EXPORT_SYMBOL(gnttab_pages_clear_private);

EXPORT_SYMBOL_GPL()


Juergen