public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] PCI hotplug cleanups
@ 2025-02-25 17:06 Lukas Wunner
  2025-02-25 17:06 ` [PATCH 1/5] PCI: hotplug: Drop superfluous pci_hotplug_slot_list Lukas Wunner
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Lukas Wunner @ 2025-02-25 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

Here's a collection of cleanups for the PCI hotplug core.
None of them should result in a behavioral change.
Just removal and untangling of ancient, unnecessary code.

Lukas Wunner (5):
  PCI: hotplug: Drop superfluous pci_hotplug_slot_list
  PCI: hotplug: Drop superfluous try_module_get() calls
  PCI: hotplug: Drop superfluous NULL pointer checks in has_*_file()
  PCI: hotplug: Avoid backpointer dereferencing in has_*_file()
  PCI: hotplug: Inline pci_hp_{create,remove}_module_link()

 drivers/pci/hotplug/pci_hotplug_core.c | 142 +++++++------------------
 drivers/pci/slot.c                     |  44 --------
 include/linux/pci.h                    |   5 -
 include/linux/pci_hotplug.h            |   2 -
 4 files changed, 41 insertions(+), 152 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] PCI: hotplug: Drop superfluous pci_hotplug_slot_list
  2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
@ 2025-02-25 17:06 ` Lukas Wunner
  2025-02-25 17:06 ` [PATCH 2/5] PCI: hotplug: Drop superfluous try_module_get() calls Lukas Wunner
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Lukas Wunner @ 2025-02-25 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

The PCI hotplug core keeps a list of all registered slots.  Its sole
purpose is to WARN() on slot removal if another slot is using the same
name.

But this can never happen because already on slot creation, an error is
returned and multiple messages are emitted if a slot's name is
duplicated:

  pci_hp_register()
    __pci_hp_register()
      __pci_hp_initialize()
        pci_create_slot()
          kobject_init_and_add()
            kobject_add_varg()
              kobject_add_internal()
                create_dir()
                  sysfs_create_dir_ns()
                    kernfs_create_dir_ns()
                      sysfs_warn_dup()
                        pr_warn("cannot create duplicate filename ...")
                pr_err("%s failed for %s with -EEXIST, ...");

Drop the superfluous list.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 32 --------------------------
 include/linux/pci_hotplug.h            |  2 --
 2 files changed, 34 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 36236ac88fd5..9e3cde91c167 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -18,14 +18,12 @@
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
-#include <linux/list.h>
 #include <linux/kobject.h>
 #include <linux/sysfs.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
 #include <linux/mount.h>
 #include <linux/namei.h>
-#include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
 #include <linux/uaccess.h>
@@ -42,9 +40,6 @@
 /* local variables */
 static bool debug;
 
-static LIST_HEAD(pci_hotplug_slot_list);
-static DEFINE_MUTEX(pci_hp_mutex);
-
 /* Weee, fun with macros... */
 #define GET_STATUS(name, type)	\
 static int get_##name(struct hotplug_slot *slot, type *value)		\
@@ -375,17 +370,6 @@ static void fs_remove_slot(struct pci_slot *pci_slot)
 	pci_hp_remove_module_link(pci_slot);
 }
 
-static struct hotplug_slot *get_slot_from_name(const char *name)
-{
-	struct hotplug_slot *slot;
-
-	list_for_each_entry(slot, &pci_hotplug_slot_list, slot_list) {
-		if (strcmp(hotplug_slot_name(slot), name) == 0)
-			return slot;
-	}
-	return NULL;
-}
-
 /**
  * __pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem
  * @slot: pointer to the &struct hotplug_slot to register
@@ -484,10 +468,6 @@ int pci_hp_add(struct hotplug_slot *slot)
 		return result;
 
 	kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
-	mutex_lock(&pci_hp_mutex);
-	list_add(&slot->slot_list, &pci_hotplug_slot_list);
-	mutex_unlock(&pci_hp_mutex);
-	dbg("Added slot %s to the list\n", hotplug_slot_name(slot));
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_hp_add);
@@ -514,21 +494,9 @@ EXPORT_SYMBOL_GPL(pci_hp_deregister);
  */
 void pci_hp_del(struct hotplug_slot *slot)
 {
-	struct hotplug_slot *temp;
-
 	if (WARN_ON(!slot))
 		return;
 
-	mutex_lock(&pci_hp_mutex);
-	temp = get_slot_from_name(hotplug_slot_name(slot));
-	if (WARN_ON(temp != slot)) {
-		mutex_unlock(&pci_hp_mutex);
-		return;
-	}
-
-	list_del(&slot->slot_list);
-	mutex_unlock(&pci_hp_mutex);
-	dbg("Removed slot %s from the list\n", hotplug_slot_name(slot));
 	fs_remove_slot(slot->pci_slot);
 }
 EXPORT_SYMBOL_GPL(pci_hp_del);
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index 3a10d6ec3ee7..ec77ccf1fc4d 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -50,7 +50,6 @@ struct hotplug_slot_ops {
 /**
  * struct hotplug_slot - used to register a physical slot with the hotplug pci core
  * @ops: pointer to the &struct hotplug_slot_ops to be used for this slot
- * @slot_list: internal list used to track hotplug PCI slots
  * @pci_slot: represents a physical slot
  * @owner: The module owner of this structure
  * @mod_name: The module name (KBUILD_MODNAME) of this structure
@@ -59,7 +58,6 @@ struct hotplug_slot {
 	const struct hotplug_slot_ops	*ops;
 
 	/* Variables below this are for use only by the hotplug pci core. */
-	struct list_head		slot_list;
 	struct pci_slot			*pci_slot;
 	struct module			*owner;
 	const char			*mod_name;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] PCI: hotplug: Drop superfluous try_module_get() calls
  2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
  2025-02-25 17:06 ` [PATCH 1/5] PCI: hotplug: Drop superfluous pci_hotplug_slot_list Lukas Wunner
