Re: [Xen-devel] [PATCH] xentrace: handle sparse cpu ids correctly in xen trace buffer handling

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 08/30/2018 10:28 AM, Juergen Gross wrote:
> On 30/08/18 10:26, Jan Beulich wrote:
>>>>> On 30.08.18 at 09:52, <jgross@xxxxxxxx> wrote:
>>> @@ -202,7 +202,7 @@ static int alloc_trace_bufs(unsigned int pages)
>>>       * Allocate buffers for all of the cpus.
>>>       * If any fails, deallocate what you have so far and exit.
>>>       */
>>> -    for_each_online_cpu(cpu)
>>> +    for_each_present_cpu(cpu)
>>>      {
>>>          offset = t_info_first_offset + (cpu * pages);
>>>          t_info->mfn_offset[cpu] = offset;
>>
>> Doesn't this go a little too far? Why would you allocate buffers for CPUs
>> which can never be brought online? There ought to be a middle ground,
>> where online-able CPUs have buffers allocated, but non-online-able ones
>> won't. On larger systems I guess the difference may be quite noticable.
> 
> According to the comments in include/xen/cpumask.h cpu_present_map
> represents the populated cpus.
> 
> I know that currently there is no support for onlining a parked cpu
> again, but I think having to think about Xentrace buffer allocation in
> case onlining of parked cpus is added would be a nearly 100% chance to
> introduce a bug.
> 
> Xentrace is used for testing purposes only. So IMHO allocating some more
> memory is acceptable.

On the other hand, size of buffers can be a significant factor in
whether you can manage to catch the behavior you want or whether there
will be gaps due to dropped records; anything that can make those
allocations go farther will be helpful.

Anyone bringing a parked cpu back up should have to go through all
instances of per_cpu() and figure out if they need to do something; that
should catch this issue, if it happens.

So although it's a bit of a tough decision, I'd leave this
for_each_online_cpu(), as Jan suggests.

Thanks,
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel