[PATCH net] driver core: auxiliary bus: Fix sysfs creation on bind

[Tariq Toukan <tariqt@nvidia.com>]

From: Amir Tzin <amirtz@nvidia.com>

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].

Using the attributes group visibility interface, expose the IRQs
attributes group if"f the xarray storing IRQs entries is not empty. Now
irq_dir_exists field is redundant and can be removed.

[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 <amirtz@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/base/auxiliary.c       |  13 +++-
 drivers/base/auxiliary_sysfs.c | 117 +++++++++++++++++++++++++--------
 include/linux/auxiliary_bus.h  |  26 ++++++--
 3 files changed, 118 insertions(+), 38 deletions(-)

diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index 04bdbff4dbe5..b0fb31279257 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -225,7 +225,16 @@ static int auxiliary_bus_probe(struct device *dev)
 		return ret;
 	}
 
-	return auxdrv->probe(auxdev, auxiliary_match_id(auxdrv->id_table, auxdev));
+	ret = auxiliary_bus_irq_dir_res_probe(auxdev);
+	if  (ret)
+		return ret;
+
+	ret = auxdrv->probe(auxdev,
+			    auxiliary_match_id(auxdrv->id_table, auxdev));
+	if (ret)
+		auxiliary_bus_irq_dir_res_remove(auxdev);
+
+	return ret;
 }
 
 static void auxiliary_bus_remove(struct device *dev)
@@ -235,6 +244,7 @@ static void auxiliary_bus_remove(struct device *dev)
 
 	if (auxdrv->remove)
 		auxdrv->remove(auxdev);
+	auxiliary_bus_irq_dir_res_remove(auxdev);
 }
 
 static void auxiliary_bus_shutdown(struct device *dev)
@@ -294,7 +304,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..8ae3ec62b3db 100644
--- a/drivers/base/auxiliary_sysfs.c
+++ b/drivers/base/auxiliary_sysfs.c
@@ -13,30 +13,71 @@ struct auxiliary_irq_info {
 	char name[AUXILIARY_MAX_IRQ_NAME];
 };
 
+static struct attribute auxiliary_irq_attr = {
+	.mode = 0,
+	.name = "DUMMY",
+};
+
 static struct attribute *auxiliary_irq_attrs[] = {
-	NULL
+	[0] = &auxiliary_irq_attr,
+	[1] = NULL,
 };
 
+static bool auxiliary_irq_dir_group_visible(struct kobject *kobj)
+{
+	struct auxiliary_device *auxdev;
+	struct device *dev;
+
+	dev = container_of(kobj, struct device, kobj);
+	auxdev = container_of(dev, struct auxiliary_device, dev);
+
+	return !xa_empty(&auxdev->sysfs.irqs);
+}
+
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(auxiliary_irq_dir);
+
 static const struct attribute_group auxiliary_irqs_group = {
 	.name = "irqs",
 	.attrs = auxiliary_irq_attrs,
+	.is_visible = SYSFS_GROUP_VISIBLE(auxiliary_irq_dir),
 };
 
-static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev)
+void auxiliary_bus_irq_dir_res_remove(struct auxiliary_device *auxdev)
 {
-	int ret = 0;
+	struct device *dev = &auxdev->dev;
 
-	guard(mutex)(&auxdev->sysfs.lock);
-	if (auxdev->sysfs.irq_dir_exists)
-		return 0;
+	sysfs_remove_group(&dev->kobj, &auxiliary_irqs_group);
+	xa_destroy(&auxdev->sysfs.irqs);
+	mutex_destroy(&auxdev->sysfs.lock);
+}
 
