Re: [NDCTL PATCH v2 2/2] cxl: Add check for regions before disabling memdev

["Cao, Quanquan/曹 全全" <caoqq@fujitsu.com>]

> Add a check for memdev disable to see if there are active regions present
> before disabling the device. This is necessary now regions are present to
> fulfill the TODO that was left there. The best way to determine if a
> region is active is to see if there are decoders enabled for the mem
> device. This is also best effort as the state is only a snapshot the
> kernel provides and is not atomic WRT the memdev disable operation. The
> expectation is the admin issuing the command has full control of the mem
> device and there are no other agents also attempt to control the device.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v2:
> - Warn if active region regardless of -f. (Alison)
> - Expound on -f behavior in man page. (Vishal)
> ---
>   Documentation/cxl/cxl-disable-memdev.txt |    4 +++-
>   cxl/memdev.c                             |   17 ++++++++++++++---
>   2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/cxl/cxl-disable-memdev.txt b/Documentation/cxl/cxl-disable-memdev.txt
> index c4edb93ee94a..34b720288705 100644
> --- a/Documentation/cxl/cxl-disable-memdev.txt
> +++ b/Documentation/cxl/cxl-disable-memdev.txt
> @@ -27,7 +27,9 @@ include::bus-option.txt[]
>   	a device if the tool determines the memdev is in active usage. Recall
>   	that CXL memory ranges might have been established by platform
>   	firmware and disabling an active device is akin to force removing
> -	memory from a running system.
> +	memory from a running system. Going down this path does not offline
> +	active memory if they are currently online. User is recommended to
> +	offline and disable the appropriate regions before disabling the memdevs.
>   
>   -v::
>   	Turn on verbose debug messages in the library (if libcxl was built with
> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index 2dd2e7fcc4dd..1d3121915284 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -437,14 +437,25 @@ static int action_free_dpa(struct cxl_memdev *memdev,
>   
>   static int action_disable(struct cxl_memdev *memdev, struct action_context *actx)
>   {
> +	struct cxl_endpoint *ep;
> +	struct cxl_port *port;
> +
>   	if (!cxl_memdev_is_enabled(memdev))
>   		return 0;
>   
> -	if (!param.force) {
> -		/* TODO: actually detect rather than assume active */
> +	ep = cxl_memdev_get_endpoint(memdev);
> +	if (!ep)
> +		return -ENODEV;
> +
> +	port = cxl_endpoint_get_port(ep);
> +	if (!port)
> +		return -ENODEV;
> +
> +	if (cxl_port_decoders_committed(port)) {
>   		log_err(&ml, "%s is part of an active region\n",
>   			cxl_memdev_get_devname(memdev));
> -		return -EBUSY;
> +		if (!param.force)
> +			return -EBUSY;
>   	}
>   
>   	return cxl_memdev_disable_invalidate(memdev);
> 
> 

Reviewed-by: Quanquan Cao <caoqq@fujitsu.com>