On 22/09/2023 07:19, Dave Jiang wrote: > > > On 9/20/23 19:58, Zhijian Li (Fujitsu) wrote: >> 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 > > Zhijian, > So this was there before the region enumeration showed up according to Dan. > Now an update to check if the memdev is part of any active region should be > added. Either you guys can send a patch or I can go added it. Let me know. > Thanks!
Understood, thanks for your information, please go ahead :) I believe our guys are willing to test it. Thanks Zhijian > > >> >> 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", ¶m.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: >>>
