search for: netif_set_xps_queu

...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. Lockdep prints a warning when we are trying to acquire cpus_read_lock when it is already held. This patch adds an ability to call __netif_set_xps_queue under cpu_read_lock(). ============================================ WARNING: possible recursive locking detected 4.18.0-rc3-next-20180703+ #1 Not tainted -------------------------------------------- swapper/0/1 is trying to acquire lock: 00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: stati...

...virtnet_set_affinity(struct virtnet_info *vi) @@ -1104,7 +1090,7 @@ static void virtnet_set_affinity(struct virtnet_info *vi) for_each_online_cpu(cpu) { virtqueue_set_affinity(vi->rq[i].vq, cpu); virtqueue_set_affinity(vi->sq[i].vq, cpu); - *per_cpu_ptr(vi->vq_index, cpu) = i; + netif_set_xps_queue(vi->dev, cpumask_of(cpu), i); i++; } @@ -1217,28 +1203,6 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu) return 0; } -/* To avoid contending a lock hold by a vcpu who would exit to host, select the - * txq based on the processor id. - */ -static u16 virtnet_sel...

...virtnet_set_affinity(struct virtnet_info *vi) @@ -1104,7 +1090,7 @@ static void virtnet_set_affinity(struct virtnet_info *vi) for_each_online_cpu(cpu) { virtqueue_set_affinity(vi->rq[i].vq, cpu); virtqueue_set_affinity(vi->sq[i].vq, cpu); - *per_cpu_ptr(vi->vq_index, cpu) = i; + netif_set_xps_queue(vi->dev, cpumask_of(cpu), i); i++; } @@ -1217,28 +1203,6 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu) return 0; } -/* To avoid contending a lock hold by a vcpu who would exit to host, select the - * txq based on the processor id. - */ -static u16 virtnet_sel...

...at redhat.com> Acked-by: Rusty Russell <rusty at rustcorp.com.au> Acked-by: Michael S. Tsirkin <mst at redhat.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- Resend since the previous complie warning disappears after commit 3573540cafa4296dd60f8be02f2aecaa31047525 (netif_set_xps_queue: make cpu mask const). --- drivers/net/virtio_net.c | 48 ++---------------------------------------------- 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index a7e9ad9..01f4eb5 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/...

...at redhat.com> Acked-by: Rusty Russell <rusty at rustcorp.com.au> Acked-by: Michael S. Tsirkin <mst at redhat.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- Resend since the previous complie warning disappears after commit 3573540cafa4296dd60f8be02f2aecaa31047525 (netif_set_xps_queue: make cpu mask const). --- drivers/net/virtio_net.c | 48 ++---------------------------------------------- 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index a7e9ad9..01f4eb5 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/...

...ng 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. > > >...

