From: Amir Tzin In case an auxiliary device with IRQs directory is unbinded, the directory is released, but auxdev->sysfs.irq_dir_exists remains true. This leads to a failure recreating the directory on bind [1]. Expose functions auxiliary_device_sysfs_irq_dir_init() and auxiliary_device_sysfs_irq_dir_destroy(). Move the responsibility for the IRQs directory creation and destruction to the drivers. This change corresponds to the general concept according to which the core driver manages the auxiliary device locking and lifetime. Now the auxiliary device sysfs related fields, irq_dir_exists and lock, are redundant and can be removed. mlx5 SFs, the only users of IRQs sysfs API, must align. Create the directory before a SF control irq is requested and destroy it upon its release. [1] [] mlx5_core.sf mlx5_core.sf.2: mlx5_irq_affinity_request:167:(pid 1939): Failed to create sysfs entry for irq 56, ret = -2 [] mlx5_core.sf mlx5_core.sf.2: mlx5_eq_table_create:1195:(pid 1939): Failed to create async EQs [] mlx5_core.sf mlx5_core.sf.2: mlx5_load:1362:(pid 1939): Failed to create EQs Fixes: a808878308a8 ("driver core: auxiliary bus: show auxiliary device IRQs") Signed-off-by: Amir Tzin Reviewed-by: Leon Romanovsky Signed-off-by: Tariq Toukan --- drivers/base/auxiliary.c | 1 - drivers/base/auxiliary_sysfs.c | 48 ++++++++++++++----- .../net/ethernet/mellanox/mlx5/core/pci_irq.c | 20 ++++++++ include/linux/auxiliary_bus.h | 16 +++++-- 4 files changed, 66 insertions(+), 19 deletions(-) V2: - Move the irq directory management from core to drivers: expose auxiliary_device_sysfs_irq_dir_init() auxiliary_device_sysfs_irq_dir_destroy(). Drop the attribute group visibility approach. Remove the mutex from the auxiliary sysfs structure (drivers are responsible for concurrency). - Revert auxiliary_device_sysfs_irq_remove() to return void. - Call auxiliary_device_sysfs_irq_dir_(init|destroy) in mlx5 SF device create/remove flows. - Link to V1: https://lore.kernel.org/all/1761200367-922346-1-git-send-email-tariqt@nvidia.com/ diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c index 04bdbff4dbe5..e1b8c4bc306e 100644 --- a/drivers/base/auxiliary.c +++ b/drivers/base/auxiliary.c @@ -294,7 +294,6 @@ int auxiliary_device_init(struct auxiliary_device *auxdev) dev->bus = &auxiliary_bus_type; device_initialize(&auxdev->dev); - mutex_init(&auxdev->sysfs.lock); return 0; } EXPORT_SYMBOL_GPL(auxiliary_device_init); diff --git a/drivers/base/auxiliary_sysfs.c b/drivers/base/auxiliary_sysfs.c index 754f21730afd..598a012ce481 100644 --- a/drivers/base/auxiliary_sysfs.c +++ b/drivers/base/auxiliary_sysfs.c @@ -22,22 +22,47 @@ static const struct attribute_group auxiliary_irqs_group = { .attrs = auxiliary_irq_attrs, }; -static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev) +/** + * auxiliary_device_sysfs_irq_dir_init - initialize the IRQ sysfs directory + * @auxdev: auxiliary bus device to initialize the sysfs directory. + * + * This function should be called by drivers to initialize the IRQ directory + * before adding any IRQ sysfs entries. The driver is responsible for ensuring + * this function is called only once and for handling any concurrency control + * if needed. + * + * Drivers must call auxiliary_device_sysfs_irq_dir_destroy() to clean up when + * done. + * + * Return: zero on success or an error code on failure. + */ +int auxiliary_device_sysfs_irq_dir_init(struct auxiliary_device *auxdev) { - int ret = 0; - - guard(mutex)(&auxdev->sysfs.lock); - if (auxdev->sysfs.irq_dir_exists) - return 0; + int ret; - ret = devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group); + ret = sysfs_create_group(&auxdev->dev.kobj, &auxiliary_irqs_group); if (ret) return ret; - auxdev->sysfs.irq_dir_exists = true; xa_init(&auxdev->sysfs.irqs); return 0; } +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_dir_init); + +/** + * auxiliary_device_sysfs_irq_dir_destroy - destroy the IRQ sysfs directory + * @auxdev: auxiliary bus device to destroy the sysfs directory. + * + * This function should be called by drivers to clean up the IRQ directory + * after all IRQ sysfs entries have been removed. The driver is responsible + * for ensuring all IRQs are removed before calling this function. + */ +void auxiliary_device_sysfs_irq_dir_destroy(struct auxiliary_device *auxdev) +{ + xa_destroy(&auxdev->sysfs.irqs); + sysfs_remove_group(&auxdev->dev.kobj, &auxiliary_irqs_group); +} +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_dir_destroy); /** * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ @@ -45,7 +70,8 @@ static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev) * @irq: The associated interrupt number. * * This function should be called after auxiliary device have successfully - * received the irq. + * received the irq. The driver must call auxiliary_device_sysfs_irq_dir_init() + * before calling this function for the first time. * The driver is responsible to add a unique irq for the auxiliary device. The * driver can invoke this function from multiple thread context safely for * unique irqs of the auxiliary devices. The driver must not invoke this API @@ -59,10 +85,6 @@ int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq) struct device *dev = &auxdev->dev; int ret; - ret = auxiliary_irq_dir_prepare(auxdev); - if (ret) - return ret; - info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c b/drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c index aa3b5878e3da..b6e872d7a925 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c @@ -456,7 +456,15 @@ static void _mlx5_irq_release(struct mlx5_irq *irq) */ void mlx5_ctrl_irq_release(struct mlx5_core_dev *dev, struct mlx5_irq *ctrl_irq) { + struct mlx5_irq_pool *pool = ctrl_irq_pool_get(dev); + mlx5_irq_affinity_irq_release(dev, ctrl_irq); + + if (mlx5_irq_pool_is_sf_pool(pool)) { + struct auxiliary_device *sadev = mlx5_sf_coredev_to_adev(dev); + + auxiliary_device_sysfs_irq_dir_destroy(sadev); + } } /** @@ -489,9 +497,21 @@ struct mlx5_irq *mlx5_ctrl_irq_request(struct mlx5_core_dev *dev) /* Allocate the IRQ in index 0. The vector was already allocated */ irq = irq_pool_request_vector(pool, 0, af_desc, NULL); } else { + struct auxiliary_device *sadev = mlx5_sf_coredev_to_adev(dev); + int err; + + err = auxiliary_device_sysfs_irq_dir_init(sadev); + if (err) { + irq = ERR_PTR(err); + goto exit; + } + irq = mlx5_irq_affinity_request(dev, pool, af_desc); + if (IS_ERR(irq)) + auxiliary_device_sysfs_irq_dir_destroy(sadev); } +exit: kvfree(af_desc); return irq; diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h index 4086afd0cc6b..d052cb687e6f 100644 --- a/include/linux/auxiliary_bus.h +++ b/include/linux/auxiliary_bus.h @@ -60,8 +60,6 @@ * @id: unique identitier if multiple devices of the same name are exported, * @sysfs: embedded struct which hold all sysfs related fields, * @sysfs.irqs: irqs xarray contains irq indices which are used by the device, - * @sysfs.lock: Synchronize irq sysfs creation, - * @sysfs.irq_dir_exists: whether "irqs" directory exists, * * An auxiliary_device represents a part of its parent device's functionality. * It is given a name that, combined with the registering drivers @@ -145,8 +143,6 @@ struct auxiliary_device { u32 id; struct { struct xarray irqs; - struct mutex lock; /* Synchronize irq sysfs creation */ - bool irq_dir_exists; } sysfs; }; @@ -222,10 +218,21 @@ int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname) #define auxiliary_device_add(auxdev) __auxiliary_device_add(auxdev, KBUILD_MODNAME) #ifdef CONFIG_SYSFS +int auxiliary_device_sysfs_irq_dir_init(struct auxiliary_device *auxdev); +void auxiliary_device_sysfs_irq_dir_destroy(struct auxiliary_device *auxdev); int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq); void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq); #else /* CONFIG_SYSFS */ +static inline int +auxiliary_device_sysfs_irq_dir_init(struct auxiliary_device *auxdev) +{ + return 0; +} + +static inline void +auxiliary_device_sysfs_irq_dir_destroy(struct auxiliary_device *auxdev) {} + static inline int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq) { @@ -238,7 +245,6 @@ auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {} static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev) { - mutex_destroy(&auxdev->sysfs.lock); put_device(&auxdev->dev); } base-commit: 8dfce8991b95d8625d0a1d2896e42f93b9d7f68d -- 2.44.0