netdev - Re: [PATCH net-next] virtio: Fix affinity for >32 VCPUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 5 Feb 2017 23:28:09 -0800
From:   Benjamin Serebrin <serebrin@...gle.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     netdev@...r.kernel.org, "Michael S. Tsirkin" <mst@...hat.com>,
        David Miller <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>,
        Venkatesh Srinivas <venkateshs@...gle.com>,
        James Mattson <jmattson@...gle.com>
Subject: Re: [PATCH net-next] virtio: Fix affinity for >32 VCPUs

On Sun, Feb 5, 2017 at 11:24 PM, Jason Wang <jasowang@...hat.com> wrote:
>
>
> On 2017年02月03日 14:19, Ben Serebrin wrote:
>>
>> From: Benjamin Serebrin <serebrin@...gle.com>
>>
>> If the number of virtio queue pairs is not equal to the
>> number of VCPUs, the virtio guest driver doesn't assign
>> any CPU affinity for the queue interrupts or the xps
>> aggregation interrupt.
>
>
> So this in fact is not a affinity fixing for #cpus > 32 but adding  affinity
> for #cpus != #queue pairs.

Fair enough.  I'll adjust the title line in the subsequent version.


>
>> Google Compute Engine currently provides 1 queue pair for
>> every VCPU, but limits that at a maximum of 32 queue pairs.
>>
>> This code assigns interrupt affinity even when there are more than
>> 32 VCPUs.
>>
>> Tested:
>>
>> (on a 64-VCPU VM with debian 8, jessie-backports 4.9.2)
>>
>> Without the fix we see all queues affinitized to all CPUs:
>
>
> [...]
>
>>   +     /* If there are more cpus than queues, then assign the queues'
>> +        * interrupts to the first cpus until we run out.
>> +        */
>>         i = 0;
>>         for_each_online_cpu(cpu) {
>> +               if (i == vi->max_queue_pairs)
>> +                       break;
>>                 virtqueue_set_affinity(vi->rq[i].vq, cpu);
>>                 virtqueue_set_affinity(vi->sq[i].vq, cpu);
>> -               netif_set_xps_queue(vi->dev, cpumask_of(cpu), i);
>>                 i++;
>>         }
>>   +     /* Stripe the XPS affinities across the online CPUs.
>> +        * Hyperthread pairs are typically assigned such that Linux's
>> +        * CPU X and X + (numcpus / 2) are hyperthread twins, so we cause
>> +        * hyperthread twins to share TX queues, in the case where there
>> are
>> +        * more cpus than queues.
>
>
> Since we use combined queue pairs, why not use the same policy for RX?

XPS is for transmit only.


> Thanks
>
>
>> +        */
>> +       for (i = 0; i < vi->max_queue_pairs; i++) {
>> +               struct cpumask mask;
>> +               int skip = i;
>> +
>> +               cpumask_clear(&mask);
>> +               for_each_online_cpu(cpu) {
>> +                       while (skip--)
>> +                               cpu = cpumask_next(cpu, cpu_online_mask);
>> +                       if (cpu < num_possible_cpus())
>> +                               cpumask_set_cpu(cpu, &mask);
>> +                       skip = vi->max_queue_pairs - 1;
>> +               }
>> +               netif_set_xps_queue(vi->dev, &mask, i);
>> +       }
>> +
>>         vi->affinity_hint_set = true;
>>   }
>>
>
>