Re: [NDCTL PATCH v2] cxl/region: Add -f option for disable-region

[Dave Jiang <dave.jiang@intel.com>]

On 10/9/23 03:52, Xiao Yang wrote:
> On 2023/9/21 6:57, Dave Jiang wrote:
>> +        if (daxctl_memory_online_no_movable(mem)) {
>> +            log_err(&rl, "%s: memory unmovable for %s\n",
>> +                    devname,
>> +                    daxctl_dev_get_devname(dev));
>> +            return -EPERM;
>> +        }
> Hi Dave,
> 
> It seems wrong to check if memory is unmovable by the return number of daxctl_memory_online_no_movable(mem) here. IIRC, the return number of daxctl_memory_online_no_movable(mem)/daxctl_memory_op(MEM_GET_ZONE) indicates how many memory blocks have the same memory zone. So I think you should check mem->zone and MEM_ZONE_NORMAL as daxctl_memory_is_movable() did.

Do you mean:
rc = daxctl_memory_online_no_movable(mem);
if (rc < 0)
	return rc;
if (rc > 0) {
	log_err(&rl, "%s memory unmovable for %s\n' ...);
	return -EPERM;
}

> 
> Besides, I send a patch to improve the implementation of daxctl_memory_online_with_zone().
> https://lore.kernel.org/nvdimm/20231009103521.1463-1-yangx.jy@fujitsu.com/T/#u

Thanks. I added my review tag.

> 
> Best Regards,
> Xiao Yang