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

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

On 10/29/23 21:33, Xiao Yang wrote:
> 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;
>     }
> 

Ah that was a mistake. I meant to call the query function and not the online op function. Do you have any objections to

if (!daxctl_memory_is_movable(mem))

> Best Regards,
> Xiao Yang