From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3E02FC433DB for ; Wed, 24 Feb 2021 18:00:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E5DAD64ECE for ; Wed, 24 Feb 2021 18:00:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232673AbhBXR77 (ORCPT ); Wed, 24 Feb 2021 12:59:59 -0500 Received: from foss.arm.com ([217.140.110.172]:42482 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231701AbhBXR74 (ORCPT ); Wed, 24 Feb 2021 12:59:56 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 46E1E1FB; Wed, 24 Feb 2021 09:59:10 -0800 (PST) Received: from e113632-lin (e113632-lin.cambridge.arm.com [10.1.194.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2AB563F73B; Wed, 24 Feb 2021 09:59:09 -0800 (PST) From: Valentin Schneider To: Peter Zijlstra , Ingo Molnar , Thomas Gleixner Cc: Vincent Guittot , Mel Gorman , Dietmar Eggemann , linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH 6/6] sched: Simplify set_affinity_pending refcounts In-Reply-To: References: <20210224122439.176543586@infradead.org> <20210224131355.724130207@infradead.org> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Wed, 24 Feb 2021 17:59:01 +0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24/02/21 16:34, Peter Zijlstra wrote: > Elsewhere Valentin argued something like the below ought to be possible. > I've not drawn diagrams yet, but if I understood his argument right it > should be possible. > > --- > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 1c56ac4df2c9..3ffbd1b76f3e 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2204,9 +2204,10 @@ static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flag > * then complete now. > */ > pending = p->migration_pending; > - if (pending && !pending->stop_pending) { > + if (pending) { > p->migration_pending = NULL; > - complete = true; > + if (!pending->stop_pending) > + complete = true; > } > > task_rq_unlock(rq, p, rf); > @@ -2286,10 +2287,9 @@ static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flag > if (task_on_rq_queued(p)) > rq = move_queued_task(rq, rf, p, dest_cpu); > > - if (!pending->stop_pending) { > - p->migration_pending = NULL; > + p->migration_pending = NULL; > + if (!pending->stop_pending) > complete = true; > - } > } > task_rq_unlock(rq, p, rf); > I was thinking of the "other way around"; i.e. modify migration_cpu_stop() into: --- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 9492f8eb242a..9546f0263970 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1926,6 +1926,11 @@ static int migration_cpu_stop(void *data) raw_spin_lock(&p->pi_lock); rq_lock(rq, &rf); + /* + * If we were passed a pending, then ->stop_pending was set, thus + * p->migration_pending must have remained stable. + */ + WARN_ON_ONCE(pending && pending != p->migration_pending); /* * If task_rq(p) != rq, it cannot be migrated here, because we're * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because @@ -1936,8 +1941,7 @@ static int migration_cpu_stop(void *data) goto out; if (pending) { - if (p->migration_pending == pending) - p->migration_pending = NULL; + p->migration_pending = NULL; complete = true; } @@ -1976,8 +1980,7 @@ static int migration_cpu_stop(void *data) * somewhere allowed, we're done. */ if (cpumask_test_cpu(task_cpu(p), p->cpus_ptr)) { - if (p->migration_pending == pending) - p->migration_pending = NULL; + p->migration_pending = NULL; complete = true; goto out; } --- Your change reinstores the "triple SCA" pattern, where a stopper can run with arg->pending && arg->pending != p->migration_pending, which I was kinda happy to see go away...