[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-ia64-devel][PATCH] don't panic dom0 if there is not enoughmemory to create guest



Hello Zhang,

I understand this is already applied, but I have some minor comments
for the future.

Aron

Zhang, Xing Z wrote:  [Thu Jan 18 2007, 09:05:21PM EST]
Content-Description: donnot_panic_dom0_2.patch
> When there is not enough memory to create a domain,
> we not need panic domain0. Just prevent it from crating.
> 
> Signed-off-by, Zhang Xin < xing.z.zhang@xxxxxxxxx >
> diff -r 58637a0a7c7e xen/arch/ia64/vmx/vmmu.c
> --- a/xen/arch/ia64/vmx/vmmu.c        Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/arch/ia64/vmx/vmmu.c        Fri Jan 19 01:38:42 2007 +0800
> @@ -129,13 +129,16 @@ purge_machine_tc_by_domid(domid_t domid)
>  #endif
>  }
>  
> -static void init_domain_vhpt(struct vcpu *v)
> +static int init_domain_vhpt(struct vcpu *v)
>  {
>      struct page_info *page;
>      void * vbase;
>      page = alloc_domheap_pages (NULL, VCPU_VHPT_ORDER, 0);
>      if ( page == NULL ) {
> -        panic_domain(vcpu_regs(v),"No enough contiguous memory for 
> init_domain_vhpt\n");
> +        //panic_domain(vcpu_regs(v),"No enough contiguous memory for 
> init_domain_vhpt\n");
> +        printk("No enough contiguous memory for init_domain_vhpt\n");

There's no need to comment out the panic_domain line.  That's what
source control is for! :-)  Better to just remove it.

> +
> +        return -1;
>      }
>      vbase = page_to_virt(page);
>      memset(vbase, 0, VCPU_VHPT_SIZE);
> @@ -147,18 +150,38 @@ static void init_domain_vhpt(struct vcpu
>      VHPT(v,cch_sz) = VCPU_VHPT_SIZE - VHPT(v,hash_sz);
>      thash_init(&(v->arch.vhpt),VCPU_VHPT_SHIFT-1);
>      v->arch.arch_vmx.mpta = v->arch.vhpt.pta.val;
> -}
> -
> -
> -
> -void init_domain_tlb(struct vcpu *v)
> +
> +    return 0;
> +}
> +
> +
> +static void free_domain_vhpt(struct vcpu *v)
> +{
> +    struct page_info *page;
> +
> +    if ( v->arch.vhpt.hash) {
> +        page = virt_to_page(v->arch.vhpt.hash);
> +        free_domheap_pages(page, VCPU_VHPT_ORDER);
> +    }
> +
> +    return;
> +}
> +
> +int init_domain_tlb(struct vcpu *v)
>  {
>      struct page_info *page;
>      void * vbase;
> -    init_domain_vhpt(v);
> +
> +    if ( init_domain_vhpt(v) != 0 )
> +        goto err;
> +
>      page = alloc_domheap_pages (NULL, VCPU_VTLB_ORDER, 0);
>      if ( page == NULL ) {
> -        panic_domain(vcpu_regs(v),"No enough contiguous memory for 
> init_domain_tlb\n");
> +        //panic_domain("No enough contiguous memory for init_domain_tlb\n");
> +        printk("No enough contiguous memory for init_domain_tlb\n");

Same here.

> +        free_domain_vhpt(v);
> +
> +        goto err;
>      }
>      vbase = page_to_virt(page);
>      memset(vbase, 0, VCPU_VTLB_SIZE);
> @@ -169,7 +192,12 @@ void init_domain_tlb(struct vcpu *v)
>      VTLB(v,cch_buf) = (void *)((u64)vbase + VTLB(v,hash_sz));
>      VTLB(v,cch_sz) = VCPU_VTLB_SIZE - VTLB(v,hash_sz);
>      thash_init(&(v->arch.vtlb),VCPU_VTLB_SHIFT-1);
> -}
> +    
> +    return 0;
> +err:
> +    return -1;
> +}

If there's nothing to clean up, IMHO there is no need for goto err.
Instead you could just return the error code directly.  

Is there a better error code to use in these cases than -1, for
example -ENOMEM?

