2023-08-13 16:02:48

by Petr Pavlu

[permalink] [raw]
Subject: [PATCH net-next v2 00/10] Convert mlx4 to use auxiliary bus

This series converts the mlx4 drivers to use auxiliary bus, similarly to
how mlx5 was converted [1]. The first 6 patches are preparatory changes,
the remaining 4 are the final conversion.

Initial motivation for this change was to address a problem related to
loading mlx4_en/mlx4_ib by mlx4_core using request_module_nowait(). When
doing such a load in initrd, the operation is asynchronous to any init
control and can get unexpectedly affected/interrupted by an eventual
root switch. Using an auxiliary bus leaves these module loads to udevd
which better integrates with systemd processing. [2]

General benefit is to get rid of custom interface logic and instead use
a common facility available for this task. An obvious risk is that some
new bug is introduced by the conversion.

Leon Romanovsky was kind enough to check for me that the series passes
their verification tests.

Changes since v1 [3]:
* Fix a missing definition of the err variable in mlx4_en_add().
* Remove not needed comments about the event type in mlx4_en_event()
and mlx4_ib_event().

[1] https://lore.kernel.org/netdev/[email protected]/
[2] https://lore.kernel.org/netdev/[email protected]/
[3] https://lore.kernel.org/netdev/[email protected]/

Petr Pavlu (10):
mlx4: Get rid of the mlx4_interface.get_dev callback
mlx4: Rename member mlx4_en_dev.nb to netdev_nb
mlx4: Replace the mlx4_interface.event callback with a notifier
mlx4: Get rid of the mlx4_interface.activate callback
mlx4: Move the bond work to the core driver
mlx4: Avoid resetting MLX4_INTFF_BONDING per driver
mlx4: Register mlx4 devices to an auxiliary virtual bus
mlx4: Connect the ethernet part to the auxiliary bus
mlx4: Connect the infiniband part to the auxiliary bus
mlx4: Delete custom device management logic

drivers/infiniband/hw/mlx4/main.c | 207 ++++++----
drivers/infiniband/hw/mlx4/mlx4_ib.h | 2 +
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 +
drivers/net/ethernet/mellanox/mlx4/en_main.c | 141 ++++---
.../net/ethernet/mellanox/mlx4/en_netdev.c | 64 +---
drivers/net/ethernet/mellanox/mlx4/intf.c | 361 ++++++++++++------
drivers/net/ethernet/mellanox/mlx4/main.c | 110 ++++--
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 16 +-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 4 +-
include/linux/mlx4/device.h | 20 +
include/linux/mlx4/driver.h | 42 +-
11 files changed, 572 insertions(+), 396 deletions(-)


base-commit: 2f4503f94c5d81d1589842bfb457be466c8c670b
--
2.35.3



2023-08-13 16:03:03

by Petr Pavlu

[permalink] [raw]
Subject: [PATCH net-next v2 10/10] mlx4: Delete custom device management logic

After the conversion to use the auxiliary bus, the custom device
management is not needed anymore and can be deleted.

Signed-off-by: Petr Pavlu <[email protected]>
Tested-by: Leon Romanovsky <[email protected]>
Reviewed-by: Leon Romanovsky <[email protected]>
Acked-by: Tariq Toukan <[email protected]>
---
drivers/net/ethernet/mellanox/mlx4/intf.c | 125 ----------------------
drivers/net/ethernet/mellanox/mlx4/main.c | 28 -----
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 -
include/linux/mlx4/driver.h | 10 --
4 files changed, 166 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/intf.c b/drivers/net/ethernet/mellanox/mlx4/intf.c
index 16b2c99ff737..c7697ee0dd05 100644
--- a/drivers/net/ethernet/mellanox/mlx4/intf.c
+++ b/drivers/net/ethernet/mellanox/mlx4/intf.c
@@ -38,15 +38,6 @@

#include "mlx4.h"

-struct mlx4_device_context {
- struct list_head list;
- struct list_head bond_list;
- struct mlx4_interface *intf;
- void *context;
-};
-
-static LIST_HEAD(intf_list);
-static LIST_HEAD(dev_list);
static DEFINE_MUTEX(intf_mutex);
static DEFINE_IDA(mlx4_adev_ida);

@@ -156,77 +147,6 @@ static void del_adev(struct auxiliary_device *adev)
auxiliary_device_uninit(adev);
}