...lloc_cpumask_var(&cpumask, GFP_KERNEL)) + return; + i = 0; for_each_online_cpu(cpu) { virtqueue_set_affinity(vi->rq[i].vq, cpu); virtqueue_set_affinity(vi->sq[i].vq, cpu); - *per_cpu_ptr(vi->vq_index, cpu) = i; + cpumask_clear(cpumask); + cpumask_set_cpu(cpu, cpumask); + netif_set_xps_queue(vi->dev, cpumask, i); i++; } vi->affinity_hint_set = true; + free_cpumask_var(cpumask); } static int virtnet_cpu_callback(struct notifier_block *nfb, @@ -1217,28 +1210,6 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu) return 0; } -/* To avoid contendin...

...lloc_cpumask_var(&cpumask, GFP_KERNEL)) + return; + i = 0; for_each_online_cpu(cpu) { virtqueue_set_affinity(vi->rq[i].vq, cpu); virtqueue_set_affinity(vi->sq[i].vq, cpu); - *per_cpu_ptr(vi->vq_index, cpu) = i; + cpumask_clear(cpumask); + cpumask_set_cpu(cpu, cpumask); + netif_set_xps_queue(vi->dev, cpumask, i); i++; } vi->affinity_hint_set = true; + free_cpumask_var(cpumask); } static int virtnet_cpu_callback(struct notifier_block *nfb, @@ -1217,28 +1210,6 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu) return 0; } -/* To avoid contendin...

...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 > > > &gt...

...return; > + > i = 0; > for_each_online_cpu(cpu) { > virtqueue_set_affinity(vi->rq[i].vq, cpu); > virtqueue_set_affinity(vi->sq[i].vq, cpu); > - *per_cpu_ptr(vi->vq_index, cpu) = i; > + cpumask_clear(cpumask); > + cpumask_set_cpu(cpu, cpumask); > + netif_set_xps_queue(vi->dev, cpumask, i); > i++; > } > > vi->affinity_hint_set = true; > + free_cpumask_var(cpumask); > } Um, isn't this just cpumask_of(cpu)? Cheers, Rusty.

...ed-off-by: Jason Wang <jasowang at redhat.com> > --- > Changes from V1: > - use cpumask_of() instead of allocate dynamically This generates build warnings: drivers/net/virtio_net.c: In function ?virtnet_set_affinity?: drivers/net/virtio_net.c:1093:3: warning: passing argument 2 of ?netif_set_xps_queue? discards ?const? qualifier from pointer target type [enabled by default] In file included from drivers/net/virtio_net.c:20:0: include/linux/netdevice.h:2275:5: note: expected ?struct cpumask *? but argument is of type ?const struct cpumask *?

...irtnet_info *vi) > @@ -1104,7 +1090,7 @@ static void virtnet_set_affinity(struct virtnet_info *vi) > for_each_online_cpu(cpu) { > virtqueue_set_affinity(vi->rq[i].vq, cpu); > virtqueue_set_affinity(vi->sq[i].vq, cpu); > - *per_cpu_ptr(vi->vq_index, cpu) = i; > + netif_set_xps_queue(vi->dev, cpumask_of(cpu), i); > i++; > } > > @@ -1217,28 +1203,6 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu) > return 0; > } > > -/* To avoid contending a lock hold by a vcpu who would exit to host, select the > - * txq based on t...

...return; > + > i = 0; > for_each_online_cpu(cpu) { > virtqueue_set_affinity(vi->rq[i].vq, cpu); > virtqueue_set_affinity(vi->sq[i].vq, cpu); > - *per_cpu_ptr(vi->vq_index, cpu) = i; > + cpumask_clear(cpumask); > + cpumask_set_cpu(cpu, cpumask); > + netif_set_xps_queue(vi->dev, cpumask, i); > i++; > } > > vi->affinity_hint_set = true; > + free_cpumask_var(cpumask); > } > > static int virtnet_cpu_callback(struct notifier_block *nfb, > @@ -1217,28 +1210,6 @@ static int virtnet_change_mtu(struct net_device *dev, int new...

...net_info *vi) > @@ -1165,7 +1151,7 @@ static void virtnet_set_affinity(struct virtnet_info *vi) > for_each_online_cpu(cpu) { > virtqueue_set_affinity(vi->rq[i].vq, cpu); > virtqueue_set_affinity(vi->sq[i].vq, cpu); > - *per_cpu_ptr(vi->vq_index, cpu) = i; > + netif_set_xps_queue(vi->dev, cpumask_of(cpu), i); > i++; > } > > @@ -1268,29 +1254,6 @@ static const struct ethtool_ops virtnet_ethtool_ops = { > .get_channels = virtnet_get_channels, > }; > > -/* To avoid contending a lock hold by a vcpu who would exit to host, select the...

...> #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: get_online_cpus > include/linux/cpu.h:126 [inline] > #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: > init_vqs+0xe1a/0x1520 drivers/net/virtio_net.c:2777 > #2: (____ptrval____) (xps_map_mutex){+.+.}, at: > __netif_set_xps_queue+0x243/0x23f0 net/core/dev.c:2278 > > stack backtrace: > CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc6+ #141 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dum...

...> #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: get_online_cpus > include/linux/cpu.h:126 [inline] > #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: > init_vqs+0xe1a/0x1520 drivers/net/virtio_net.c:2777 > #2: (____ptrval____) (xps_map_mutex){+.+.}, at: > __netif_set_xps_queue+0x243/0x23f0 net/core/dev.c:2278 > > stack backtrace: > CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc6+ #141 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dum...

...) (cpu_hotplug_lock.rw_sem){++++}, at: get_online_cpus > > include/linux/cpu.h:126 [inline] > > #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: > > init_vqs+0xe1a/0x1520 drivers/net/virtio_net.c:2777 > > #2: (____ptrval____) (xps_map_mutex){+.+.}, at: > > __netif_set_xps_queue+0x243/0x23f0 net/core/dev.c:2278 > > > > stack backtrace: > > CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc6+ #141 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > > Google 01/01/2011 > > Call Trace: > > __dump_stack lib...