From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752024AbcFVSEV (ORCPT ); Wed, 22 Jun 2016 14:04:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49875 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751286AbcFVSEU (ORCPT ); Wed, 22 Jun 2016 14:04:20 -0400 Message-ID: <1466618651.15275.21.camel@redhat.com> Subject: Re: [PATCH v2 02/13] sched/fair: Consistent use of prev_cpu in wakeup path From: Rik van Riel To: Morten Rasmussen , peterz@infradead.org, mingo@redhat.com Cc: dietmar.eggemann@arm.com, yuyang.du@intel.com, vincent.guittot@linaro.org, mgalbraith@suse.de, linux-kernel@vger.kernel.org Date: Wed, 22 Jun 2016 14:04:11 -0400 In-Reply-To: <1466615004-3503-3-git-send-email-morten.rasmussen@arm.com> References: <1466615004-3503-1-git-send-email-morten.rasmussen@arm.com> <1466615004-3503-3-git-send-email-morten.rasmussen@arm.com> Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-1HYpJrIXzyd4ZnoS1fLi" Mime-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 22 Jun 2016 18:04:14 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-1HYpJrIXzyd4ZnoS1fLi Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2016-06-22 at 18:03 +0100, Morten Rasmussen wrote: > In commit ac66f5477239 ("sched/numa: Introduce migrate_swap()") > select_task_rq() got a 'cpu' argument to enable overriding of > prev_cpu > in special cases (NUMA task swapping). However, the > select_task_rq_fair() helper functions: wake_affine() and > select_idle_sibling(), still use task_cpu(p) directly to work out > prev_cpu which leads to inconsistencies. >=20 > This patch passes prev_cpu (potentially overridden by NUMA code) into > the helper functions to ensure prev_cpu is indeed the same cpu > everywhere in the wakeup path. >=20 > cc: Ingo Molnar > cc: Peter Zijlstra > cc: Rik van Riel >=20 > Signed-off-by: Morten Rasmussen > --- > =C2=A0kernel/sched/fair.c | 24 +++++++++++++----------- > =C2=A01 file changed, 13 insertions(+), 11 deletions(-) >=20 > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index c6dd8bab010c..eec8e29104f9 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -656,7 +656,7 @@ static u64 sched_vslice(struct cfs_rq *cfs_rq, > struct sched_entity *se) > =C2=A0} > =C2=A0 > =C2=A0#ifdef CONFIG_SMP > -static int select_idle_sibling(struct task_struct *p, int cpu); > +static int select_idle_sibling(struct task_struct *p, int prev_cpu, > int cpu); > =C2=A0static unsigned long task_h_load(struct task_struct *p); > =C2=A0 > =C2=A0/* > @@ -1483,7 +1483,8 @@ static void task_numa_compare(struct > task_numa_env *env, > =C2=A0 =C2=A0* Call select_idle_sibling to maybe find a better one. > =C2=A0 =C2=A0*/ > =C2=A0 if (!cur) > - env->dst_cpu =3D select_idle_sibling(env->p, env- > >dst_cpu); > + env->dst_cpu =3D select_idle_sibling(env->p, env- > >src_cpu, > + =C2=A0=C2=A0=C2=A0env->dst_cpu); It is worth remembering that "prev" will only ever be returned by select_idle_sibling() if it is part of the same NUMA node as target. That means this patch does not change behaviour of the NUMA balancing code, since that always migrates between nodes. Now lets look at try_to_wake_up(). It will pass p->wake_cpu as the argument for "prev_cpu", which again appears to be the same CPU number as that used by the current code. I have no objection to your patch, but must be overlooking something, since I cannot find a change in behaviour that your patch would create. --=20 All rights reversed --=-1HYpJrIXzyd4ZnoS1fLi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJXatMcAAoJEM553pKExN6DmdoIAJeNaAHqsim+tkDrRVF351Wa x0cLRbXzaWddZhtVppX8wfK9NKnuHLM76abJUj7fu7WRVjUnEdTeLZ8fp8eEeOcY sRDUWnip+m6fsPyR7oMrhJxf2sut6QTbhs4ZRtDtMNAIm37oeh4SWuMHyPj5bNSj geW7gNfnlHykhxt2DWYtrd9TsMAdTeL2An+Od6l143lZvkxGMda7CgG/WUbJSkKj dXtuOEQgt6+h5UwX1Jg8pg8wqIX5B9zWxJyjXxuM2bgKIxN0vz2FyDJdXLd8aocM 8utY8xPXfojNnjBcY69Dv2ikWxmphFZoDwqtc6uuCMshApXchkd4JLgiEAd4DVw= =zRsX -----END PGP SIGNATURE----- --=-1HYpJrIXzyd4ZnoS1fLi--