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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

Please clarify.

Jan

>>          p2m_initialise(d, p2m);
>>          p2m->write_p2m_entry = nestedp2m_write_p2m_entry;
>>          list_add(&p2m->np2m_list, &p2m_get_hostp2m(d)->np2m_list);
>> @@ -117,6 +118,8 @@ int p2m_init(struct domain *d)
>>      p2m_get_hostp2m(d) = p2m = xzalloc(struct p2m_domain);
>>      if ( p2m == NULL )
>>          return -ENOMEM;
>> +    if ( !zalloc_cpumask_var(&p2m->dirty_cpumask) )
>> +        return -ENOMEM;
> 
> Likewise.
> 
> Apart from that, 
> Acked-by: Tim Deegan <tim@xxxxxxx>




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