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

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


 


Rackspace

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