Re: [RFC PATCH qemu-server] fix #7282: allow (NUMA aware) vCPU pinning

[Dominik Csapak <d.csapak@proxmox.com>]

On 9/10/26 4:37 PM, Elias Huhsovitz wrote:
> On Tue Feb 17, 2026 at 12:01 PM CET, Dominik Csapak wrote:
>> Introduce a new 'pinning' property, which (for now) has two methods for
>> vCPU pinning:
> 
> IMO very important patch for specific use cases.
> 
> * I noticed 1 potential cause for bugs when using one-to-one pinning (See
> choose_single_cpu)
> * numa setting might be working correctly. I tried different approach
> * but benchmark results are unchanged (stream benchmark [0] still benefits
> * greatly from the numam setting)
> 
> Also see other comments inline.
> 
> [...]
> 
>>
[snip]>> +my sub limit_by_affinity($host_cpus, $affinity_members) {
>> +    return $host_cpus if !$affinity_members || scalar($affinity_members->%*) == 0;
>> +
>> +    for my $node_id (keys $host_cpus->%*) {
>> +        my $node = $host_cpus->{$node_id};
>> +        for my $cpu_id (keys $node->%*) {
>> +            delete $node->{$cpu_id} if !defined($affinity_members->{$cpu_id});
>> +        }
>> +    }
>> +
>> +    return $host_cpus;
>> +}
>> +
>> +=head2 get_vnuma_vcpu_map
>> +
>> +Returns a hash from virtual NUMA nodes to vCPUs and an optional host NUMA node
>> +
>> +=cut
> 
> IMO the nesting in this subroutine is a bit overkill. I would:
> 1. invert the if statement, so we have an early return in the Non-NUMA
> case
> 2. Introduce my $node = $map->{$i} = { vcpus => {} };
> 3. Remove redundant parathesis, e.g.,
> for my $socket ((0 .. ($sockets - 1)))
> becomes
> for my $socket (0 .. ($sockets - 1))
> 4. Extract the deep for-loops into their own subroutines.

sure, makes sense

[snip]
>> +
>> +sub choose_single_cpu($vcpu_map, $host_cpus, $vcpu) {
>> +    my $hostnode = $vcpu_map->{$vcpu};
>> +    if (!defined($hostnode)) {
>> +        # choose a numa node at random
>> +        $hostnode = (keys $host_cpus->%*)[0];
>> +    }
>> +
>> +    # choose one at random
> 
> I think choosing a real CPU at random here might lead to random
> performance issues. If my understanding is correct, this could lead
> to SMT sibling collision.
> 
> In our case with the AMD EPYC 7351P 16-Core Processor:
> CPU NODE SOCKET CORE L1d:L1i:L2:L3
>   0    0      0    0 0:0:0:0
>   1    0      0    1 1:1:1:0
>   2    0      0    2 4:4:4:2
>   3    0      0    3 5:5:5:2
>   4    1      0    4 8:8:8:4
>   5    1      0    5 9:9:9:4
>   6    1      0    6 12:12:12:6
>   7    1      0    7 13:13:13:6
>   8    2      0    8 16:16:16:8
>   9    2      0    9 17:17:17:8
> 10    2      0   10 20:20:20:10
> 11    2      0   11 21:21:21:10
> 12    3      0   12 24:24:24:12
> 13    3      0   13 25:25:25:12
> 14    3      0   14 28:28:28:14
> 15    3      0   15 29:29:29:14
> 16    0      0    0 0:0:0:0
> 17    0      0    1 1:1:1:0
> 18    0      0    2 4:4:4:2
> 19    0      0    3 5:5:5:2
> 20    1      0    4 8:8:8:4
> 21    1      0    5 9:9:9:4
> 22    1      0    6 12:12:12:6
> 23    1      0    7 13:13:13:6
> 24    2      0    8 16:16:16:8
> 25    2      0    9 17:17:17:8
> 26    2      0   10 20:20:20:10
> 27    2      0   11 21:21:21:10
> 28    3      0   12 24:24:24:12
> 29    3      0   13 25:25:25:12
> 30    3      0   14 28:28:28:14
> 31    3      0   15 29:29:29:14
> 
> CPU 0 and 16 are siblings. They run on the same core, sharing the same L1 & L2 cache.
> If we use random selection process it might assign:
>      ┌─────────────────┐
>      │ vCPU 0 → CPU 0  │  ← Same physical core
>      │ vCPU 1 → CPU 16 │  ← SMT sibling
>      └─────────────────┘
> 
> So we have different programms vCPU0 and vCPU1 but the utilize the same hardware core.
> This would degrade performance as both of these CPUs share the same cache.
> 
> We should have some kind of safe guard to avoid SMT collisions.
> e.g., we could create a cpu selection policy:
> 1. Read core_id for every logical CPU in the node.
> 
> (Read it from /sys/devices/system/cpu/cpu{N}/topology/core_id since the hardware
> core id should be the most reliable)
> 
> 2. Build a map with: key=core-id, value=list of cpus that reside on that core
> In order to avoid any randomness, we should sort these entries, e.g.
> core-0 = [cpu0, cpu16]
> core-1 = [cpu1, cpu17]
> core-2 = [cpu2, cpu18]
> core-3 = [cpu3, cpu19]
> core-4 = [cpu4, cpu20]
> ...
> 
> 3. Return the cpus in a round-robin fashion. So calling choose_single_cpu would yield:
> 1. cpu0
> 2. cpu1
> 3. cpu2
> 4. cpu3
> 5. cpu16
> 6. cpu17 <-- here we could give a notice that we are overcomitting
> ...
> 
> Let me know what you think of this approach!

I think there is already a solution built in so that we don't have
to take these into account at all:

we already have the 'cpu affinity' config, and there users can e.g.
limit to only 'full' cores (instead of including hyperthreads)

so the user can just enter there '0,2,4,6,...' and so on
to exclude hyperthreads.

I don't want to do this automatically since this feature is
already complicated enough and overwriting some functionality
of another config we already have makes the interaction
even more complicated.

We could think of having e.g. a shortcut setting in the affinity
to only use real cores (or maybe even a GUI only checkbox that
writes the correct cores out) but i don't think
selecting the 'right' cores magically here is better...

If we really want this, I'd opt for an additional config option for that
to restrict to avoid selecting same cores.

Also what's missing is some kind of global rebalancer (e.g. like we
have for containers) but that can be done afterwards too IMO.

