Re: [PATCH v2 06/13] drivers/base/memory: Add cxl_offline_memory() to offline a physical range

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl_offline_memory() leaves memory blocks partially offlined and permanently stranded if device_offline() fails midway, lacking a state-tracking and rollback mechanism.
--

--- Patch [6]: [PATCH v2 06/13] drivers/base/memory: Add cxl_offline_memory() to offline a physical range ---
commit 00648cd16fe5b1311f9ee6c83c16afccf56f6b3d
Author: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

drivers/base/memory: Add cxl_offline_memory() to offline a physical range

A CXL region whose capacity is online as System RAM must be taken
offline before a Secondary Bus Reset of the CXL Downstream Port it is
reached through.

> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index bcfe2d9f4adbd..ce89b251d34ac 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c

[ ... ]

> @@ -1018,6 +1018,42 @@ int walk_memory_blocks(unsigned long start, unsigned long size,
>  	return ret;
>  }
>  
> +static int cxl_offline_memory_block(struct memory_block *mem, void *arg)
> +{
> +	int rc = device_offline(&mem->dev);
> +
> +	/* device_offline() returns a positive value when already offline. */
> +	if (rc > 0)
> +		return 0;
> +
> +	return rc;
> +}
> +
> +/**
> + * cxl_offline_memory - offline the memory blocks spanning a physical range

[ ... ]

> +int cxl_offline_memory(u64 start, u64 size)
> +{
> +	int rc;
> +
> +	lock_device_hotplug();
> +	rc = walk_memory_blocks(start, size, NULL, cxl_offline_memory_block);

[Severity: High]
If device_offline() fails midway through a range (for example, due to
unmovable or pinned pages), will this leave the system in an inconsistent
state with partially offlined memory ranges and silent capacity loss?

When walk_memory_blocks() aborts on an error from cxl_offline_memory_block(),
there doesn't appear to be a rollback mechanism for blocks that were already
successfully offlined.

Additionally, since cxl_offline_memory_block() treats an already-offline
block as a success but doesn't record that state, how could a caller
safely roll back without accidentally onlining a block that was originally
offline before cxl_offline_memory() was called? Core memory hotplug APIs
typically allocate an array to track original states to handle these failures.

> +	unlock_device_hotplug();
> +
> +	return rc;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825022628.3651434-1-fabio.m.de.francesco@linux.intel.com?part=6