Dave,

Forgive me for not having a new thread, I'd ask a possible relevant questions 
about disable-memdev
We noticed that only -f is implemented for disable-memdev, and it left a
"TODO: actually detect rather than assume active" in cxl/memdev.c.

My questions are:
1. Does the *active* here mean the region(the memdev belongs to) is active ?
2. Is the without force method under developing ?

My colleagues(in CC's) are investigating how to gracefully disable-memdev

Thanks
Zhijian

On 21/09/2023 06:57, Dave Jiang wrote:
> The current operation for disable_region does not check if the memory
> covered by a region is online before attempting to disable the cxl region.
> Provide a -f option for the region to force offlining of currently online
> memory before disabling the region. Also add a check to fail the operation
> entirely if the memory is non-movable.
> 
> Signed-off-by: Dave Jiang <[email protected]>
> 
> ---
> v2:
> - Update documentation and help output. (Vishal)
> ---
>   Documentation/cxl/cxl-disable-region.txt |    7 ++++
>   cxl/region.c                             |   49 
> +++++++++++++++++++++++++++++-
>   2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/cxl/cxl-disable-region.txt 
> b/Documentation/cxl/cxl-disable-region.txt
> index 6a39aee6ea69..9b98d4d8745a 100644
> --- a/Documentation/cxl/cxl-disable-region.txt
> +++ b/Documentation/cxl/cxl-disable-region.txt
> @@ -25,6 +25,13 @@ OPTIONS
>   -------
>   include::bus-option.txt[]
>   
> +-f::
> +--force::
> +     Attempt to offline any memory that has been hot-added into the system
> +     via the CXL region before disabling the region. This won't be attempted
> +     if the memory was not added as 'movable', and may still fail even if it
> +     was movable.
> +
>   include::decoder-option.txt[]
>   
>   include::debug-option.txt[]
> diff --git a/cxl/region.c b/cxl/region.c
> index bcd703956207..f8303869727a 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -14,6 +14,7 @@
>   #include <util/parse-options.h>
>   #include <ccan/minmax/minmax.h>
>   #include <ccan/short_types/short_types.h>
> +#include <daxctl/libdaxctl.h>
>   
>   #include "filter.h"
>   #include "json.h"
> @@ -95,6 +96,8 @@ static const struct option enable_options[] = {
>   
>   static const struct option disable_options[] = {
>       BASE_OPTIONS(),
> +     OPT_BOOLEAN('f', "force", &param.force,
> +                 "attempt to offline memory before disabling the region"),
>       OPT_END(),
>   };
>   
> @@ -789,13 +792,57 @@ static int destroy_region(struct cxl_region *region)
>       return cxl_region_delete(region);
>   }
>   
> +static int disable_region(struct cxl_region *region)
> +{
> +     const char *devname = cxl_region_get_devname(region);
> +     struct daxctl_region *dax_region;
> +     struct daxctl_memory *mem;
> +     struct daxctl_dev *dev;
> +     int rc;
> +
> +     dax_region = cxl_region_get_daxctl_region(region);
> +     if (!dax_region)
> +             goto out;
> +
> +     daxctl_dev_foreach(dax_region, dev) {
> +             mem = daxctl_dev_get_memory(dev);
> +             if (!mem)
> +                     return -ENXIO;
> +
> +             if (daxctl_memory_online_no_movable(mem)) {
> +                     log_err(&rl, "%s: memory unmovable for %s\n",
> +                                     devname,
> +                                     daxctl_dev_get_devname(dev));
> +                     return -EPERM;
> +             }
> +
> +             /*
> +              * If memory is still online and user wants to force it, attempt
> +              * to offline it.
> +              */
> +             if (daxctl_memory_is_online(mem) && param.force) {
> +                     rc = daxctl_memory_offline(mem);
> +                     if (rc) {
> +                             log_err(&rl, "%s: unable to offline %s: %s\n",
> +                                     devname,
> +                                     daxctl_dev_get_devname(dev),
> +                                     strerror(abs(rc)));
> +                             return rc;
> +                     }
> +             }
> +     }
> +
> +out:
> +     return cxl_region_disable(region);
> +}
> +
>   static int do_region_xable(struct cxl_region *region, enum region_actions 
> action)
>   {
>       switch (action) {
>       case ACTION_ENABLE:
>               return cxl_region_enable(region);
>       case ACTION_DISABLE:
> -             return cxl_region_disable(region);
> +             return disable_region(region);
>       case ACTION_DESTROY:
>               return destroy_region(region);
>       default:
> 
> 

Reply via email to