All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antoine Tenart <atenart@kernel.org>
To: Maxim Mykytianskyi <maximmi@nvidia.com>,
	Saeed Mahameed <saeed@kernel.org>,
	Tariq Toukan <tariqt@nvidia.com>,
	alexander.duyck@gmail.com, davem@davemloft.net, kuba@kernel.org
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net-next v3 15/16] net/mlx5e: take the rtnl lock when calling netif_set_xps_queue
Date: Wed, 17 Mar 2021 09:55:43 +0100	[thread overview]
Message-ID: <161597134352.3996.436408610278743110@kwain.local> (raw)
In-Reply-To: <c6a4224370e57d31b1f28e27e7a7d4e1ab237ec2.camel@kernel.org>

Quoting Saeed Mahameed (2021-03-12 21:54:18)
> On Fri, 2021-03-12 at 16:04 +0100, Antoine Tenart wrote:
> > netif_set_xps_queue must be called with the rtnl lock taken, and this
> > is
> > now enforced using ASSERT_RTNL(). mlx5e_attach_netdev was taking the
> > lock conditionally, fix this by taking the rtnl lock all the time.
> 
> There is a reason why it is conditional:
> we had a bug in the past of double locking here:
> 
> [ 4255.283960] echo/644 is trying to acquire lock:
> 
>  [ 4255.285092] ffffffff85101f90 (rtnl_mutex){+..}, at:
> mlx5e_attach_netdev0xd4/0×3d0 [mlx5_core]
> 
>  [ 4255.287264] 
> 
>  [ 4255.287264] but task is already holding lock:
> 
>  [ 4255.288971] ffffffff85101f90 (rtnl_mutex){+..}, at:
> ipoib_vlan_add0×7c/0×2d0 [ib_ipoib]
> 
> ipoib_vlan_add is called under rtnl and will eventually call 
> mlx5e_attach_netdev, we don't have much control over this in mlx5
> driver since the rdma stack provides a per-prepared netdev to attach to
> our hw. maybe it is time we had a nested rtnl lock .. 

Not sure we want to add a nested rtnl lock because of xps. I'd like to
see other options first. Could be having a locking mechanism for xps not
relying on rtnl; if that's possible.

As for this series, patches 6, 15 (this one) and 16 are not linked to
and do not rely on the other patches. They're improvement or fixes for
already existing behaviours. The series already gained enough new
patches since v1 and I don't want to maintain it out-of-tree for too
long, so I'll resend it without patches 6, 15 and 16; and then we'll be
able to focus on the xps locking relationship with rtnl.

Antoine

  parent reply	other threads:[~2021-03-17  8:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-12 15:04 [PATCH net-next v3 00/16] net: xps: improve the xps maps handling Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 01/16] net-sysfs: convert xps_cpus_show to bitmap_zalloc Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 02/16] net-sysfs: store the return of get_netdev_queue_index in an unsigned int Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 03/16] net-sysfs: make xps_cpus_show and xps_rxqs_show consistent Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 04/16] net: embed num_tc in the xps maps Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 05/16] net: embed nr_ids " Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 06/16] net: assert the rtnl lock is held when calling __netif_set_xps_queue Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 07/16] net: remove the xps possible_mask Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 08/16] net: move the xps maps to an array Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 09/16] net: add an helper to copy xps maps to the new dev_maps Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 10/16] net: improve queue removal readability in __netif_set_xps_queue Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 11/16] net-sysfs: move the rtnl unlock up in the xps show helpers Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 12/16] net-sysfs: move the xps cpus/rxqs retrieval in a common function Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 13/16] net: fix use after free in xps Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 14/16] net: NULL the old xps map entries when freeing them Antoine Tenart
2021-03-12 15:04 ` [PATCH net-next v3 15/16] net/mlx5e: take the rtnl lock when calling netif_set_xps_queue Antoine Tenart
2021-03-12 20:54   ` Saeed Mahameed
2021-03-15  8:38     ` Antoine Tenart
2021-03-15 14:53       ` Maxim Mikityanskiy
2021-03-15 15:13         ` Antoine Tenart
2021-03-17  8:55     ` Antoine Tenart [this message]
2021-03-12 15:04 ` [PATCH net-next v3 16/16] virtio_net: take the rtnl lock when calling virtnet_set_affinity Antoine Tenart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=161597134352.3996.436408610278743110@kwain.local \
    --to=atenart@kernel.org \
    --cc=alexander.duyck@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=maximmi@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeed@kernel.org \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.