> 
>> +    my $real_cpu = (keys $host_cpus->{$hostnode}->%*)[0];
>> +    delete $host_cpus->{$hostnode}->{$real_cpu};
>> +    if (scalar($host_cpus->{$hostnode}->%*) == 0) {
>> +        delete $host_cpus->{$hostnode};
>> +    }
>> +
>> +    return $real_cpu;
>> +}
>> +
>> +=head2 get_numa_cpulist
>> +
>> +Returns the list of usable CPUs for the given C<$vcpu> index with the help of
>> +the vCPU to NUMA node map C<$vcpu_map> and the available C<$host_cpus> as a
>> +string usable by the 'taskset' command.
>> +
>> +=cut
>> +
>> +sub get_numa_cpulist($vcpu_map, $host_cpus, $vcpu) {
>> +    my $hostnode = $vcpu_map->{$vcpu};
>> +    if (!defined($hostnode)) {
>> +        # if there is not explicit mapping, simply don't pin at all
>> +        return undef;
>> +    }
>> +
>> +    return join(',', keys $host_cpus->{$hostnode}->%*);
>> +}
>> +
>> +=head2 assert_pinning_constraints
>> +
>> +Used to verify the constraints from pinnning by trying to construct
>> +the pinning configuration. Useful to check the config before actually starting
>> +the guest.
>> +
>> +=cut
>> +
>> +sub assert_pinning_constraints($conf) {
>> +
>> +    my $pinning = $conf->{pinning} // $DEFAULT_VCPU_PINNING;
>> +    if ($pinning ne $DEFAULT_VCPU_PINNING) {
>> +        my $host_cpus = get_filtered_host_cpus($conf);
>> +        get_vcpu_to_host_numa_map($conf, $host_cpus);
>> +    }
>> +}
>> +
>> +=head2 pin_threads_to_cpus
>> +
>> +Pins the vCPU threads of a running guest to host CPUs according to the
>> +pinning, affinity and NUMA configuraton.
>> +
>> +Needs the guest to be running, since it querys QMP for the vCPU thread list.
>> +
>> +=cut
>> +
>> +sub pin_threads_to_cpus($conf, $vmid) {
>> +    my $pinning = $conf->{pinning} // $DEFAULT_VCPU_PINNING;
>> +    if ($pinning ne $DEFAULT_VCPU_PINNING) {
>> +        my $host_cpus = get_filtered_host_cpus($conf);
>> +        my $vcpu_map = get_vcpu_to_host_numa_map($conf, $host_cpus);
>> +
>> +        my $cpuinfo = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-cpus-fast');
>> +        for my $vcpu ($cpuinfo->@*) {
>> +
>> +            my $vcpu_index = $vcpu->{'cpu-index'};
>> +
>> +            my $cpus;
>> +            if ($pinning eq 'one-to-one') {
>> +                $cpus = choose_single_cpu($vcpu_map, $host_cpus, $vcpu_index);
>> +            } elsif ($pinning eq 'numa') {
>> +                $cpus = get_numa_cpulist($vcpu_map, $host_cpus, $vcpu_index);
> 
> I am still not sure what causes the sysbench performance degradation
> when using the numa setting. Altough the stream benchmark seems to
> benefit greatly, so perhaps the config is working correctly? I will
> check if there are other benchmarks to measure the performance
> implications.
> 
>> +            }
>> +
>> +            die "no cpus selected for pinning vcpu $vcpu_index\n"
>> +                if !defined($cpus);
>> +
>> +            my $tid = $vcpu->{'thread-id'};
>> +            print "pinning vcpu $vcpu_index (thread $tid) to cpu(s) $cpus\n";
> 
> nit: The affinity settings uses the absolute path: /usr/bin/taskset
> IMO we either always use relative or absolute call for consistency

true, thanks for noticing

> 
>> +            run_command(
>> +                ['taskset', '-c', '-p', $cpus, $vcpu->{'thread-id'}], logfunc => sub { },
>> +            );
> 
> Calling taskset for every cpu might create some overhead.
> If we have 32 cpus, we are creating 32 threads that call taskset.
> 
> Instead we could create a big string containing all taskset commands
> and execute it in a single run_command call.
> 
> e.g.
> my @taskset_cmd;
> push @taskset_cmd, "taskset -c -p $cpus $tid";
> my $cmd_str = join(' && ', @taskset_cmds);
> run_command($cmd_str, logfunc => sub { print "$_[0]\n" });
> 

yep makes sense

>> +        }
>> +    }
>> +}
>> +
>> +1;
> 
> [0] https://www.cs.virginia.edu/stream/ref.html