Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx> writes:
> On 01/09/2013 07:31 AM, Rusty Russell wrote:
>> Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx> writes:
>>> */
>>> static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
>>> {
>>> - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
>>> - smp_processor_id();
>>> + int txq = 0;
>>> +
>>> + if (skb_rx_queue_recorded(skb))
>>> + txq = skb_get_rx_queue(skb);
>>> + else if ((txq = per_cpu(vq_index, smp_processor_id())) == -1)
>>> + txq = 0;
>>
>> You should use __get_cpu_var() instead of smp_processor_id() here, ie:
>>
>> else if ((txq = __get_cpu_var(vq_index)) == -1)
>>
>> And AFAICT, no reason to initialize txq to 0 to start with.
>>
>> So:
>>
>> int txq;
>>
>> if (skb_rx_queue_recorded(skb))
>> txq = skb_get_rx_queue(skb);
>> else {
>> txq = __get_cpu_var(vq_index);
>> if (txq == -1)
>> txq = 0;
>> }
>
> Got it, thank you.
>
>>
>> Now, just to confirm, I assume this can happen even if we use vq_index,
>> right, because of races with virtnet_set_channels?
>
> I still can't understand this race, could you explain more? thank you.
I assume that someone can call virtnet_set_channels() while we are
inside virtnet_select_queue(), so they reduce dev->real_num_tx_queues,
causing virtnet_set_channels to do:
while (unlikely(txq >= dev->real_num_tx_queues))
txq -= dev->real_num_tx_queues;
Otherwise, when is this loop called?
Thanks,
Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
