LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Abdul Haleem <abdhalee@linux.vnet.ibm.com>,
	Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com
Subject: Re: [PATCH 1/2] workqueue: Move wq_update_unbound_numa() to the beginning of CPU_ONLINE
Date: Thu, 16 Jun 2016 15:35:04 -0400
Message-ID: <20160616193504.GB3262@mtj.duckdns.org> (raw)
In-Reply-To: <20160615192844.GA20301@in.ibm.com>

Hello,

So, the issue of the initial worker not having its affinity set
correctly wasn't caused by the order of the operations.  Reordering
just made set_cpus_allowed tried one more time late enough so that it
hides the race condition most of the time.  The problem is that
CPU_ONLINE callbacks are called while the cpu being onlined is online
but not active and select_fallback_rq() only considers active cpus, so
if a kthread gets scheduled in the meantime and it doesn't have any
cpu which is active in its allowed mask, it's allowed mask gets reset
to cpu_possible_mask.

Would something like the following make sense?

Thanks.
------ 8< ------
Subject: [PATCH] sched: allow kthreads to fallback to online && !active cpus

During CPU hotplug, CPU_ONLINE callbacks are run while the CPU is
online but not active.  A CPU_ONLINE callback may create or bind a
kthread so that its cpus_allowed mask only allows the CPU which is
being brought online.  The kthread may start executing before the CPU
is made active and can end up in select_fallback_rq().

In such cases, the expected behavior is selecting the CPU which is
coming online; however, because select_fallback_rq() only chooses from
active CPUs, it determines that the task doesn't have any viable CPU
in its allowed mask and ends up overriding it to cpu_possible_mask.

CPU_ONLINE callbacks should be able to put kthreads on the CPU which
is coming online.  Update select_fallback_rq() so that it follows
cpu_online() rather than cpu_active() for kthreads.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Gautham R Shenoy <ego@linux.vnet.ibm.com>
---
 kernel/sched/core.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 017d539..a12e3db 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1536,7 +1536,9 @@ static int select_fallback_rq(int cpu, struct task_struct *p)
 	for (;;) {
 		/* Any allowed, online CPU? */
 		for_each_cpu(dest_cpu, tsk_cpus_allowed(p)) {
-			if (!cpu_active(dest_cpu))
+			if (!(p->flags & PF_KTHREAD) && !cpu_active(dest_cpu))
+				continue;
+			if (!cpu_online(dest_cpu))
 				continue;
 			goto out;
 		}

  reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-19 10:57 WARNING at kernel/sched/core.c:1166 while booting 4.6.0 mainline on ppc64le bare metal abdhalee
2016-05-19 12:34 ` Gavin Shan
2016-05-26 15:11 ` Gautham R Shenoy
2016-06-07 12:29   ` Abdul Haleem
2016-06-07 15:14     ` [PATCH 0/2] Fix CPU Online handling for unbounded worker threads Gautham R. Shenoy
2016-06-07 15:14       ` [PATCH 1/2] workqueue: Move wq_update_unbound_numa() to the beginning of CPU_ONLINE Gautham R. Shenoy
2016-06-15 15:53         ` Tejun Heo
2016-06-15 19:28           ` Gautham R Shenoy
2016-06-16 19:35             ` Tejun Heo [this message]
2016-06-21 14:12               ` Gautham R Shenoy
2016-06-21 15:36                 ` Tejun Heo
2016-06-21 19:37                   ` Peter Zijlstra
2016-06-21 19:43                     ` Tejun Heo
2016-06-21 19:47                       ` Peter Zijlstra
2016-06-22  5:15                         ` Gautham R Shenoy
2016-06-07 15:14       ` [PATCH 2/2] workqueue:Fix affinity of an unbound worker of a node with 1 online CPU Gautham R. Shenoy
2016-06-08  6:03         ` Abdul Haleem
2016-06-14 11:22         ` Peter Zijlstra
2016-06-15 10:19           ` Gautham R Shenoy
2016-06-15 11:32             ` Peter Zijlstra
2016-06-15 12:50               ` Gautham R Shenoy
2016-06-15 13:14                 ` Peter Zijlstra
2016-06-15 16:01                   ` Tejun Heo
2016-06-16 12:11                     ` Michael Ellerman
2016-06-16 12:45                       ` Peter Zijlstra
2016-06-16 19:39                         ` Tejun Heo
2016-06-17  1:49                           ` Michael Ellerman
2016-07-15  5:27                           ` Gautham R Shenoy
2016-07-15  5:30                           ` Michael Ellerman
     [not found]                           ` <57887507.911f240a.687de.08c5SMTPIN_ADDED_BROKEN@mx.google.com>
2016-07-15 12:10                             ` Tejun Heo
2016-06-13  5:44       ` [PATCH 0/2] Fix CPU Online handling for unbounded worker threads Gautham R Shenoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160616193504.GB3262@mtj.duckdns.org \
    --to=htejun@gmail.com \
    --cc=abdhalee@linux.vnet.ibm.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=ego@linux.vnet.ibm.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git