Re: [Xen-devel] BUG: using smp_processor_id() in preemptible [00000000] code: blkback.1.xvdb/9138 caller is decrease_reservation

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 11 Sep 2013, David Vrabel wrote:
> xen/balloon: ensure preemption is disabled when using a scratch page
> 
> In decrease_reservation(), if the kernel is preempted between updating
> the mapping and updating the p2m then they may end up using different
> scratch pages.
> 
> Use get_balloon_scratch_page() and put_balloon_scratch_page() which use
> get_cpu_var() and put_cpu_var() to correctly disable preemption.
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>

The patch makes sense.

Reviewed-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

Do you want me to add it to the queue or are you doing it?


> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 3101cf6..a74647b 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -305,6 +305,18 @@ static enum bp_state reserve_additional_memory(long 
> credit)
>  }
>  #endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
>  
> +struct page *get_balloon_scratch_page(void)
> +{
> +     struct page *ret = get_cpu_var(balloon_scratch_page);
> +     BUG_ON(ret == NULL);
> +     return ret;
> +}
> +
> +void put_balloon_scratch_page(void)
> +{
> +     put_cpu_var(balloon_scratch_page);
> +}
> +
>  static enum bp_state increase_reservation(unsigned long nr_pages)
>  {
>       int rc;
> @@ -380,6 +392,7 @@ static enum bp_state decrease_reservation(unsigned long 
> nr_pages, gfp_t gfp)
>       enum bp_state state = BP_DONE;
>       unsigned long  pfn, i;
>       struct page   *page;
> +     struct page   *scratch_page;
>       int ret;
>       struct xen_memory_reservation reservation = {
>               .address_bits = 0,
> @@ -399,6 +412,8 @@ static enum bp_state decrease_reservation(unsigned long 
> nr_pages, gfp_t gfp)
>       if (nr_pages > ARRAY_SIZE(frame_list))
>               nr_pages = ARRAY_SIZE(frame_list);
>  
> +     scratch_page = get_balloon_scratch_page();
> +
>       for (i = 0; i < nr_pages; i++) {
>               page = alloc_page(gfp);
>               if (page == NULL) {
> @@ -416,7 +431,7 @@ static enum bp_state decrease_reservation(unsigned long 
> nr_pages, gfp_t gfp)
>               if (xen_pv_domain() && !PageHighMem(page)) {
>                       ret = HYPERVISOR_update_va_mapping(
>                               (unsigned long)__va(pfn << PAGE_SHIFT),
> -                             
> pfn_pte(page_to_pfn(__get_cpu_var(balloon_scratch_page)),
> +                             pfn_pte(page_to_pfn(scratch_page),
>                                       PAGE_KERNEL_RO), 0);
>                       BUG_ON(ret);
>               }
> @@ -432,14 +447,14 @@ static enum bp_state decrease_reservation(unsigned long 
> nr_pages, gfp_t gfp)
>               pfn = mfn_to_pfn(frame_list[i]);
>               if (!xen_feature(XENFEAT_auto_translated_physmap)) {
>                       unsigned long p;
> -                     struct page *pg;
> -                     pg = __get_cpu_var(balloon_scratch_page);
> -                     p = page_to_pfn(pg);
> +                     p = page_to_pfn(scratch_page);
>                       __set_phys_to_machine(pfn, pfn_to_mfn(p));
>               }
>               balloon_append(pfn_to_page(pfn));
>       }
>  
> +     put_balloon_scratch_page();
> +
>       set_xen_guest_handle(reservation.extent_start, frame_list);
>       reservation.nr_extents   = nr_pages;
>       ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
> @@ -491,18 +506,6 @@ static void balloon_process(struct work_struct *work)
>       mutex_unlock(&balloon_mutex);
>  }
>  
> -struct page *get_balloon_scratch_page(void)
> -{
> -     struct page *ret = get_cpu_var(balloon_scratch_page);
> -     BUG_ON(ret == NULL);
> -     return ret;
> -}
> -
> -void put_balloon_scratch_page(void)
> -{
> -     put_cpu_var(balloon_scratch_page);
> -}
> -
>  /* Resets the Xen limit, sets new target, and kicks off processing. */
>  void balloon_set_new_target(unsigned long target)
>  {
> 

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