| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZShpScwHoLV/2Bi2@x130>
Date: Thu, 12 Oct 2023 14:46:49 -0700
From: Saeed Mahameed <saeed@...nel.org>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Eric Dumazet <edumazet@...gle.com>,
Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org,
Tariq Toukan <tariqt@...dia.com>, Shay Drory <shayd@...dia.com>,
Mark Bloch <mbloch@...dia.com>
Subject: Re: [net-next V2 04/15] net/mlx5: Refactor LAG peer device lookout
bus logic to mlx5 devcom
On 12 Oct 14:26, Jacob Keller wrote:
>
>
>On 10/12/2023 12:27 PM, Saeed Mahameed wrote:
>> From: Shay Drory <shayd@...dia.com>
>>
>> LAG peer device lookout bus logic required the usage of global lock,
>> mlx5_intf_mutex.
>> As part of the effort to remove this global lock, refactor LAG peer
>> device lookout to use mlx5 devcom layer.
>>
>> Signed-off-by: Shay Drory <shayd@...dia.com>
>> Reviewed-by: Mark Bloch <mbloch@...dia.com>
>> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/dev.c | 68 -------------------
>> .../net/ethernet/mellanox/mlx5/core/lag/lag.c | 12 ++--
>> .../ethernet/mellanox/mlx5/core/lib/devcom.c | 14 ++++
>> .../ethernet/mellanox/mlx5/core/lib/devcom.h | 4 ++
>> .../net/ethernet/mellanox/mlx5/core/main.c | 25 +++++++
>> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 1 -
>> include/linux/mlx5/driver.h | 1 +
>> 7 files changed, 52 insertions(+), 73 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
>> index 1fc03480c2ff..6e3a8c22881f 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
>> @@ -566,74 +566,6 @@ bool mlx5_same_hw_devs(struct mlx5_core_dev *dev, struct mlx5_core_dev *peer_dev
>> return (fsystem_guid && psystem_guid && fsystem_guid == psystem_guid);
>> }
>>
>> -static u32 mlx5_gen_pci_id(const struct mlx5_core_dev *dev)
>> -{
>> - return (u32)((pci_domain_nr(dev->pdev->bus) << 16) |
>> - (dev->pdev->bus->number << 8) |
>> - PCI_SLOT(dev->pdev->devfn));
>> -}
>> -
>> -static int _next_phys_dev(struct mlx5_core_dev *mdev,
>> - const struct mlx5_core_dev *curr)
>> -{
>> - if (!mlx5_core_is_pf(mdev))
>> - return 0;
>> -
>> - if (mdev == curr)
>> - return 0;
>> -
>> - if (!mlx5_same_hw_devs(mdev, (struct mlx5_core_dev *)curr) &&
>> - mlx5_gen_pci_id(mdev) != mlx5_gen_pci_id(curr))
>> - return 0;
>> -
>> - return 1;
>> -}
>> -
>> -static void *pci_get_other_drvdata(struct device *this, struct device *other)
>> -{
>> - if (this->driver != other->driver)
>> - return NULL;
>> -
>> - return pci_get_drvdata(to_pci_dev(other));
>> -}
>> -
>> -static int next_phys_dev_lag(struct device *dev, const void *data)
>> -{
>> - struct mlx5_core_dev *mdev, *this = (struct mlx5_core_dev *)data;
>> -
>> - mdev = pci_get_other_drvdata(this->device, dev);
>> - if (!mdev)
>> - return 0;
>> -
>> - if (!mlx5_lag_is_supported(mdev))
>> - return 0;
>> -
>> - return _next_phys_dev(mdev, data);
>> -}
>> -
>> -static struct mlx5_core_dev *mlx5_get_next_dev(struct mlx5_core_dev *dev,
>> - int (*match)(struct device *dev, const void *data))
>> -{
>> - struct device *next;
>> -
>> - if (!mlx5_core_is_pf(dev))
>> - return NULL;
>> -
>> - next = bus_find_device(&pci_bus_type, NULL, dev, match);
>> - if (!next)
>> - return NULL;
>> -
>> - put_device(next);
>> - return pci_get_drvdata(to_pci_dev(next));
>> -}
>> -
>> -/* Must be called with intf_mutex held */
>> -struct mlx5_core_dev *mlx5_get_next_phys_dev_lag(struct mlx5_core_dev *dev)
>> -{
>> - lockdep_assert_held(&mlx5_intf_mutex);
>
>The old flow had a lockdep_assert_held
>
>> - return mlx5_get_next_dev(dev, &next_phys_dev_lag);
>> -}
>> -
>> void mlx5_dev_list_lock(void)
>> {
>> mutex_lock(&mlx5_intf_mutex);
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> index af3fac090b82..f0b57f97739f 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> @@ -1212,13 +1212,14 @@ static void mlx5_ldev_remove_mdev(struct mlx5_lag *ldev,
>> dev->priv.lag = NULL;
>> }
>>
>> -/* Must be called with intf_mutex held */
>> +/* Must be called with HCA devcom component lock held */
>> static int __mlx5_lag_dev_add_mdev(struct mlx5_core_dev *dev)
>> {
>> + struct mlx5_devcom_comp_dev *pos = NULL;
>> struct mlx5_lag *ldev = NULL;
>> struct mlx5_core_dev *tmp_dev;
>>
>> - tmp_dev = mlx5_get_next_phys_dev_lag(dev);
>> + tmp_dev = mlx5_devcom_get_next_peer_data(dev->priv.hca_devcom_comp, &pos);
>> if (tmp_dev)
>> ldev = mlx5_lag_dev(tmp_dev);
>>
>
>But you didn't bother to add one here? Does
>mlx5_devcom_get_next_peer_data already do that?
>
The global mutex isn't required anymore after this refactoring,
and devcom has own locking and sync mechanism.
>Not a big deal either way to me.
>
>Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
Powered by blists - more mailing lists