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

[Xiao Yang <yangx.jy@fujitsu.com>]

On 2023/10/14 6:38, Dave Jiang wrote:
> 
> 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;
> }
> 
Hi Dave,

Sorry for the late reply.

Is it necessary to try to online the memory region to the 
MEM_ZONE_NORMAL by daxctl_memory_online_no_movable(mem)? If you just 
want to check if the onlined memory region is in the MEM_ZONE_NORMAL, 
the following code seems better:
     mem->zone = 0;
     rc = daxctl_memory_op(mem, MEM_GET_ZONE);
     if (rc < 0)
         return rc;
     if (mem->zone == MEM_ZONE_NORMAL) {
         log_err(&rl, "%s memory unmovable for %s\n' ...);
	return -EPERM;
     }

Best Regards,
Xiao Yang