Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue

["Nambiar, Amritha" <amritha.nambiar@intel.com>]

On 8/1/2018 5:11 PM, Andrei Vagin wrote:
> On Tue, Jul 10, 2018 at 07:28:49PM -0700, Nambiar, Amritha wrote:
>> With this patch series, I introduced static_key for XPS maps
>> (xps_needed), so static_key_slow_inc() is used to switch branches. The
>> definition of static_key_slow_inc() has cpus_read_lock in place. In the
>> virtio_net driver, XPS queues are initialized after setting the
>> queue:cpu affinity in virtnet_set_affinity() which is already protected
>> within cpus_read_lock. Hence, the warning here trying to acquire
>> cpus_read_lock when it is already held.
>>
>> A quick fix for this would be to just extract netif_set_xps_queue() out
>> of the lock by simply wrapping it with another put/get_online_cpus
>> (unlock right before and hold lock right after).
> 
> virtnet_set_affinity() is called from virtnet_cpu_online(), which is
> called under cpus_read_lock too.
> 
> It looks like now we can't call netif_set_xps_queue() from cpu hotplug
> callbacks.
> 
> I can suggest a very straightforward fix for this problem. The patch is
> attached.
> 

Thanks for looking into this. I was thinking of fixing this in the
virtio_net driver by moving the XPS initialization (and have a new
get_affinity utility) in the ndo_open (so it is together with other tx
preparation) instead of probe. Your patch solves this in general for
setting up cpu hotplug callbacks which is under cpus_read_lock.

>> But this may not a
>> clean solution. It'd help if I can get suggestions on what would be a
>> clean option to fix this without extensively changing the code in
>> virtio_net. Is it mandatory to protect the affinitization with
>> read_lock? I don't see similar lock in other drivers while setting the
>> affinity. I understand this warning should go away, but isn't it safe to
>> have multiple readers.
>>
>>> On Fri, Jun 29, 2018 at 09:27:07PM -0700, Amritha Nambiar wrote: