Re: [Xen-ia64-devel] [PATCH] fix memory leak when domVTI is created

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

Hi Tristan. Thank you for your comment and sorry for delay.
Here is the updated patch.

On Mon, Jul 31, 2006 at 08:27:43AM +0200, Tristan Gingold wrote:
> Le Vendredi 28 Juillet 2006 12:30, Isaku Yamahata a écrit :
> > Please discard the previous one. Use the attached one.
> > Sorry for confusion.
> >
> > On Fri, Jul 28, 2006 at 12:50:08PM +0900, Isaku Yamahata wrote:
> > > fix memory leak when VT-i domain is created.
> > > When domain is created, xend sets max vcpu before domain setup.
> > > So alloc_vcpu_struct() thinks that the domain is normal domU, not domVTI.
> > > And next xend set the domain as domVTI. so the memory is allocated for
> > > domU won't be freed.
> > >
> > > --
> > > yamahata
> Good catch, but I have a few comments.
> 
> diff -r 2d73714911c2 -r a8eddc3f1f82 xen/arch/ia64/xen/domain.c
> --- a/xen/arch/ia64/xen/domain.c        Thu Jul 27 10:43:34 2006 -0600
> +++ b/xen/arch/ia64/xen/domain.c        Fri Jul 28 19:28:44 2006 +0900
> @@ -261,13 +261,19 @@ struct vcpu *alloc_vcpu_struct(struct do
>  
>         if (!is_idle_domain(d)) {
>             if (!d->arch.is_vti) {
> -               /* Create privregs page only if not VTi.  */
> -               v->arch.privregs = 
> -                   alloc_xenheap_pages(get_order(sizeof(mapped_regs_t)));
> -               BUG_ON(v->arch.privregs == NULL);
> -               memset(v->arch.privregs, 0, PAGE_SIZE);
> -               share_xen_page_with_guest(virt_to_page(v->arch.privregs),
> -                                         d, XENSHARE_writable);
> +            int order;
> +            int i;
> +
> +            /* Create privregs page only if not VTi.  */
> +            order = get_order_from_shift(XMAPPEDREGS_SHIFT);
> Note: order could be a compile-time constant.
> +            BUG_ON(get_order(sizeof(mapped_regs_t)) !=
> +                   get_order_from_shift(XMAPPEDREGS_SHIFT));
> As a consequence this should be an #error by adding a new entry in 
> asm-offset.c
> Earlier is better than latter!
> Because sizeof(mapped_regs_t) is 4096, this couldn't be wrong now.
> 
> +            v->arch.privregs = alloc_xenheap_pages(order);
> +            BUG_ON(v->arch.privregs == NULL);
> +            memset(v->arch.privregs, 0, (1 << order) * PAGE_SIZE);
> I'd prefer 1 << XMAPPEDREGS_SHIFT (more readable).
> +            for (i = 0; i < (1 << order); i++)
> +                share_xen_page_with_guest(virt_to_page(v->arch.privregs) + i,
> +                                          d, XENSHARE_writable);
>             }
> 
> Tristan.
> 
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel

-- 
yamahata

Attachment: 10923:30f69cd84f48_dom_vti_memory_leak.patch
Description: Text document

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