-	ret = devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group);
-	if (ret)
-		return ret;
+int auxiliary_bus_irq_dir_res_probe(struct auxiliary_device *auxdev)
+{
+	struct device *dev = &auxdev->dev;
 
-	auxdev->sysfs.irq_dir_exists = true;
+	mutex_init(&auxdev->sysfs.lock);
 	xa_init(&auxdev->sysfs.irqs);
-	return 0;
+	return sysfs_create_group(&dev->kobj, &auxiliary_irqs_group);
+}
+
+static struct auxiliary_irq_info *auxiliary_irq_info_init(int irq)
+{
+	struct auxiliary_irq_info *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return NULL;
+
+	sysfs_attr_init(&info->sysfs_attr.attr);
+	snprintf(info->name, AUXILIARY_MAX_IRQ_NAME, "%d", irq);
+	info->sysfs_attr.attr.name = info->name;
+
+	return info;
+}
+
+static void auxiliary_irq_info_destroy(struct auxiliary_irq_info *info)
+{
+	kfree(info);
 }
 
 /**
@@ -55,36 +96,41 @@ static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev)
  */
 int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
 {
-	struct auxiliary_irq_info *info __free(kfree) = NULL;
 	struct device *dev = &auxdev->dev;
+	struct auxiliary_irq_info *info;
+	bool sysfs_add_error = false;
 	int ret;
 
-	ret = auxiliary_irq_dir_prepare(auxdev);
-	if (ret)
-		return ret;
-
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	info = auxiliary_irq_info_init(irq);
 	if (!info)
 		return -ENOMEM;
 
-	sysfs_attr_init(&info->sysfs_attr.attr);
-	snprintf(info->name, AUXILIARY_MAX_IRQ_NAME, "%d", irq);
-
+	mutex_lock(&auxdev->sysfs.lock);
 	ret = xa_insert(&auxdev->sysfs.irqs, irq, info, GFP_KERNEL);
 	if (ret)
-		return ret;
+		goto unlock;
+
+	ret = sysfs_update_group(&dev->kobj, &auxiliary_irqs_group);
+	if (ret)
+		goto irq_erase;
 
-	info->sysfs_attr.attr.name = info->name;
 	ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
 				      auxiliary_irqs_group.name);
-	if (ret)
-		goto sysfs_add_err;
+	if (ret) {
+		sysfs_add_error = true;
+		goto irq_erase;
+	}
 
-	xa_store(&auxdev->sysfs.irqs, irq, no_free_ptr(info), GFP_KERNEL);
+	mutex_unlock(&auxdev->sysfs.lock);
 	return 0;
 
-sysfs_add_err:
+irq_erase:
 	xa_erase(&auxdev->sysfs.irqs, irq);
+	if (sysfs_add_error)
+		sysfs_update_group(&dev->kobj, &auxiliary_irqs_group);
+unlock:
+	mutex_unlock(&auxdev->sysfs.lock);
+	auxiliary_irq_info_destroy(info);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_add);
@@ -97,17 +143,30 @@ EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_add);
  * This function should be called to remove an IRQ sysfs entry.
  * The driver must invoke this API when IRQ is released by the device.
  */
-void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
+int auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
 {
-	struct auxiliary_irq_info *info __free(kfree) = xa_load(&auxdev->sysfs.irqs, irq);
 	struct device *dev = &auxdev->dev;
+	struct auxiliary_irq_info *info;
+	int err;
 
+	mutex_lock(&auxdev->sysfs.lock);
+	info = xa_load(&auxdev->sysfs.irqs, irq);
 	if (!info) {
+		mutex_unlock(&auxdev->sysfs.lock);
 		dev_err(&auxdev->dev, "IRQ %d doesn't exist\n", irq);
-		return;
+		return -ENOMEM;
 	}
+
 	sysfs_remove_file_from_group(&dev->kobj, &info->sysfs_attr.attr,
 				     auxiliary_irqs_group.name);
 	xa_erase(&auxdev->sysfs.irqs, irq);
+	err = sysfs_update_group(&dev->kobj, &auxiliary_irqs_group);
+	if (err)
+		dev_err(&auxdev->dev,
+			"Failed to update IRQs group, irq %d\n", irq);
+
+	mutex_unlock(&auxdev->sysfs.lock);
+	auxiliary_irq_info_destroy(info);
+	return err;
 }
 EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_remove);
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index 4086afd0cc6b..06c247b647ab 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -61,7 +61,6 @@
  * @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
@@ -146,7 +145,6 @@ struct auxiliary_device {
 	struct {
 		struct xarray irqs;
 		struct mutex lock; /* Synchronize irq sysfs creation */
-		bool irq_dir_exists;
 	} sysfs;
 };
 
@@ -222,23 +220,37 @@ 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
+void auxiliary_bus_irq_dir_res_remove(struct auxiliary_device *auxdev);
+int auxiliary_bus_irq_dir_res_probe(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);
+int auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq);
 #else /* CONFIG_SYSFS */
+static inline void
+auxiliary_bus_irq_dir_res_remove(struct auxiliary_device *auxdev)
+{
+}
+
+static inline int
+auxiliary_bus_irq_dir_res_probe(struct auxiliary_device *auxdev)
+{
+	return 0;
+}
+
 static inline int
 auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
 {
 	return 0;
 }
 
-static inline void
-auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {}
+static inline int
+auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
+{
+	return 0;
+}
 #endif
 
 static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
 {
-	mutex_destroy(&auxdev->sysfs.lock);
 	put_device(&auxdev->dev);
 }
 

base-commit: c0178eec8884231a5ae0592b9fce827bccb77e86
-- 
2.31.1