-static void mlx4_add_device(struct mlx4_interface *intf, struct mlx4_priv *priv)
-{
- struct mlx4_device_context *dev_ctx;
-
- dev_ctx = kmalloc(sizeof(*dev_ctx), GFP_KERNEL);
- if (!dev_ctx)
- return;
-
- dev_ctx->intf = intf;
- dev_ctx->context = intf->add(&priv->dev);
-
- if (dev_ctx->context) {
- spin_lock_irq(&priv->ctx_lock);
- list_add_tail(&dev_ctx->list, &priv->ctx_list);
- spin_unlock_irq(&priv->ctx_lock);
- } else
- kfree(dev_ctx);
-
-}
-
-static void mlx4_remove_device(struct mlx4_interface *intf, struct mlx4_priv *priv)
-{
- struct mlx4_device_context *dev_ctx;
-
- list_for_each_entry(dev_ctx, &priv->ctx_list, list)
- if (dev_ctx->intf == intf) {
- spin_lock_irq(&priv->ctx_lock);
- list_del(&dev_ctx->list);
- spin_unlock_irq(&priv->ctx_lock);
-
- intf->remove(&priv->dev, dev_ctx->context);
- kfree(dev_ctx);
- return;
- }
-}
-
-int mlx4_register_interface(struct mlx4_interface *intf)
-{
- struct mlx4_priv *priv;
-
- if (!intf->add || !intf->remove)
- return -EINVAL;
-
- mutex_lock(&intf_mutex);
-
- list_add_tail(&intf->list, &intf_list);
- list_for_each_entry(priv, &dev_list, dev_list) {
- mlx4_add_device(intf, priv);
- }
-
- mutex_unlock(&intf_mutex);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(mlx4_register_interface);
-
-void mlx4_unregister_interface(struct mlx4_interface *intf)
-{
- struct mlx4_priv *priv;
-
- mutex_lock(&intf_mutex);
-
- list_for_each_entry(priv, &dev_list, dev_list)
- mlx4_remove_device(intf, priv);
-
- list_del(&intf->list);
-
- mutex_unlock(&intf_mutex);
-}
-EXPORT_SYMBOL_GPL(mlx4_unregister_interface);
-
int mlx4_register_auxiliary_driver(struct mlx4_adrv *madrv)
{
return auxiliary_driver_register(&madrv->adrv);
@@ -242,10 +162,7 @@ EXPORT_SYMBOL_GPL(mlx4_unregister_auxiliary_driver);
int mlx4_do_bond(struct mlx4_dev *dev, bool enable)
{
struct mlx4_priv *priv = mlx4_priv(dev);
- struct mlx4_device_context *dev_ctx = NULL, *temp_dev_ctx;
- unsigned long flags;
int i, ret;
- LIST_HEAD(bond_list);

if (!(dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_PORT_REMAP))
return -EOPNOTSUPP;
@@ -267,36 +184,6 @@ int mlx4_do_bond(struct mlx4_dev *dev, bool enable)
dev->flags &= ~MLX4_FLAG_BONDED;
}

- spin_lock_irqsave(&priv->ctx_lock, flags);
- list_for_each_entry_safe(dev_ctx, temp_dev_ctx, &priv->ctx_list, list) {
- if (!(dev_ctx->intf->flags & MLX4_INTFF_BONDING))
- continue;
-
- if (mlx4_is_mfunc(dev)) {
- mlx4_dbg(dev,
- "SRIOV, disabled HA mode for intf proto %d\n",
- dev_ctx->intf->protocol);
- continue;
- }
-
- list_add_tail(&dev_ctx->bond_list, &bond_list);
- list_del(&dev_ctx->list);
- }
- spin_unlock_irqrestore(&priv->ctx_lock, flags);
-
- list_for_each_entry(dev_ctx, &bond_list, bond_list) {
- dev_ctx->intf->remove(dev, dev_ctx->context);
- dev_ctx->context = dev_ctx->intf->add(dev);
-
- spin_lock_irqsave(&priv->ctx_lock, flags);
- list_add_tail(&dev_ctx->list, &priv->ctx_list);
- spin_unlock_irqrestore(&priv->ctx_lock, flags);
-
- mlx4_dbg(dev, "Interface for protocol %d restarted with bonded mode %s\n",
- dev_ctx->intf->protocol, enable ?
- "enabled" : "disabled");
- }
-
mutex_lock(&intf_mutex);

for (i = 0; i < ARRAY_SIZE(mlx4_adev_devices); i++) {
@@ -447,16 +334,11 @@ static int rescan_drivers_locked(struct mlx4_dev *dev)

int mlx4_register_device(struct mlx4_dev *dev)
{
- struct mlx4_priv *priv = mlx4_priv(dev);
- struct mlx4_interface *intf;
int ret;

mutex_lock(&intf_mutex);

dev->persist->interface_state |= MLX4_INTERFACE_STATE_UP;
- list_add_tail(&priv->dev_list, &dev_list);
- list_for_each_entry(intf, &intf_list, list)
- mlx4_add_device(intf, priv);

ret = rescan_drivers_locked(dev);

@@ -474,9 +356,6 @@ int mlx4_register_device(struct mlx4_dev *dev)

void mlx4_unregister_device(struct mlx4_dev *dev)
{
- struct mlx4_priv *priv = mlx4_priv(dev);
- struct mlx4_interface *intf;
-
if (!(dev->persist->interface_state & MLX4_INTERFACE_STATE_UP))
return;

@@ -495,10 +374,6 @@ void mlx4_unregister_device(struct mlx4_dev *dev)
}
mutex_lock(&intf_mutex);

- list_for_each_entry(intf, &intf_list, list)
- mlx4_remove_device(intf, priv);
-
- list_del(&priv->dev_list);
dev->persist->interface_state &= ~MLX4_INTERFACE_STATE_UP;

rescan_drivers_locked(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index c4ec7377aa71..2581226836b5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -42,7 +42,6 @@
#include <linux/slab.h>
#include <linux/io-mapping.h>
#include <linux/delay.h>
-#include <linux/kmod.h>
#include <linux/etherdevice.h>
#include <net/devlink.h>

@@ -1091,27 +1090,6 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
return err;
}

-static void mlx4_request_modules(struct mlx4_dev *dev)
-{
- int port;
- int has_ib_port = false;
- int has_eth_port = false;
-#define EN_DRV_NAME "mlx4_en"
-#define IB_DRV_NAME "mlx4_ib"
-
- for (port = 1; port <= dev->caps.num_ports; port++) {
- if (dev->caps.port_type[port] == MLX4_PORT_TYPE_IB)
- has_ib_port = true;
- else if (dev->caps.port_type[port] == MLX4_PORT_TYPE_ETH)
- has_eth_port = true;
- }
-
- if (has_eth_port)
- request_module_nowait(EN_DRV_NAME);
- if (has_ib_port || (dev->caps.flags & MLX4_DEV_CAP_FLAG_IBOE))
- request_module_nowait(IB_DRV_NAME);
-}
-
/*
* Change the port configuration of the device.
* Every user of this function must hold the port mutex.
@@ -1147,7 +1125,6 @@ int mlx4_change_port_types(struct mlx4_dev *dev,
mlx4_err(dev, "Failed to register device\n");
goto out;
}
- mlx4_request_modules(dev);
}

out:
@@ -3426,9 +3403,6 @@ static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
devl_assert_locked(devlink);
dev = &priv->dev;

- INIT_LIST_HEAD(&priv->ctx_list);
- spin_lock_init(&priv->ctx_lock);
-
err = mlx4_adev_init(dev);
if (err)
return err;
@@ -3732,8 +3706,6 @@ static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
if (err)
goto err_port;

- mlx4_request_modules(dev);
-
mlx4_sense_init(dev);
mlx4_start_sense(dev);

diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index d5050bfb342f..d707b790536f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -882,9 +882,6 @@ enum {
struct mlx4_priv {
struct mlx4_dev dev;

- struct list_head dev_list;
- struct list_head ctx_list;
- spinlock_t ctx_lock;
struct mlx4_adev **adev;
int adev_idx;
struct atomic_notifier_head event_nh;
diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
index 9cf157d381c6..69825223081f 100644
--- a/include/linux/mlx4/driver.h
+++ b/include/linux/mlx4/driver.h
@@ -58,22 +58,12 @@ enum {
MLX4_INTFF_BONDING = 1 << 0
};

-struct mlx4_interface {
- void * (*add) (struct mlx4_dev *dev);
- void (*remove)(struct mlx4_dev *dev, void *context);
- struct list_head list;
- enum mlx4_protocol protocol;
- int flags;
-};
-
struct mlx4_adrv {
struct auxiliary_driver adrv;
enum mlx4_protocol protocol;
int flags;
};

-int mlx4_register_interface(struct mlx4_interface *intf);
-void mlx4_unregister_interface(struct mlx4_interface *intf);
int mlx4_register_auxiliary_driver(struct mlx4_adrv *madrv);
void mlx4_unregister_auxiliary_driver(struct mlx4_adrv *madrv);

--
2.35.3