> +
>  
>  void free_domain_tlb(struct vcpu *v)
>  {
> @@ -179,10 +207,8 @@ void free_domain_tlb(struct vcpu *v)
>          page = virt_to_page(v->arch.vtlb.hash);
>          free_domheap_pages(page, VCPU_VTLB_ORDER);
>      }
> -    if ( v->arch.vhpt.hash) {
> -        page = virt_to_page(v->arch.vhpt.hash);
> -        free_domheap_pages(page, VCPU_VHPT_ORDER);
> -    }
> +
> +    free_domain_vhpt(v);
>  }
>  
>  /*
> diff -r 58637a0a7c7e xen/arch/ia64/vmx/vmx_init.c
> --- a/xen/arch/ia64/vmx/vmx_init.c    Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/arch/ia64/vmx/vmx_init.c    Thu Jan 18 09:56:06 2007 +0800
> @@ -290,7 +290,7 @@ static void vmx_release_assist_channel(s
>   * Initialize VMX envirenment for guest. Only the 1st vp/vcpu
>   * is registered here.
>   */
> -void
> +int
>  vmx_final_setup_guest(struct vcpu *v)
>  {
>       vpd_t *vpd;
> @@ -305,7 +305,8 @@ vmx_final_setup_guest(struct vcpu *v)
>        * to this solution. Maybe it can be deferred until we know created
>        * one as vmx domain */
>  #ifndef HASH_VHPT
> -     init_domain_tlb(v);
> +     if ( init_domain_tlb(v) != 0 )
> +        goto err;
>  #endif
>       vmx_create_event_channels(v);
>  
> @@ -322,6 +323,10 @@ vmx_final_setup_guest(struct vcpu *v)
>  
>       /* Set up guest 's indicator for VTi domain*/
>       set_bit(ARCH_VMX_DOMAIN, &v->arch.arch_vmx.flags);
> +
> +    return 0;
> +err:
> +    return -1;
>  }

Same here.

>  void
> diff -r 58637a0a7c7e xen/arch/ia64/xen/domain.c
> --- a/xen/arch/ia64/xen/domain.c      Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/arch/ia64/xen/domain.c      Thu Jan 18 09:56:06 2007 +0800
> @@ -585,8 +585,11 @@ int arch_set_info_guest(struct vcpu *v, 
>       if (test_bit(_VCPUF_initialised, &v->vcpu_flags))
>               return 0;
>  
> -     if (d->arch.is_vti)
> -             vmx_final_setup_guest(v);
> +     if (d->arch.is_vti){
> +             rc = vmx_final_setup_guest(v);
> +        if ( rc != 0 )
> +            return rc;
> +    }
>       else {
>               rc = vcpu_late_initialise(v);
>               if (rc != 0)
> diff -r 58637a0a7c7e xen/include/asm-ia64/vmmu.h
> --- a/xen/include/asm-ia64/vmmu.h     Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/include/asm-ia64/vmmu.h     Thu Jan 18 09:56:06 2007 +0800
> @@ -295,7 +295,7 @@ extern void purge_machine_tc_by_domid(do
>  extern void purge_machine_tc_by_domid(domid_t domid);
>  extern void machine_tlb_insert(struct vcpu *d, thash_data_t *tlb);
>  extern ia64_rr vmmu_get_rr(struct vcpu *vcpu, u64 va);
> -extern void init_domain_tlb(struct vcpu *d);
> +extern int init_domain_tlb(struct vcpu *d);
>  extern void free_domain_tlb(struct vcpu *v);
>  extern thash_data_t * vsa_thash(PTA vpta, u64 va, u64 vrr, u64 *tag);
>  extern thash_data_t * vhpt_lookup(u64 va);
> diff -r 58637a0a7c7e xen/include/asm-ia64/vmx.h
> --- a/xen/include/asm-ia64/vmx.h      Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/include/asm-ia64/vmx.h      Thu Jan 18 09:56:06 2007 +0800
> @@ -31,7 +31,7 @@ extern void identify_vmx_feature(void);
>  extern void identify_vmx_feature(void);
>  extern unsigned int vmx_enabled;
>  extern void vmx_init_env(void);
> -extern void vmx_final_setup_guest(struct vcpu *v);
> +extern int vmx_final_setup_guest(struct vcpu *v);
>  extern void vmx_save_state(struct vcpu *v);
>  extern void vmx_load_state(struct vcpu *v);
>  extern void vmx_setup_platform(struct domain *d);

> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.