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-devel

Re: [Xen-devel] [PATCH 08/12] x86/p2m: allocate CPU masks dynamically

At 15:22 +0100 on 20 Oct (1319124131), Tim Deegan wrote:
> At 15:18 +0100 on 20 Oct (1319123893), Tim Deegan wrote:
> > At 15:07 +0100 on 20 Oct (1319123275), Jan Beulich wrote:
> > > >>> On 20.10.11 at 16:00, Tim Deegan <tim@xxxxxxx> wrote:
> > > > At 14:41 +0100 on 20 Oct (1319121707), Jan Beulich wrote:
> > > >> --- 2011-10-18.orig/xen/arch/x86/mm/p2m.c      2011-10-14 
> > > >> 09:47:46.000000000 +0200
> > > >> +++ 2011-10-18/xen/arch/x86/mm/p2m.c   2011-10-18 16:45:49.000000000 
> > > >> +0200
> > > >> @@ -81,7 +81,6 @@ static void p2m_initialise(struct domain
> > > >>      p2m->default_access = p2m_access_rwx;
> > > >>  
> > > >>      p2m->cr3 = CR3_EADDR;
> > > >> -    cpumask_clear(&p2m->p2m_dirty_cpumask);
> > > >>  
> > > >>      if ( hap_enabled(d) && (boot_cpu_data.x86_vendor == 
> > > >> X86_VENDOR_INTEL) )
> > > >>          ept_p2m_init(p2m);
> > > >> @@ -102,6 +101,8 @@ p2m_init_nestedp2m(struct domain *d)
> > > >>          d->arch.nested_p2m[i] = p2m = xzalloc(struct p2m_domain);
> > > >>          if (p2m == NULL)
> > > >>              return -ENOMEM;
> > > >> +        if ( !zalloc_cpumask_var(&p2m->dirty_cpumask) )
> > > >> +            return -ENOMEM;
> > > > 
> > > > This leaks 'p2m'.
> > > 
> > > If that's really true, then there is a leak already without that patch:
> > > p2m_init() calls p2m_init_nestedp2m() without recovering from failure
> > > in that function. It was my understanding that since failure here
> > > ultimately leads to failure of domain construction, which I thought
> > > (hoped - didn't verify) would result in p2m_final_teardown() getting
> > > called.
> > 
> > You're quite right; it will all get tidied up by p2m_final_teardown().
> 
> Except that arch_domain_create() doesn't actually call
> paging_final_teardown() if paging_domain_init() fails, so that path
> probably leaks quite badly -- at least the nested-paging stuff you
> pointed out, and possibly other things.  I'll have a look at it. 

OK, it looks like it was just the nested-p2m state.  That should be
fixed now, as staging/xen-unstable.hg 23981:6c583d35d76d

Cheers,

Tim

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