On Fri, Oct 16, 2026 at 05:42:44PM -0400, Gregory Price (Meta) wrote:
> From: Gregory Price <[email protected]>
>
> When the kernel provides daxX.Y/state, online a fully offline device
> by writing the online policy to it rather than onlining each block.
> This avoids racing other agents (udev, auto-online) that online blocks
> as they appear. The kernel only onlines from 'unplugged', so a device
> in 'offline' is unplugged first; if that fails (e.g. no memory
> hotremove support), the device is unchanged and is onlined per-block.
>
> Memory onlined by another agent is left alone: the 'offline' case
> re-checks that no block is online before unplugging, and losing a
> racing online falls back to the per-block path. A small window remains
> between that check and the unplug.
>
> The per-block path now fails when the device has no memory blocks,
> rather than reporting zero sections already online.
>
> Signed-off-by: Gregory Price <[email protected]>
> ---
> daxctl/device.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/daxctl/device.c b/daxctl/device.c
> index 47942f1..fd84799 100644
> --- a/daxctl/device.c
> +++ b/daxctl/device.c
> @@ -510,10 +510,99 @@ static const char *parse_device_options(int argc, const
> char **argv,
> return device;
> }
>
> +static int dev_online_memory_atomic(struct daxctl_dev *dev,
> + struct daxctl_memory *mem, enum daxctl_kmem_state state)
> +{
> + const char *devname = daxctl_dev_get_devname(dev);
> + enum daxctl_kmem_state target;
> + int num_sections, num_on, rc;
> +
> + if (param.no_movable)
> + target = DAXCTL_KMEM_ONLINE;
> + else
> + target = DAXCTL_KMEM_ONLINE_MOVABLE;
> +
Hi Greg,
A couple of things here...
The atomic --no-movable path uses DAXCTL_KMEM_ONLINE, leaving zone selection
to the kernel. With memory_hotplug.online_policy=auto-movable, IIUC this could
place memory in ZONE_MOVABLE despite --no-movable.
The per-block path checks the resulting zone and fails if it isn't ZONE_NORMAL,
but the atomic path skips that check.
Should this use DAXCTL_KMEM_ONLINE_KERNEL instead, or retain DAXCTL_KMEM_ONLINE
and add the same zone check?
> + if (state == DAXCTL_KMEM_OFFLINE) {
> + /*
> + * Onlining requires 'unplugged'. Don't unplug memory another
> + * agent onlined meanwhile (a small window remains).
> + */
With this, a "reconfigure-device -m system-ram" may add the memory twice.
First kmem adds it offline at probe and then daxctl unplugs it and adds it
back online. If that re-add fails, the device is left unplugged rather than
offline.
I get that falling back to per-block onlining would reopen the race this
series is addressing. Could the kernel interface support transitioning
directly from offline to an online state instead? It seems that would avoid
both the extra hotplug work and the left-unplugged failure case.
-- Alison
> + num_on = daxctl_memory_is_online(mem);
> + if (num_on < 0) {
> + fprintf(stderr, "%s: failed to determine online state:
> %s\n",
> + devname, strerror(-num_on));
> + return num_on;
> + }
> + if (num_on > 0)
> + return -EAGAIN;
> + rc = daxctl_dev_set_kmem_state(dev, DAXCTL_KMEM_UNPLUGGED);
> + if (rc < 0) {
> + /* unchanged (e.g. no hotremove support); go per-block
> */
> + if (param.verbose)
> + fprintf(stderr, "%s: unplug failed: %s\n",
> + devname, strerror(-rc));
> + return -EAGAIN;
> + }
> + }
> +
> + rc = daxctl_dev_set_kmem_state(dev, target);
> + if (rc == -EBUSY) {
> + /* back off only if another writer onlined it */
> + switch (daxctl_dev_get_kmem_state(dev)) {
> + case DAXCTL_KMEM_ONLINE:
> + case DAXCTL_KMEM_ONLINE_KERNEL:
> + case DAXCTL_KMEM_ONLINE_MOVABLE:
> + return -EAGAIN;
> + default:
> + break;
> + }
> + }
> + if (rc < 0) {
> + fprintf(stderr, "%s: failed to online memory: %s\n",
> + devname, strerror(-rc));
> + if (state == DAXCTL_KMEM_OFFLINE)
> + fprintf(stderr,
> + "%s: device left unplugged; retry to recover\n",
> + devname);
> + return rc;
> + }
> +
> + /* all sections should now be online */
> + num_sections = daxctl_memory_num_sections(mem);
> + if (num_sections < 0) {
> + fprintf(stderr, "%s: failed to get number of memory sections\n",
> + devname);
> + return num_sections;
> + }
> + if (num_sections == 0) {
> + fprintf(stderr, "%s: no memory sections present after online\n",
> + devname);
> + return -ENXIO;
> + }
> +
> + num_on = daxctl_memory_is_online(mem);
> + if (num_on < 0) {
> + fprintf(stderr, "%s: failed to determine online state: %s\n",
> + devname, strerror(-num_on));
> + return num_on;
> + }
> + if (num_on < num_sections) {
> + fprintf(stderr, "%s: failed to online %d memory sections\n",
> + devname, num_sections - num_on);
> + return -ENXIO;
> + }
> + if (param.verbose)
> + fprintf(stderr, "%s: %d memory section%s onlined\n", devname,
> + num_on, num_on == 1 ? "" : "s");
> +
> + return 0;
> +}
> +
snip
