WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel][PATCH] don't panic dom0 if there is not enoughmemor

To: "Zhang, Xing Z" <xing.z.zhang@xxxxxxxxx>
Subject: Re: [Xen-ia64-devel][PATCH] don't panic dom0 if there is not enoughmemory to create guest
From: Aron Griffis <aron@xxxxxx>
Date: Wed, 24 Jan 2007 01:07:24 -0500
Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 23 Jan 2007 22:07:04 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <58A36151585E4047913F40517D307BAE31AB5F@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-ia64-devel-request@lists.xensource.com?subject=help>
List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
List-post: <mailto:xen-ia64-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=unsubscribe>
Mail-followup-to: "Zhang, Xing Z" <xing.z.zhang@xxxxxxxxx>, xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
References: <58A36151585E4047913F40517D307BAE31AB5E@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <58A36151585E4047913F40517D307BAE31AB5F@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.13 (2006-08-11)
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