linux-kernel - Re: [PATCH 2/2] sched:fix: Signed overflow prevention for vacancy calculation

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: <51A6B07C.4060108@intel.com>
Date:	Thu, 30 May 2013 09:50:52 +0800
From:	Alex Shi <alex.shi@...el.com>
To:	Lukasz Majewski <l.majewski@...sung.com>
CC:	Linux PM list <linux-pm@...r.kernel.org>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Jonghwa Lee <jonghwa3.lee@...sung.com>,
	Myungjoo Ham <myungjoo.ham@...sung.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH 2/2] sched:fix: Signed overflow prevention for vacancy
 calculation

On 05/23/2013 04:34 PM, Lukasz Majewski wrote:
> Nasty bug with vacancy calculation has been fixed.
> In the if statement the FULL_UTIL is a large constant, max_cfs_util()
> returns unsigned and putil is also defined as unsigned. The outcome
> is that this condition is always true.
> 
> As observed, this was the reason for frequent jumps of processes between
> CPUs.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@...sung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>

Thanks for the catching!
I do a little change, would you like to keep the authorship?

---

>From 4f7f2d091b7210e0be689d3e063d3cc82da1b1af Mon Sep 17 00:00:00 2001
From: Lukasz Majewski <l.majewski@...sung.com>
Date: Thu, 30 May 2013 09:21:46 +0800
Subject: [PATCH 2/2] sched:fix: Signed overflow prevention for vacancy
 calculation

Nasty bug with vacancy calculation has been fixed.
In the if statement the FULL_UTIL is a large constant, max_cfs_util()
returns unsigned and putil is also defined as unsigned. The outcome
is that this condition is always true.

As observed, this was the reason for frequent jumps of processes between
CPUs.

Signed-off-by: Lukasz Majewski <l.majewski@...sung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
Signed-off-by: Alex Shi <alex.shi@...el.com>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 3a4917c..56a9d16 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3468,10 +3468,10 @@ find_leader_cpu(struct sched_group *group, struct task_struct *p, int this_cpu,
 	unsigned putil;
 	putil = div_u64(((u64)p->se.avg.runnable_avg_sum << SCHED_POWER_SHIFT),
 				p->se.avg.runnable_avg_period + 1);
+	vacancy = FULL_UTIL - max_cfs_util(this_cpu, wakeup) - (putil << 2);
 
 	/* bias toward local cpu */
-	if (cpumask_test_cpu(this_cpu, tsk_cpus_allowed(p)) &&
-		FULL_UTIL - max_cfs_util(this_cpu, wakeup) - (putil << 2) > 0)
+	if (cpumask_test_cpu(this_cpu, tsk_cpus_allowed(p)) && vacancy > 0)
 		return this_cpu;
 
 	/* Traverse only the allowed CPUs */
-- 
1.7.12


-- 
Thanks
    Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/