Re: [Xen-devel] [PATCH RESEND v5 04/24] x86: refactor psr: implement CPU init and free flow.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.01.17 at 03:44, <konrad.wilk@xxxxxxxxxx> wrote:
> On Thu, Jan 19, 2017 at 02:01:06PM +0800, Yi Sun wrote:
>> @@ -127,6 +130,13 @@ struct feat_node {
>>      struct list_head list;
>>  };
>>  
>> +struct cpuid_leaf_regs {
>> +    unsigned int eax;
>> +    unsigned int ebx;
>> +    unsigned int ecx;
>> +    unsigned int edx;
>> +};
> 
> Could you use 'struct cpuid_leaf' in x86_emulate.h ?
> 
> It would only require "#include <asm/x86_emulate.h>" I believe.

Indeed - I recall specifically having asked for that.

>>  static int psr_cpu_prepare(unsigned int cpu)
>>  {
>> +    if ( !socket_info )
>> +        return 0;
>> +
>> +    /* Malloc memory for the global feature head here. */
>> +    if ( feat_l3_cat == NULL &&
>> +         (feat_l3_cat = xzalloc(struct feat_node)) == NULL )
> 
> /me scratches his head.
> 
> Lets say we have a two socket machine. Each socket has
> two CPUs (so total of 4 CPUs).
> 
> On CPU0  (in psr_presmp_init) it allocates feat_l3_cat. Then
> later in (in psr_presmp_init) you call psr_cpu_init which
> sets feat_l3_cat to NULL (and 'feat' is feat_l3_cat).
> 
> When CPU1 starts up then, psr_cpu_prepare is called
> which means we allocate a new feat_l3_cat. 'cpu_init_work'
> short-circuits (as info->feat_mask has a value) and does not
> do anything.
> 
> When CPU2 (socket two), starts up then feat_l3_cat (as 'feat')
> is used (in psr_cpu_init and again sets feat_l3_cat to NULL).
> 
> The last CPU3 (socket #2) starts up, and since feat_l3_cat is
> NULL - it is allocated. But in 'cpu_init_work' it short-circuits
> so we don't use third allocated 'feat_l3_cat' .
> 
> And we have an 'feat_l3_cat' that is not used for anything..
> 
> In other words - a memory leak. Granted a very small one.

I did recommend to do it that way, to simplify things a little.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel