Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751995AbbH1Gd5 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 28 Aug 2015 02:33:57 -0400
Received: from mail-pa0-f46.google.com ([209.85.220.46]:34284 "EHLO
	mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751112AbbH1Gd4 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 28 Aug 2015 02:33:56 -0400
Date: Fri, 28 Aug 2015 14:33:32 +0800
From: Boqun Feng <boqun.feng@gmail.com>
To: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Ingo Molnar <mingo@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
        Sasha Levin <sasha.levin@oracle.com>,
        kernel test robot <ying.huang@intel.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] sched: fix tsk->pi_lock isn't held when
 do_set_cpus_allowed()
Message-ID: <20150828063332.GA26559@fixme-laptop.cn.ibm.com>
References: <BLU437-SMTP70037F60C2299FC76FB544806E0@phx.gbl>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="4Ckj6UjgE2iN1+kY"
Content-Disposition: inline
In-Reply-To: <BLU437-SMTP70037F60C2299FC76FB544806E0@phx.gbl>
User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3404
Lines: 103


--4Ckj6UjgE2iN1+kY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Wanpeng,

On Fri, Aug 28, 2015 at 12:02:47PM +0800, Wanpeng Li wrote:
<snip>
> This patch fix it by following the rules for changing task_struct::cpus_a=
llowed
> w/ both pi_lock and rq->lock are held.=20
>=20
> Reported-by: kernel test robot <ying.huang@intel.com>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  v1 -> v2:
>   * fix the silly double lock stuff
>   * follow the rules for changing task_struct::cpus_allowed
>=20
>  kernel/sched/core.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>=20
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b3386c6..8cf87e3 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5186,6 +5186,27 @@ static void migrate_tasks(struct rq *dead_rq)
>  		BUG_ON(!next);
>  		next->sched_class->put_prev_task(rq, next);
> =20
> +		/*
> +		 * Rules for changing task_struct::cpus_allowed are holding
> +		 * both pi_lock and rq->lock, such that holding either
> +		 * stabilizes the mask.
> +		 *
> +		 * Drop rq->lock is not quite as disastrous as it usually is
> +		 * because !cpu_active at this point, which means load-balance
> +		 * will not interfere.
> +		 */
> +		lockdep_unpin_lock(&rq->lock);
> +		raw_spin_unlock(&rq->lock);
> +		raw_spin_lock(&next->pi_lock);
> +		raw_spin_lock(&rq->lock);
> +		lockdep_pin_lock(&rq->lock);
> +		if (!(task_rq(next) =3D=3D rq && task_on_rq_queued(next))) {
> +			lockdep_unpin_lock(&rq->lock);
> +			raw_spin_unlock(&rq->lock);

Dropping rq->lock here means we will continue the loop without the
rq->lock, right? But we do have a lockdep_pin_lock(&rq->lock) in the
beginning of every iteration. So can we release rq->lock here?

Regards,
Boqun

> +			raw_spin_unlock(&next->pi_lock);
> +			continue;
> +		}
> +
>  		/* Find suitable destination for @next, with force if needed. */
>  		dest_cpu =3D select_fallback_rq(dead_rq->cpu, next);
> =20
> @@ -5196,6 +5217,7 @@ static void migrate_tasks(struct rq *dead_rq)
>  			rq =3D dead_rq;
>  			raw_spin_lock(&rq->lock);
>  		}
> +		raw_spin_unlock(&next->pi_lock);
>  	}
> =20
>  	rq->stop =3D stop;
> --=20
> 1.7.1
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--4Ckj6UjgE2iN1+kY
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAABCAAGBQJV4AC4AAoJEEl56MO1B/q47CoH/j/4z4A59MK3eUVKjaRPFeQc
gt6CV+cYRxXYWHGul/letNkEY+BgJwNPiPPjGEMdNyoNVw0/NGXb+Dccsnjbykn7
o1gCoqLr0edpsNi1r/Yd/v/3aQFmtom8NIaGINkChsp7g9SRjCTn9ccfVwR4z6N+
WS1jJWqMYGA2c0xTl2g/qrajC/vhwJVBbo7C7qcZyXjn83gsBIYiZBcRHsltU2SC
eoqja/T2WUxu59y9UT6X6MHBjzjAnZd57euffJ3GZ1JdYrc3aUSeteRYi2pMz4NF
yTSZY67a5dOh/dp4R+P1tRt4ybL2teMW/LGrt7MSjakSDnP9mEP7qkut6Cx5YHI=
=wL9o
-----END PGP SIGNATURE-----

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