Re: [RFC PATCH 6/8] driver core: Defer uevents for devices registered in a staged window

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Sep 11, 2026 at 07:43:38PM +0200, Pavol Sakac wrote:
> Staged registration publishes a device's subtree in one step, so nothing
> observes the device before it is complete. That does not hold for a
> device registered from inside the window: device_add() calls hooks that
> can register a child of the device being added, and such a child takes
> the eager path and announces itself with KOBJ_ADD while its own path
> resolves to nothing. Nothing replays that event, so a consumer that
> cannot open the DEVPATH never learns of the child. Neither opt-in in
> this series reaches that case: nothing attaches a wakeup source to a
> PCI VF or to the VFIO class devices, and pci_acpi_setup() enables
> wakeup only for a bridge that can do D3. It is reachable by code the
> opt-in caller does not control: dpm_sysfs_add() registers a
> wakeup-source device whenever power.wakeup is already attached, and
> wakeup_source_register() registers one directly for a device that is
> already registered, which a staged device is. The driver core
> therefore handles the case rather than forbidding it in the opt-in
> contract.
> 
> Defer such a registration, composing the idiom block/genhd.c already uses
> for a disk's partition tree: suppress the uevents, complete the tree,
> then unsuppress and replay KOBJ_ADD. A device whose directory kernfs
> marked staged is enqueued suppressed on the window of the nearest
> ancestor that opted in, and the window is drained as the ancestor's
> device_add() returns, just after its own KOBJ_ADD, each member's ADD
> delivered in registration order followed by BIND where a driver bound
> in the window. An ancestor whose owner suppresses its uevent and
> replays it after registration will see members announced before its
> replayed ADD. The window lives in a global hashtable keyed by the
> opted-in device, so no struct grows.
> 
> Members are expected to be registered synchronously by the opted-in
> device_add() that owns the window, the only shape the driver core can
> reason about here. Raw uevent_suppress is not reused as the detection
> marker, precisely because a subsystem may already own it; a device found
> already suppressed is left out of the window entirely. An in-window
> KOBJ_CHANGE is dropped rather than replayed, as in the genhd case, since
> only the addition can be reconstructed afterwards. A failed opted-in
> registration closes its window without replaying anything and leaves
> suppression set, so a member's KOBJ_REMOVE is dropped too and userspace
> never sees a removal for an addition it never saw.
> 
> The staged_device suite gains five cases for the window: the replay of a
> member's addition once the owner's device_add() returns, the aborted
> window that replays nothing and leaves suppression set, a member behind
> a class glue directory, the reconstructed KOBJ_BIND of a driver bound in
> the window, and a member found already suppressed being left out of the
> window.
> 
> If you would rather not carry this until a caller needs it, it can be
> dropped and the restriction stated as an opt-in condition instead.

...

> +	member->dev = get_device(dev);
> +
> +	spin_lock(&staged_windows_lock);
> +	hash_for_each_possible(staged_windows, win, node, (unsigned long)top) {
> +		if (win->top != top)
> +			continue;
> +		dev_set_uevent_suppress(dev, 1);
> +		list_add_tail(&member->node, &win->members);
> +		member = NULL;
> +		break;
> +	}
> +	spin_unlock(&staged_windows_lock);
> +
> +	if (member) {
> +		put_device(dev);

With this condition it makes sense to use the same argument, id est
member->dev.

> +		kfree(member);
> +	}

...

> +/*
> + * Records the child's replayed uevents as they are dispatched through the
> + * bus uevent callback -- which runs upstream of netlink broadcast, so this
> + * observes action-specific kernel dispatch and ordering, not reception by
> + * any consumer.  Scoped to the active context's child: the same static bus
> + * serves another case, and teardown emits further events.
> + */
> +static int staged_test_bus_uevent(const struct device *dev,
> +				  struct kobj_uevent_env *env)
> +{
> +	struct staged_window_ctx *c = staged_window_ctx;
> +	int i;

Why signed?

> +	if (!c || dev != c->child)
> +		return 0;
> +
> +	for (i = 0; i < env->envp_idx; i++) {


	for (int i = 0; i < env->envp_idx; i++) {

> +		if (!strcmp(env->envp[i], "ACTION=add")) {
> +			c->add_uevents++;
> +			c->add_seq = ++c->uevent_seq;
> +		} else if (!strcmp(env->envp[i], "ACTION=bind")) {
> +			c->bind_uevents++;
> +			c->bind_seq = ++c->uevent_seq;

Why preincrements?

> +		}
> +	}

Missing blank line.

> +	return 0;
> +}

...

> +static struct attribute in_window_attr = {
> +	.name = "in_window_attr", .mode = 0644,
> +};

I believe you haven't read the generated code. Do you understand what it does
(I mean the whole your patch and patch series)?

...

> +static const struct attribute_group *in_window_grps[] = {
> +	&in_window_grp, NULL,
> +};

Same style issue.

Also we have __ATRIBUTE_GROUPS() macro.

-- 
With Best Regards,
Andy Shevchenko