Re: [PATCH v4] PCI: hotplug: Add 'uevent' sysfs attribute to trigger slot events

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Leon, driver-core]

(Ramesh, when you post new versions of a series, please cc anybody who
responded to earlier versions.  Also, v2, v3, and v4 are identical, so
there's no need to post them as new "versions"; you can just ping the
original thread or label them as "RESEND")

On Wed, May 20, 2026 at 05:13:20PM -0500, Ramesh Errabolu wrote:
> Add a write-only 'uevent' sysfs attribute for synthesizing
> uevents for a PCI slot. This extends the existing uevent
> support which emits a KOBJ_ADD uevent in pci_hp_add() with
> the ability to replay such uevents for cold plugged devices.
> As such events are only emitted by hotplug capable PCI slots
> so is the support for synthesizing them.
> 
> The change was validated by manually triggering 'add' uevent
> for a specific hotplug PCI slot:
> 
>     $ echo "add $(uuidgen)" | sudo tee   \
>                 /sys/bus/pci/slots/<slot-id>/uevent
> 
> Signed-off-by: Ramesh Errabolu <ramesh@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/pci/hotplug/pci_hotplug_core.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
> index fadcf98a8a66..c3634b1cc7a8 100644
> --- a/drivers/pci/hotplug/pci_hotplug_core.c
> +++ b/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -173,12 +173,27 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
>  
>  static struct pci_slot_attribute hotplug_slot_attr_presence = {
>  	.attr = {.name = "adapter", .mode = S_IFREG | S_IRUGO},
>  	.show = presence_read_file,
>  };
>  
> +static ssize_t uevent_write_file(struct pci_slot *slot,
> +				 const char *buf, size_t len)
> +{
> +	int rc;
> +
> +	rc = kobject_synth_uevent(&slot->kobj, buf, len);

I haven't followed the discussion closely, but I'm skeptical because
this would be the only use of kobject_synth_uevent() outside the
driver core.  That means a change like this should include a
description of something unique about this PCI slot situation that is
different from all other buses.

For driver-core, the preceding discussion is here:
https://lore.kernel.org/linux-pci/20260225150815.81268-1-ramesh@linux.ibm.com/t/#m57bf51ce1c073b685b391867d4a9932e5f9dccc9

> +	return rc ? rc : len;
> +}
> +
> +static struct pci_slot_attribute hotplug_slot_attr_uevent = {
> +	.attr = {.name = "uevent", .mode = S_IFREG | 0200},
> +	.show = NULL,

Unnecessary to set ".show = NULL" since that's the default.

> +	.store = uevent_write_file
> +};
> +
>  static ssize_t test_write_file(struct pci_slot *pci_slot, const char *buf,
>  			       size_t count)
>  {
>  	struct hotplug_slot *slot = pci_slot->hotplug;
>  	unsigned long ltest;
>  	u32 test;
> @@ -251,12 +266,17 @@ static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
>  		if (retval)
>  			dev_err(&pci_slot->bus->dev,
>  				"Error creating sysfs link (%d)\n", retval);
>  		kobject_put(kobj);
>  	}
>  
> +	retval = sysfs_create_file(&pci_slot->kobj,
> +				   &hotplug_slot_attr_uevent.attr);

Krzysztof, any sysfs race issues here?  I assume this (and the
existing uses for power, attention, latch, etc) are not candidates for
static attributes?

> +	if (retval)
> +		goto exit_uevent;
> +
>  	if (has_power_file(slot)) {
>  		retval = sysfs_create_file(&pci_slot->kobj,
>  					   &hotplug_slot_attr_power.attr);
>  		if (retval)
>  			goto exit_power;
>  	}
> @@ -303,19 +323,24 @@ static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
>  		sysfs_remove_file(&pci_slot->kobj,
>  				  &hotplug_slot_attr_attention.attr);
>  exit_attention:
>  	if (has_power_file(slot))
>  		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
>  exit_power:
> +	sysfs_remove_file(&pci_slot->kobj,
> +			  &hotplug_slot_attr_uevent.attr);
> +exit_uevent:
>  	sysfs_remove_link(&pci_slot->kobj, "module");
>  exit:
>  	return retval;
>  }
>  
>  static void fs_remove_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
>  {
> +	sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_uevent.attr);
> +
>  	if (has_power_file(slot))
>  		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
>  
>  	if (has_attention_file(slot))
>  		sysfs_remove_file(&pci_slot->kobj,
>  				  &hotplug_slot_attr_attention.attr);
> -- 
> 2.43.0
>