@ 2025-02-25 17:06 ` Lukas Wunner
  2025-02-25 17:06 ` [PATCH 3/5] PCI: hotplug: Drop superfluous NULL pointer checks in has_*_file() Lukas Wunner
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Lukas Wunner @ 2025-02-25 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

In December 2002, historic commit

  https://git.kernel.org/tglx/history/c/bec7aa00ffe5
  ("[PATCH] more module warning fixes")

amended the PCI hotplug core to acquire a reference on the hotplug
driver module when a sysfs attribute is accessed.  That was necessary
because back in the day, sysfs code did not take any precautions to
prevent module unloading when an attribute was accessed.

Soon after in July 2003, historic commit

  https://git.kernel.org/tglx/history/c/1cf6d20f6078
  ("[PATCH] SYSFS: add module referencing to sysfs attribute files.")

addressed that deficiency.  But the commit neglected to remove the now
unnecessary reference acquisition from the PCI hotplug core.

The commit acquired a module reference for the entire duration between
open() and close() of a sysfs attribute.  This made it impossible to
unload a module while attributes were kept open by user space.
That's possible today:

When a hotplug driver module is unloaded, it removes sysfs attributes of
all its hotplug slots by calling pci_hp_del().  This will wait for any
concurrent user space operation to finish:

  pci_hp_del()
    fs_remove_slot()
      sysfs_remove_file()
        sysfs_remove_file_ns()
          kernfs_remove_by_name_ns()
            __kernfs_remove()
              kernfs_drain()

A user space operation such as read() briefly acquires a reference on
the attribute with kernfs_get_active().  kernfs_drain() waits until all
such references are released before allowing attribute removal.  Once
the attribute is removed, any subsequent user space operation on a still
open attribute file will return -ENODEV.

Thus, reference acquisition by the PCI hotplug core is still unnecessary
today.  So drop it at long last.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 9e3cde91c167..de5b501b40be 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -14,7 +14,7 @@
  *   Scott Murray <scottm@somanetworks.com>
  */
 
