diff mbox series

[net-next,01/10] mlx4: Get rid of the mlx4_interface.get_dev callback

Message ID 20230804150527.6117-2-petr.pavlu@suse.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Convert mlx4 to use auxiliary bus | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1328 this patch: 1328
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 1351 this patch: 1351
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success Found: 'dev_hold(' was: 1 now: 1
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1351 this patch: 1351
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 196 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Petr Pavlu Aug. 4, 2023, 3:05 p.m. UTC 
Simplify the mlx4 driver interface by removing mlx4_get_protocol_dev()
and the associated mlx4_interface.get_dev callbacks. This is done in
preparation to use an auxiliary bus to model the mlx4 driver structure.

The change is motivated by the following situation:
* The mlx4_en interface is being initialized by mlx4_en_add() and
  mlx4_en_activate().
* The latter activate function calls mlx4_en_init_netdev() ->
  register_netdev() to register a new net_device.
* A netdev event NETDEV_REGISTER is raised for the device.
* The netdev notififier mlx4_ib_netdev_event() is called and it invokes
  mlx4_ib_scan_netdevs() -> mlx4_get_protocol_dev() ->
  mlx4_en_get_netdev() [via mlx4_interface.get_dev].

This chain creates a problem when mlx4_en gets switched to be an
auxiliary driver. It contains two device calls which would both need to
take a respective device lock.

Avoid this situation by updating mlx4_ib_scan_netdevs() to no longer
call mlx4_get_protocol_dev() but instead to utilize the information
passed in net_device.parent and net_device.dev_port. This data is
sufficient to determine that an updated port is one that the mlx4_ib
driver should take care of and to keep mlx4_ib_dev.iboe.netdevs up to
date.

Following that, update mlx4_ib_get_netdev() to also not call
mlx4_get_protocol_dev() and instead scan all current netdevs to find
find a matching one. Note that mlx4_ib_get_netdev() is called early from
ib_register_device() and cannot use data tracked in
mlx4_ib_dev.iboe.netdevs which is not at that point yet set.

Finally, remove function mlx4_get_protocol_dev() and the
mlx4_interface.get_dev callbacks (only mlx4_en_get_netdev()) as they
became unused.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Tested-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/hw/mlx4/main.c            | 89 ++++++++++----------
 drivers/net/ethernet/mellanox/mlx4/en_main.c |  8 --
 drivers/net/ethernet/mellanox/mlx4/intf.c    | 21 -----
 include/linux/mlx4/driver.h                  |  3 -
 4 files changed, 43 insertions(+), 78 deletions(-)

Comments

