Re: [PATCH v2] sched/numa: Introduce per cgroup numa balance control

["Chen, Yu C" <yu.c.chen@intel.com>]

On 7/2/2025 1:58 AM, Libo Chen wrote:
> Hi Chenyu
> 
> On 7/1/25 09:36, Chen, Yu C wrote:
>>>> +
>>>>    /*
>>>>     * Got a PROT_NONE fault for a page on @node.
>>>>     */
>>>> @@ -3189,6 +3212,9 @@ void task_numa_fault(int last_cpupid, int mem_node, int pages, int flags)
>>>>             !cpupid_valid(last_cpupid)))
>>>>            return;
>>>>    +    if (!tg_numa_balance_enabled(p))
>>>> +        return;
>>>> +
>>>
>>> I think this one may be redundant when you already have it in task_numa_work().  Without the
>>> scanning, there won't be any hinting page faults on that task, so neither do_numa_page() nor
>>> do_huge_pmd_numa_page() will be called. Though it's a minor issue if tg_numa_balance_enabled(p)
>>> is fast.
>>>
>>
>> Previously I was thinking of the following sequence:
>> 1. the NUMA balancing is enabled and task_numa_work() is invoked,
>>     pages are scanned and PROT_NONE is set.
>> 2. cgroup NUMA balancing is disabled by the user
>> 3. do_numa_page() is triggered and PROT_NONE is cleared.
>>     We don't want to do further task migration and
>>     task_numa_fault() bails out.(page migration is still
>>     allowed as we mainly want to control the behavior of
>>     the task)
>>
> 
> Ah right, that makes sense. Does that fall under unlikely()? The timing window seems to be
> quite small to me.
> 

Yup, and I guess that depends on how often the task access the
"invalid" address - it might take quite some time after the
page been set invalid and then the task access it to trigger
a page fault.

Thanks,
Chenyu