-#include <linux/module.h>	/* try_module_get & module_put */
+#include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
@@ -46,11 +46,8 @@ static int get_##name(struct hotplug_slot *slot, type *value)		\
 {									\
 	const struct hotplug_slot_ops *ops = slot->ops;			\
 	int retval = 0;							\
-	if (!try_module_get(slot->owner))				\
-		return -ENODEV;						\
 	if (ops->get_##name)						\
 		retval = ops->get_##name(slot, value);			\
-	module_put(slot->owner);					\
 	return retval;							\
 }
 
@@ -83,10 +80,6 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
 	power = (u8)(lpower & 0xff);
 	dbg("power = %d\n", power);
 
-	if (!try_module_get(slot->owner)) {
-		retval = -ENODEV;
-		goto exit;
-	}
 	switch (power) {
 	case 0:
 		if (slot->ops->disable_slot)
@@ -102,9 +95,7 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
 		err("Illegal value specified for power\n");
 		retval = -EINVAL;
 	}
-	module_put(slot->owner);
 
-exit:
 	if (retval)
 		return retval;
 	return count;
@@ -141,15 +132,9 @@ static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
 	attention = (u8)(lattention & 0xff);
 	dbg(" - attention = %d\n", attention);
 
-	if (!try_module_get(slot->owner)) {
-		retval = -ENODEV;
-		goto exit;
-	}
 	if (ops->set_attention_status)
 		retval = ops->set_attention_status(slot, attention);
-	module_put(slot->owner);
 
-exit:
 	if (retval)
 		return retval;
 	return count;
@@ -207,15 +192,9 @@ static ssize_t test_write_file(struct pci_slot *pci_slot, const char *buf,
 	test = (u32)(ltest & 0xffffffff);
 	dbg("test = %d\n", test);
 
-	if (!try_module_get(slot->owner)) {
-		retval = -ENODEV;
-		goto exit;
-	}
 	if (slot->ops->hardware_test)
 		retval = slot->ops->hardware_test(slot, test);
-	module_put(slot->owner);
 
-exit:
 	if (retval)
 		return retval;
 	return count;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] PCI: hotplug: Drop superfluous NULL pointer checks in has_*_file()
  2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
  2025-02-25 17:06 ` [PATCH 1/5] PCI: hotplug: Drop superfluous pci_hotplug_slot_list Lukas Wunner
  2025-02-25 17:06 ` [PATCH 2/5] PCI: hotplug: Drop superfluous try_module_get() calls Lukas Wunner
@ 2025-02-25 17:06 ` Lukas Wunner
  2025-02-25 17:06 ` [PATCH 4/5] PCI: hotplug: Avoid backpointer dereferencing " Lukas Wunner
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Lukas Wunner @ 2025-02-25 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

The PCI hotplug core contains five has_*_file() functions to determine
whether a certain sysfs file shall be added (or removed) for a given
hotplug slot.

The functions perform NULL pointer checks for the hotplug_slot and its
hotplug_slot_ops.  However the callers already perform these checks:

  pci_hp_register()
    __pci_hp_register()
      __pci_hp_initialize()

  pci_hp_deregister()
    pci_hp_del()

The only way to actually trigger these checks is to call pci_hp_add()
without having called pci_hp_initialize().

Amend pci_hp_add() to catch that and drop the now superfluous NULL
pointer checks in has_*_file().

Drop the same superfluous checks from pci_hp_create_module_link(),
which is (only) called from pci_hp_add().

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 17 ++++++-----------
 drivers/pci/slot.c                     |  2 --
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index de5b501b40be..d4c12451570b 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -209,8 +209,6 @@ static bool has_power_file(struct pci_slot *pci_slot)
 {
 	struct hotplug_slot *slot = pci_slot->hotplug;
 
-	if ((!slot) || (!slot->ops))
-		return false;
 	if ((slot->ops->enable_slot) ||
 	    (slot->ops->disable_slot) ||
 	    (slot->ops->get_power_status))
@@ -222,8 +220,6 @@ static bool has_attention_file(struct pci_slot *pci_slot)
 {
 	struct hotplug_slot *slot = pci_slot->hotplug;
 
-	if ((!slot) || (!slot->ops))
-		return false;
 	if ((slot->ops->set_attention_status) ||
 	    (slot->ops->get_attention_status))
 		return true;
@@ -234,8 +230,6 @@ static bool has_latch_file(struct pci_slot *pci_slot)
 {
 	struct hotplug_slot *slot = pci_slot->hotplug;
 
-	if ((!slot) || (!slot->ops))
-		return false;
 	if (slot->ops->get_latch_status)
 		return true;
 	return false;
@@ -245,8 +239,6 @@ static bool has_adapter_file(struct pci_slot *pci_slot)
 {
 	struct hotplug_slot *slot = pci_slot->hotplug;
 
-	if ((!slot) || (!slot->ops))
-		return false;
 	if (slot->ops->get_adapter_status)
 		return true;
 	return false;
@@ -256,8 +248,6 @@ static bool has_test_file(struct pci_slot *pci_slot)
 {
 	struct hotplug_slot *slot = pci_slot->hotplug;
 
-	if ((!slot) || (!slot->ops))
-		return false;
 	if (slot->ops->hardware_test)
 		return true;
 	return false;
@@ -439,9 +429,14 @@ EXPORT_SYMBOL_GPL(__pci_hp_initialize);
  */
 int pci_hp_add(struct hotplug_slot *slot)
 {
-	struct pci_slot *pci_slot = slot->pci_slot;
+	struct pci_slot *pci_slot;
 	int result;
 
+	if (WARN_ON(!slot))
+		return -EINVAL;
+
+	pci_slot = slot->pci_slot;
+
 	result = fs_add_slot(pci_slot);
 	if (result)
 		return result;
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index 36b44be0489d..dd6e80b7db09 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -340,8 +340,6 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot)
 	struct kobject *kobj = NULL;
 	int ret;
 
-	if (!slot || !slot->ops)
-		return;
 	kobj = kset_find_obj(module_kset, slot->mod_name);
 	if (!kobj)
 		return;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] PCI: hotplug: Avoid backpointer dereferencing in has_*_file()
  2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
                   ` (2 preceding siblings ...)
  2025-02-25 17:06 ` [PATCH 3/5] PCI: hotplug: Drop superfluous NULL pointer checks in has_*_file() Lukas Wunner
@ 2025-02-25 17:06 ` Lukas Wunner
  2025-02-25 17:06 ` [PATCH 5/5] PCI: hotplug: Inline pci_hp_{create,remove}_module_link() Lukas Wunner
  2025-03-04 23:02 ` [PATCH 0/5] PCI hotplug cleanups Bjorn Helgaas
  5 siblings, 0 replies; 7+ messages in thread
From: Lukas Wunner @ 2025-02-25 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

The PCI hotplug core contains five has_*_file() functions to determine
whether a certain sysfs file shall be added (or removed) for a given
hotplug slot.

The functions receive a struct pci_slot pointer which they have to
dereference back to a struct hotplug_slot.

Avoid by passing them a struct hotplug_slot pointer directly.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 56 +++++++++++---------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index d4c12451570b..a992bf51af98 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -205,10 +205,8 @@ static struct pci_slot_attribute hotplug_slot_attr_test = {
 	.store = test_write_file
 };
 
-static bool has_power_file(struct pci_slot *pci_slot)
+static bool has_power_file(struct hotplug_slot *slot)
 {
-	struct hotplug_slot *slot = pci_slot->hotplug;
-
 	if ((slot->ops->enable_slot) ||
 	    (slot->ops->disable_slot) ||
 	    (slot->ops->get_power_status))
@@ -216,79 +214,71 @@ static bool has_power_file(struct pci_slot *pci_slot)
 	return false;
 }
 
-static bool has_attention_file(struct pci_slot *pci_slot)
+static bool has_attention_file(struct hotplug_slot *slot)
 {
-	struct hotplug_slot *slot = pci_slot->hotplug;
-
 	if ((slot->ops->set_attention_status) ||
 	    (slot->ops->get_attention_status))
 		return true;
 	return false;
 }
 
-static bool has_latch_file(struct pci_slot *pci_slot)
+static bool has_latch_file(struct hotplug_slot *slot)
 {
-	struct hotplug_slot *slot = pci_slot->hotplug;
-
 	if (slot->ops->get_latch_status)
 		return true;
 	return false;
 }
 
-static bool has_adapter_file(struct pci_slot *pci_slot)
+static bool has_adapter_file(struct hotplug_slot *slot)
 {
-	struct hotplug_slot *slot = pci_slot->hotplug;
-
 	if (slot->ops->get_adapter_status)
 		return true;
 	return false;
 }
 
-static bool has_test_file(struct pci_slot *pci_slot)
+static bool has_test_file(struct hotplug_slot *slot)
 {
-	struct hotplug_slot *slot = pci_slot->hotplug;
-
 	if (slot->ops->hardware_test)
 		return true;
 	return false;
 }
 
-static int fs_add_slot(struct pci_slot *pci_slot)
+static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 {
 	int retval = 0;
 
 	/* Create symbolic link to the hotplug driver module */
 	pci_hp_create_module_link(pci_slot);
 
-	if (has_power_file(pci_slot)) {
+	if (has_power_file(slot)) {
 		retval = sysfs_create_file(&pci_slot->kobj,
 					   &hotplug_slot_attr_power.attr);
 		if (retval)
 			goto exit_power;
 	}
 
-	if (has_attention_file(pci_slot)) {
+	if (has_attention_file(slot)) {
 		retval = sysfs_create_file(&pci_slot->kobj,
 					   &hotplug_slot_attr_attention.attr);
 		if (retval)
 			goto exit_attention;
 	}
 
-	if (has_latch_file(pci_slot)) {
+	if (has_latch_file(slot)) {
 		retval = sysfs_create_file(&pci_slot->kobj,
 					   &hotplug_slot_attr_latch.attr);
 		if (retval)
 			goto exit_latch;
 	}
 
-	if (has_adapter_file(pci_slot)) {
+	if (has_adapter_file(slot)) {
 		retval = sysfs_create_file(&pci_slot->kobj,
 					   &hotplug_slot_attr_presence.attr);
 		if (retval)
 			goto exit_adapter;
 	}
 
-	if (has_test_file(pci_slot)) {
+	if (has_test_file(slot)) {
 		retval = sysfs_create_file(&pci_slot->kobj,
 					   &hotplug_slot_attr_test.attr);
 		if (retval)
@@ -298,18 +288,18 @@ static int fs_add_slot(struct pci_slot *pci_slot)
 	goto exit;
 
 exit_test:
-	if (has_adapter_file(pci_slot))
+	if (has_adapter_file(slot))
 		sysfs_remove_file(&pci_slot->kobj,
 				  &hotplug_slot_attr_presence.attr);
 exit_adapter:
-	if (has_latch_file(pci_slot))
+	if (has_latch_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_latch.attr);
 exit_latch:
-	if (has_attention_file(pci_slot))
+	if (has_attention_file(slot))
 		sysfs_remove_file(&pci_slot->kobj,
 				  &hotplug_slot_attr_attention.attr);
 exit_attention:
-	if (has_power_file(pci_slot))
+	if (has_power_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
 exit_power:
 	pci_hp_remove_module_link(pci_slot);
@@ -317,23 +307,23 @@ static int fs_add_slot(struct pci_slot *pci_slot)
 	return retval;
 }
 
-static void fs_remove_slot(struct pci_slot *pci_slot)
+static void fs_remove_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 {
-	if (has_power_file(pci_slot))
+	if (has_power_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
 
-	if (has_attention_file(pci_slot))
+	if (has_attention_file(slot))
 		sysfs_remove_file(&pci_slot->kobj,
 				  &hotplug_slot_attr_attention.attr);
 
-	if (has_latch_file(pci_slot))
+	if (has_latch_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_latch.attr);
 
-	if (has_adapter_file(pci_slot))
+	if (has_adapter_file(slot))
 		sysfs_remove_file(&pci_slot->kobj,
 				  &hotplug_slot_attr_presence.attr);
 
-	if (has_test_file(pci_slot))
+	if (has_test_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_test.attr);
 
 	pci_hp_remove_module_link(pci_slot);
@@ -437,7 +427,7 @@ int pci_hp_add(struct hotplug_slot *slot)
 
 	pci_slot = slot->pci_slot;
 
-	result = fs_add_slot(pci_slot);
+	result = fs_add_slot(slot, pci_slot);
 	if (result)
 		return result;
 
@@ -471,7 +461,7 @@ void pci_hp_del(struct hotplug_slot *slot)
 	if (WARN_ON(!slot))
 		return;
 
-	fs_remove_slot(slot->pci_slot);
+	fs_remove_slot(slot, slot->pci_slot);
 }
 EXPORT_SYMBOL_GPL(pci_hp_del);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] PCI: hotplug: Inline pci_hp_{create,remove}_module_link()
  2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
                   ` (3 preceding siblings ...)
  2025-02-25 17:06 ` [PATCH 4/5] PCI: hotplug: Avoid backpointer dereferencing " Lukas Wunner
@ 2025-02-25 17:06 ` Lukas Wunner
  2025-03-04 23:02 ` [PATCH 0/5] PCI hotplug cleanups Bjorn Helgaas
  5 siblings, 0 replies; 7+ messages in thread
From: Lukas Wunner @ 2025-02-25 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

For no apparent reason, the pci_hp_{create,remove}_module_link() helpers
live in slot.c, even though they're only called from two functions in
pci_hotplug_core.c.

Inline the helpers to reduce code size and number of exported symbols.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 14 +++++++--
 drivers/pci/slot.c                     | 42 --------------------------
 include/linux/pci.h                    |  5 ---
 3 files changed, 11 insertions(+), 50 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index a992bf51af98..d30f1316c98e 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -245,10 +245,18 @@ static bool has_test_file(struct hotplug_slot *slot)
 
 static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 {
+	struct kobject *kobj;
 	int retval = 0;
 
 	/* Create symbolic link to the hotplug driver module */
-	pci_hp_create_module_link(pci_slot);
+	kobj = kset_find_obj(module_kset, slot->mod_name);
+	if (kobj) {
+		retval = sysfs_create_link(&pci_slot->kobj, kobj, "module");
+		if (retval)
+			dev_err(&pci_slot->bus->dev,
+				"Error creating sysfs link (%d)\n", retval);
+		kobject_put(kobj);
+	}
 
 	if (has_power_file(slot)) {
 		retval = sysfs_create_file(&pci_slot->kobj,
@@ -302,7 +310,7 @@ static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 	if (has_power_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
 exit_power:
-	pci_hp_remove_module_link(pci_slot);
+	sysfs_remove_link(&pci_slot->kobj, "module");
 exit:
 	return retval;
 }
@@ -326,7 +334,7 @@ static void fs_remove_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
 	if (has_test_file(slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_test.attr);
 
-	pci_hp_remove_module_link(pci_slot);
+	sysfs_remove_link(&pci_slot->kobj, "module");
 }
 
 /**
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index dd6e80b7db09..50fb3eb595fe 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -7,7 +7,6 @@
 
 #include <linux/kobject.h>
 #include <linux/slab.h>
-#include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/err.h>
 #include "pci.h"
@@ -325,47 +324,6 @@ void pci_destroy_slot(struct pci_slot *slot)
 }
 EXPORT_SYMBOL_GPL(pci_destroy_slot);
 
-#if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)
-#include <linux/pci_hotplug.h>
-/**
- * pci_hp_create_module_link - create symbolic link to hotplug driver module
- * @pci_slot: struct pci_slot
- *
- * Helper function for pci_hotplug_core.c to create symbolic link to
- * the hotplug driver module.
- */
-void pci_hp_create_module_link(struct pci_slot *pci_slot)
-{
-	struct hotplug_slot *slot = pci_slot->hotplug;
-	struct kobject *kobj = NULL;
-	int ret;
-
-	kobj = kset_find_obj(module_kset, slot->mod_name);
-	if (!kobj)
-		return;
-	ret = sysfs_create_link(&pci_slot->kobj, kobj, "module");
-	if (ret)
-		dev_err(&pci_slot->bus->dev, "Error creating sysfs link (%d)\n",
-			ret);
-	kobject_put(kobj);
-}
-EXPORT_SYMBOL_GPL(pci_hp_create_module_link);
-
-/**
- * pci_hp_remove_module_link - remove symbolic link to the hotplug driver
- * 	module.
- * @pci_slot: struct pci_slot
- *
- * Helper function for pci_hotplug_core.c to remove symbolic link to
- * the hotplug driver module.
- */
-void pci_hp_remove_module_link(struct pci_slot *pci_slot)
-{
-	sysfs_remove_link(&pci_slot->kobj, "module");
-}
-EXPORT_SYMBOL_GPL(pci_hp_remove_module_link);
-#endif
-
 static int pci_slot_init(void)
 {
 	struct kset *pci_bus_kset;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 47b31ad724fa..a0f5c8fcd9c7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2447,11 +2447,6 @@ static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int res
 static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe) { }
 #endif
 
-#if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)
-void pci_hp_create_module_link(struct pci_slot *pci_slot);
-void pci_hp_remove_module_link(struct pci_slot *pci_slot);
-#endif
-
 /**
  * pci_pcie_cap - get the saved PCIe capability offset
  * @dev: PCI device
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/5] PCI hotplug cleanups
  2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
                   ` (4 preceding siblings ...)
  2025-02-25 17:06 ` [PATCH 5/5] PCI: hotplug: Inline pci_hp_{create,remove}_module_link() Lukas Wunner
@ 2025-03-04 23:02 ` Bjorn Helgaas
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2025-03-04 23:02 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: linux-pci

On Tue, Feb 25, 2025 at 06:06:00PM +0100, Lukas Wunner wrote:
> Here's a collection of cleanups for the PCI hotplug core.
> None of them should result in a behavioral change.
> Just removal and untangling of ancient, unnecessary code.
> 
> Lukas Wunner (5):
>   PCI: hotplug: Drop superfluous pci_hotplug_slot_list
>   PCI: hotplug: Drop superfluous try_module_get() calls
>   PCI: hotplug: Drop superfluous NULL pointer checks in has_*_file()
>   PCI: hotplug: Avoid backpointer dereferencing in has_*_file()
>   PCI: hotplug: Inline pci_hp_{create,remove}_module_link()
> 
>  drivers/pci/hotplug/pci_hotplug_core.c | 142 +++++++------------------
>  drivers/pci/slot.c                     |  44 --------
>  include/linux/pci.h                    |   5 -
>  include/linux/pci_hotplug.h            |   2 -
>  4 files changed, 41 insertions(+), 152 deletions(-)

Applied to pci/hotplug for v6.15, thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-03-04 23:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-25 17:06 [PATCH 0/5] PCI hotplug cleanups Lukas Wunner
2025-02-25 17:06 ` [PATCH 1/5] PCI: hotplug: Drop superfluous pci_hotplug_slot_list Lukas Wunner
2025-02-25 17:06 ` [PATCH 2/5] PCI: hotplug: Drop superfluous try_module_get() calls Lukas Wunner
2025-02-25 17:06 ` [PATCH 3/5] PCI: hotplug: Drop superfluous NULL pointer checks in has_*_file() Lukas Wunner
2025-02-25 17:06 ` [PATCH 4/5] PCI: hotplug: Avoid backpointer dereferencing " Lukas Wunner
2025-02-25 17:06 ` [PATCH 5/5] PCI: hotplug: Inline pci_hp_{create,remove}_module_link() Lukas Wunner
2025-03-04 23:02 ` [PATCH 0/5] PCI hotplug cleanups Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox