linux-kernel - RE: [PATCH] sched/fair: fix task_numa_migrate to consider both task and group benefits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c57fcd55047412b9752aeba79f86dec@hygon.cn>
Date: Fri, 5 Sep 2025 02:14:30 +0000
From: Jianyong Wu <wujianyong@...on.cn>
To: Ethan Zhao <etzhao1900@...il.com>
CC: "jianyong.wu@...look.com" <jianyong.wu@...look.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] sched/fair: fix task_numa_migrate to consider both task
 and group benefits

Hello Ethan,

Thanks for reply.

There is inconsistency between the comments and the code. See the discussion in an older patch here https://lkml.org/lkml/2015/6/16/540

Following that, the issue is with the comments, not the code.

Bests
Jianyong 

> -----Original Message-----
> From: Ethan Zhao <etzhao1900@...il.com>
> Sent: Friday, September 5, 2025 9:13 AM
> To: Jianyong Wu <wujianyong@...on.cn>
> Cc: jianyong.wu@...look.com; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH] sched/fair: fix task_numa_migrate to consider both task
> and group benefits
> 
> 
> 
> On 8/29/2025 4:55 PM, Jianyong Wu wrote:
> > The comment indicates that when searching for a suitable NUMA node, we
> > should ensure that the selected node benefits both the task and its
> > NUMA group. However, the current implementation can only guarantee
> > that either the task or the group benefits, but not necessarily both.
> >
> > Signed-off-by: Jianyong Wu <wujianyong@...on.cn>
> > ---
> >   kernel/sched/fair.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index
> > b173a059315c..58c899738399 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -2568,7 +2568,7 @@ static int task_numa_migrate(struct task_struct
> *p)
> >   			/* Only consider nodes where both task and groups benefit
> */
> >   			taskimp = task_weight(p, nid, dist) - taskweight;
> >   			groupimp = group_weight(p, nid, dist) - groupweight;
> > -			if (taskimp < 0 && groupimp < 0)
> > +			if (taskimp < 0 || groupimp < 0)
> Perhaps you misunderstand the comment, && means either the task or the
> group has NO benefit from this migration, it wouldn't be done.
> But if you replace it with ||, you will ignore the target node that could benefit
> either the task or the group.
> 
> There is more logic to consider the benefit for both task & group in the later
> function part.
> 
> One question, why not
> if (taskimp <= 0 && groupimp <= 0) ?
> 
> Thanks,
> Ethan
> >   				continue;
> >
> >   			env.dist = dist;
>