* Re: [PATCH] virtio_net: Fix recursive call to cpus_read_lock()
[not found] <20201222033648.14752-1-jdike@akamai.com>
@ 2020-12-22 3:49 ` Jason Wang
2020-12-23 2:30 ` Jakub Kicinski
2020-12-22 22:32 ` Michael S. Tsirkin
1 sibling, 1 reply; 3+ messages in thread
From: Jason Wang @ 2020-12-22 3:49 UTC (permalink / raw)
To: Jeff Dike, Michael S. Tsirkin; +Cc: virtualization, netdev@vger.kernel.org
On 2020/12/22 上午11:36, Jeff Dike wrote:
> virtnet_set_channels can recursively call cpus_read_lock if CONFIG_XPS
> and CONFIG_HOTPLUG are enabled.
>
> The path is:
> virtnet_set_channels - calls get_online_cpus(), which is a trivial
> wrapper around cpus_read_lock()
> netif_set_real_num_tx_queues
> netif_reset_xps_queues_gt
> netif_reset_xps_queues - calls cpus_read_lock()
>
> This call chain and potential deadlock happens when the number of TX
> queues is reduced.
>
> This commit the removes netif_set_real_num_[tr]x_queues calls from
> inside the get/put_online_cpus section, as they don't require that it
> be held.
>
> Signed-off-by: Jeff Dike <jdike@akamai.com>
> ---
Adding netdev.
The patch can go with -net and is needed for -stable.
Acked-by: Jason Wang <jasowang@redhat.com>
> drivers/net/virtio_net.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 052975ea0af4..e02c7e0f1cf9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2093,14 +2093,16 @@ static int virtnet_set_channels(struct net_device *dev,
>
> get_online_cpus();
> err = _virtnet_set_queues(vi, queue_pairs);
> - if (!err) {
> - netif_set_real_num_tx_queues(dev, queue_pairs);
> - netif_set_real_num_rx_queues(dev, queue_pairs);
> -
> - virtnet_set_affinity(vi);
> + if (err){
> + put_online_cpus();
> + goto err;
> }
> + virtnet_set_affinity(vi);
> put_online_cpus();
>
> + netif_set_real_num_tx_queues(dev, queue_pairs);
> + netif_set_real_num_rx_queues(dev, queue_pairs);
> + err:
> return err;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio_net: Fix recursive call to cpus_read_lock()
[not found] <20201222033648.14752-1-jdike@akamai.com>
2020-12-22 3:49 ` [PATCH] virtio_net: Fix recursive call to cpus_read_lock() Jason Wang
@ 2020-12-22 22:32 ` Michael S. Tsirkin
1 sibling, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2020-12-22 22:32 UTC (permalink / raw)
To: Jeff Dike; +Cc: Jason Wang, virtualization, netdev, David Miller
On Mon, Dec 21, 2020 at 10:36:48PM -0500, Jeff Dike wrote:
> virtnet_set_channels can recursively call cpus_read_lock if CONFIG_XPS
> and CONFIG_HOTPLUG are enabled.
>
> The path is:
> virtnet_set_channels - calls get_online_cpus(), which is a trivial
> wrapper around cpus_read_lock()
> netif_set_real_num_tx_queues
> netif_reset_xps_queues_gt
> netif_reset_xps_queues - calls cpus_read_lock()
>
> This call chain and potential deadlock happens when the number of TX
> queues is reduced.
>
> This commit the removes netif_set_real_num_[tr]x_queues calls from
> inside the get/put_online_cpus section, as they don't require that it
> be held.
>
> Signed-off-by: Jeff Dike <jdike@akamai.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 052975ea0af4..e02c7e0f1cf9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2093,14 +2093,16 @@ static int virtnet_set_channels(struct net_device *dev,
>
> get_online_cpus();
> err = _virtnet_set_queues(vi, queue_pairs);
> - if (!err) {
> - netif_set_real_num_tx_queues(dev, queue_pairs);
> - netif_set_real_num_rx_queues(dev, queue_pairs);
> -
> - virtnet_set_affinity(vi);
> + if (err){
> + put_online_cpus();
> + goto err;
> }
> + virtnet_set_affinity(vi);
> put_online_cpus();
>
> + netif_set_real_num_tx_queues(dev, queue_pairs);
> + netif_set_real_num_rx_queues(dev, queue_pairs);
> + err:
> return err;
> }
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio_net: Fix recursive call to cpus_read_lock()
2020-12-22 3:49 ` [PATCH] virtio_net: Fix recursive call to cpus_read_lock() Jason Wang
@ 2020-12-23 2:30 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2020-12-23 2:30 UTC (permalink / raw)
To: Jason Wang
Cc: Jeff Dike, Michael S. Tsirkin, virtualization,
netdev@vger.kernel.org
On Tue, 22 Dec 2020 11:49:04 +0800 Jason Wang wrote:
> On 2020/12/22 上午11:36, Jeff Dike wrote:
> > virtnet_set_channels can recursively call cpus_read_lock if CONFIG_XPS
> > and CONFIG_HOTPLUG are enabled.
> >
> > The path is:
> > virtnet_set_channels - calls get_online_cpus(), which is a trivial
> > wrapper around cpus_read_lock()
> > netif_set_real_num_tx_queues
> > netif_reset_xps_queues_gt
> > netif_reset_xps_queues - calls cpus_read_lock()
> >
> > This call chain and potential deadlock happens when the number of TX
> > queues is reduced.
> >
> > This commit the removes netif_set_real_num_[tr]x_queues calls from
> > inside the get/put_online_cpus section, as they don't require that it
> > be held.
> >
> > Signed-off-by: Jeff Dike <jdike@akamai.com>
>
> Adding netdev.
>
> The patch can go with -net and is needed for -stable.
>
> Acked-by: Jason Wang <jasowang@redhat.com>
Thanks, we'll need a repost with netdev CCed to apply it to net,
I don't have this one in my inbox or in patchwork.
Jeff please make sure to keep the acks when reposting.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-23 2:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20201222033648.14752-1-jdike@akamai.com>
2020-12-22 3:49 ` [PATCH] virtio_net: Fix recursive call to cpus_read_lock() Jason Wang
2020-12-23 2:30 ` Jakub Kicinski
2020-12-22 22:32 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).