Re: [Qemu-devel] [PATCH] x86: Increase max vcpu number to 352

[Lan Tianyu]

On 2017年08月11日 03:22, Radim Krčmář wrote:
> 2017-08-10 15:16-0300, Eduardo Habkost:
>> On Thu, Aug 10, 2017 at 02:41:03PM +0200, Radim Krčmář wrote:
>>> 2017-08-10 19:02+0800, Lan Tianyu:
>>>> On 2017年08月10日 18:26, Daniel P. Berrange wrote:
>>>>> On Thu, Aug 10, 2017 at 06:08:07PM +0800, Lan Tianyu wrote:
>>>>>> Intel Xeon phi chip will support 352 logical threads. For HPC
>>>>>> usage case, it will create a huge VM with vcpus number as same as host
>>>>>> cpus. This patch is to increase max vcpu number to 352.
>>>>>
>>>>> If we pick arbitray limits based on size of physical CPUs that happen
>>>>> to be shipping today, we'll continue the cat+mouse game forever trailing
>>>>> latest CPUs that vendors ship.
>>>>>
>>>>> IMHO we should pick a higher number influenced by technical constraints
>>>>> of the q35 impl instead. eg can we go straight to something like 512 or
>>>>> 1024  ?
>>>>
>>>> Sure. 512 should be enough and some arrays is defined according to max
>>>> vcpu number.
>>>
>>> Hm, which arrays are that?  I was thinking it is safe to bump it to
>>> INT_MAX as the number is only used when setting global max_cpus.
>>
>> We had a MAX_CPUMASK_BITS macro, and bitmaps whose sizes were
>> defined at compile time based on it.  But commit
>> cdda2018e3b9ce0c18938767dfdb1e05a05b67ca removed it.  Probably
>> those arrays all use max_cpus, by now (and the default for
>> max_cpus is smp_cpus, not MachineClass::max_cpus).
> 
> Ah, thanks.
> 
>> Anyway, if we set it to INT_MAX, there are some cases where more
>> appropriate error checking/reporting could be required because
>> they won't handle overflow very well:
>> * pcms->apic_id_limit initialization at pc_cpus_init()
>> * ACPI code that assumes possible_cpus->cpus[i].arch_id fits
>>   in a 32-bit integer
>> * Other x86_cpu_apic_id_from_index() calls in PC code
>>   (especially the initialization of possible_cpus->cpus[i].arch_id).
>>   Note that x86_cpu_apic_id_from_index(cpu_index) might not fit
>>   in 32 bits even if cpu_index <= UINT32_MAX.
> 
> Good point, looks like it all comes to x86_cpu_apic_id_from_index().
> Each level of the topology has at most one underutilized bit, so
> 2^(32 - 3) would be safe.
> 
> It is still needlessly large for the foreseeable future, but 512 is
> going to be surpassed pretty soon, so I think that jumping at least to
> 8k would be better.
> (8k the current default maximum for Linux and the resulting overcommit
>  of ~20 is bearable for smoke testing on current hardware.)
> 

Hi All:
        Thanks for your input. I tried Qemu with 8192 as max_vcpu and it works
normally. I will update patches.

-- 
Best regards
Tianyu Lan