Leon Romanovsky Aug. 8, 2023, 6:55 p.m. UTC | #1 
On Fri, Aug 04, 2023 at 05:05:18PM +0200, Petr Pavlu wrote:
> Simplify the mlx4 driver interface by removing mlx4_get_protocol_dev()
> and the associated mlx4_interface.get_dev callbacks. This is done in
> preparation to use an auxiliary bus to model the mlx4 driver structure.
> 
> The change is motivated by the following situation:
> * The mlx4_en interface is being initialized by mlx4_en_add() and
>   mlx4_en_activate().
> * The latter activate function calls mlx4_en_init_netdev() ->
>   register_netdev() to register a new net_device.
> * A netdev event NETDEV_REGISTER is raised for the device.
> * The netdev notififier mlx4_ib_netdev_event() is called and it invokes
>   mlx4_ib_scan_netdevs() -> mlx4_get_protocol_dev() ->
>   mlx4_en_get_netdev() [via mlx4_interface.get_dev].
> 
> This chain creates a problem when mlx4_en gets switched to be an
> auxiliary driver. It contains two device calls which would both need to
> take a respective device lock.
> 
> Avoid this situation by updating mlx4_ib_scan_netdevs() to no longer
> call mlx4_get_protocol_dev() but instead to utilize the information
> passed in net_device.parent and net_device.dev_port. This data is
> sufficient to determine that an updated port is one that the mlx4_ib
> driver should take care of and to keep mlx4_ib_dev.iboe.netdevs up to
> date.
> 
> Following that, update mlx4_ib_get_netdev() to also not call
> mlx4_get_protocol_dev() and instead scan all current netdevs to find
> find a matching one. Note that mlx4_ib_get_netdev() is called early from
> ib_register_device() and cannot use data tracked in
> mlx4_ib_dev.iboe.netdevs which is not at that point yet set.
> 
> Finally, remove function mlx4_get_protocol_dev() and the
> mlx4_interface.get_dev callbacks (only mlx4_en_get_netdev()) as they
> became unused.
> 
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> Tested-by: Leon Romanovsky <leon@kernel.org>
> ---
>  drivers/infiniband/hw/mlx4/main.c            | 89 ++++++++++----------
>  drivers/net/ethernet/mellanox/mlx4/en_main.c |  8 --
>  drivers/net/ethernet/mellanox/mlx4/intf.c    | 21 -----
>  include/linux/mlx4/driver.h                  |  3 -
>  4 files changed, 43 insertions(+), 78 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index b18e9f2adc82..7dd70d778b6b 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -125,12 +125,14 @@  static struct net_device *mlx4_ib_get_netdev(struct ib_device *device,
 					     u32 port_num)
 {
 	struct mlx4_ib_dev *ibdev = to_mdev(device);
-	struct net_device *dev;
+	struct net_device *dev, *ret = NULL;
 
 	rcu_read_lock();
-	dev = mlx4_get_protocol_dev(ibdev->dev, MLX4_PROT_ETH, port_num);
+	for_each_netdev_rcu(&init_net, dev) {
+		if (dev->dev.parent != ibdev->ib_dev.dev.parent ||
+		    dev->dev_port + 1 != port_num)
+			continue;
 
-	if (dev) {
 		if (mlx4_is_bonded(ibdev->dev)) {
 			struct net_device *upper = NULL;
 
@@ -143,11 +145,14 @@  static struct net_device *mlx4_ib_get_netdev(struct ib_device *device,
 					dev = active;
 			}
 		}
+
+		dev_hold(dev);
+		ret = dev;
+		break;
 	}
-	dev_hold(dev);
 
 	rcu_read_unlock();
-	return dev;
+	return ret;
 }
 
 static int mlx4_ib_update_gids_v1(struct gid_entry *gids,
@@ -2319,61 +2324,53 @@  static void mlx4_ib_update_qps(struct mlx4_ib_dev *ibdev,
 	mutex_unlock(&ibdev->qp1_proxy_lock[port - 1]);
 }
 
-static void mlx4_ib_scan_netdevs(struct mlx4_ib_dev *ibdev,
-				 struct net_device *dev,
-				 unsigned long event)
+static void mlx4_ib_scan_netdev(struct mlx4_ib_dev *ibdev,
+				struct net_device *dev,
+				unsigned long event)
 
 {
-	struct mlx4_ib_iboe *iboe;
-	int update_qps_port = -1;
-	int port;
+	struct mlx4_ib_iboe *iboe = &ibdev->iboe;
 
 	ASSERT_RTNL();
 
-	iboe = &ibdev->iboe;
+	if (dev->dev.parent != ibdev->ib_dev.dev.parent)
+		return;
 
 	spin_lock_bh(&iboe->lock);
-	mlx4_foreach_ib_transport_port(port, ibdev->dev) {
-
-		iboe->netdevs[port - 1] =
-			mlx4_get_protocol_dev(ibdev->dev, MLX4_PROT_ETH, port);
 
-		if (dev == iboe->netdevs[port - 1] &&
-		    (event == NETDEV_CHANGEADDR || event == NETDEV_REGISTER ||
-		     event == NETDEV_UP || event == NETDEV_CHANGE))
-			update_qps_port = port;
+	iboe->netdevs[dev->dev_port] = event != NETDEV_UNREGISTER ? dev : NULL;
 
-		if (dev == iboe->netdevs[port - 1] &&
-		    (event == NETDEV_UP || event == NETDEV_DOWN)) {
-			enum ib_port_state port_state;
-			struct ib_event ibev = { };
-
-			if (ib_get_cached_port_state(&ibdev->ib_dev, port,
-						     &port_state))
-				continue;
+	if (event == NETDEV_UP || event == NETDEV_DOWN) {
+		enum ib_port_state port_state;
+		struct ib_event ibev = { };
 
-			if (event == NETDEV_UP &&
-			    (port_state != IB_PORT_ACTIVE ||
-			     iboe->last_port_state[port - 1] != IB_PORT_DOWN))
-				continue;
-			if (event == NETDEV_DOWN &&
-			    (port_state != IB_PORT_DOWN ||
-			     iboe->last_port_state[port - 1] != IB_PORT_ACTIVE))
-				continue;
-			iboe->last_port_state[port - 1] = port_state;
+		if (ib_get_cached_port_state(&ibdev->ib_dev, dev->dev_port + 1,
+					     &port_state))
+			goto iboe_out;
 
-			ibev.device = &ibdev->ib_dev;
-			ibev.element.port_num = port;
-			ibev.event = event == NETDEV_UP ? IB_EVENT_PORT_ACTIVE :
-							  IB_EVENT_PORT_ERR;
-			ib_dispatch_event(&ibev);
-		}
+		if (event == NETDEV_UP &&
+		    (port_state != IB_PORT_ACTIVE ||
+		     iboe->last_port_state[dev->dev_port] != IB_PORT_DOWN))
+			goto iboe_out;
+		if (event == NETDEV_DOWN &&
+		    (port_state != IB_PORT_DOWN ||
+		     iboe->last_port_state[dev->dev_port] != IB_PORT_ACTIVE))
+			goto iboe_out;
+		iboe->last_port_state[dev->dev_port] = port_state;
 
+		ibev.device = &ibdev->ib_dev;
+		ibev.element.port_num = dev->dev_port + 1;
+		ibev.event = event == NETDEV_UP ? IB_EVENT_PORT_ACTIVE :
+						  IB_EVENT_PORT_ERR;
+		ib_dispatch_event(&ibev);
 	}
+
+iboe_out:
 	spin_unlock_bh(&iboe->lock);
 
-	if (update_qps_port > 0)
-		mlx4_ib_update_qps(ibdev, dev, update_qps_port);
+	if (event == NETDEV_CHANGEADDR || event == NETDEV_REGISTER ||
+	    event == NETDEV_UP || event == NETDEV_CHANGE)
+		mlx4_ib_update_qps(ibdev, dev, dev->dev_port + 1);
 }
 
 static int mlx4_ib_netdev_event(struct notifier_block *this,
@@ -2386,7 +2383,7 @@  static int mlx4_ib_netdev_event(struct notifier_block *this,
 		return NOTIFY_DONE;
 
 	ibdev = container_of(this, struct mlx4_ib_dev, iboe.nb);
-	mlx4_ib_scan_netdevs(ibdev, dev, event);
+	mlx4_ib_scan_netdev(ibdev, dev, event);
 
 	return NOTIFY_DONE;
 }
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_main.c b/drivers/net/ethernet/mellanox/mlx4/en_main.c
index f1259bdb1a29..6a42bec6bd85 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_main.c
@@ -183,13 +183,6 @@  static void mlx4_en_get_profile(struct mlx4_en_dev *mdev)
 	}
 }
 
-static void *mlx4_en_get_netdev(struct mlx4_dev *dev, void *ctx, u8 port)
-{
-	struct mlx4_en_dev *endev = ctx;
-
-	return endev->pndev[port];
-}
-
 static void mlx4_en_event(struct mlx4_dev *dev, void *endev_ptr,
 			  enum mlx4_dev_event event, unsigned long port)
 {
@@ -354,7 +347,6 @@  static struct mlx4_interface mlx4_en_interface = {
 	.add		= mlx4_en_add,
 	.remove		= mlx4_en_remove,
 	.event		= mlx4_en_event,
-	.get_dev	= mlx4_en_get_netdev,
 	.protocol	= MLX4_PROT_ETH,
 	.activate	= mlx4_en_activate,
 };
diff --git a/drivers/net/ethernet/mellanox/mlx4/intf.c b/drivers/net/ethernet/mellanox/mlx4/intf.c
index 65482f004e50..28d7da925d36 100644
--- a/drivers/net/ethernet/mellanox/mlx4/intf.c
+++ b/drivers/net/ethernet/mellanox/mlx4/intf.c
@@ -245,27 +245,6 @@  void mlx4_unregister_device(struct mlx4_dev *dev)
 	mutex_unlock(&intf_mutex);
 }
 
-void *mlx4_get_protocol_dev(struct mlx4_dev *dev, enum mlx4_protocol proto, int port)
-{
-	struct mlx4_priv *priv = mlx4_priv(dev);
-	struct mlx4_device_context *dev_ctx;
-	unsigned long flags;
-	void *result = NULL;
-
-	spin_lock_irqsave(&priv->ctx_lock, flags);
-
-	list_for_each_entry(dev_ctx, &priv->ctx_list, list)
-		if (dev_ctx->intf->protocol == proto && dev_ctx->intf->get_dev) {
-			result = dev_ctx->intf->get_dev(dev, dev_ctx->context, port);
-			break;
-		}
-
-	spin_unlock_irqrestore(&priv->ctx_lock, flags);
-
-	return result;
-}
-EXPORT_SYMBOL_GPL(mlx4_get_protocol_dev);
-
 struct devlink_port *mlx4_get_devlink_port(struct mlx4_dev *dev, int port)
 {
 	struct mlx4_port_info *info = &mlx4_priv(dev)->port[port];
diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
index 1834c8fad12e..923951e19300 100644
--- a/include/linux/mlx4/driver.h
+++ b/include/linux/mlx4/driver.h
@@ -59,7 +59,6 @@  struct mlx4_interface {
 	void			(*remove)(struct mlx4_dev *dev, void *context);
 	void			(*event) (struct mlx4_dev *dev, void *context,
 					  enum mlx4_dev_event event, unsigned long param);
-	void *			(*get_dev)(struct mlx4_dev *dev, void *context, u8 port);
 	void			(*activate)(struct mlx4_dev *dev, void *context);
 	struct list_head	list;
 	enum mlx4_protocol	protocol;
@@ -88,8 +87,6 @@  struct mlx4_port_map {
 
 int mlx4_port_map_set(struct mlx4_dev *dev, struct mlx4_port_map *v2p);
 
-void *mlx4_get_protocol_dev(struct mlx4_dev *dev, enum mlx4_protocol proto, int port);
-
 struct devlink_port *mlx4_get_devlink_port(struct mlx4_dev *dev, int port);
 
 #endif /* MLX4_DRIVER_H */