From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754898AbeEALbl (ORCPT ); Tue, 1 May 2018 07:31:41 -0400 Received: from bombadil.infradead.org ([198.137.202.133]:49100 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753717AbeEALbk (ORCPT ); Tue, 1 May 2018 07:31:40 -0400 Date: Tue, 1 May 2018 13:31:32 +0200 From: Peter Zijlstra To: "Kohli, Gaurav" Cc: tglx@linutronix.de, mpe@ellerman.id.au, mingo@kernel.org, bigeasy@linutronix.de, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Neeraj Upadhyay , Will Deacon , Oleg Nesterov Subject: Re: [PATCH v1] kthread/smpboot: Serialize kthread parking against wakeup Message-ID: <20180501113132.GF12217@hirez.programming.kicks-ass.net> References: <1524645199-5596-1-git-send-email-gkohli@codeaurora.org> <20180425200917.GZ4082@hirez.programming.kicks-ass.net> <20180426084131.GV4129@hirez.programming.kicks-ass.net> <20180426085719.GW4129@hirez.programming.kicks-ass.net> <4d3f68f8-e599-6b27-a2e8-9e96b401d57a@codeaurora.org> <20180430111744.GE4082@hirez.programming.kicks-ass.net> <3af3365b-4e3f-e388-8e90-45a3bd4120fd@codeaurora.org> <20180501101845.GE12217@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.5 (2018-04-13) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 01, 2018 at 04:10:53PM +0530, Kohli, Gaurav wrote: > Yes with loop, it will reset TASK_PARKED but that is not happening in the > dumps we have seen. But was that with or without the fixed wait-loop? I don't care about stuff you might have seen with the current code, that is clearly broken. > > takedown_cpu() can proceed beyond smpboot_park_threads() and kill the > > CPU before any of the threads are parked -- per having the complete() > > before hitting schedule(). > > > > And, afaict, that is harmless. When we go offline, sched_cpu_dying() -> > > migrate_tasks() will migrate any still runnable threads off the cpu. > > But because at this point the thread must be in the PARKED wait-loop, it > > will hit schedule() and go to sleep eventually. > > > > Also note that kthread_unpark() does __kthread_bind() to rebind the > > threads. > > > > Aaaah... I think I've spotted a problem there. We clear SHOULD_PARK > > before we rebind, so if the thread lost the first PARKED store, > > does the completion, gets migrated, cycles through the loop and now > > observes !SHOULD_PARK and bails the wait-loop, then __kthread_bind() > > will forever wait. > > > > So during next unpark > __kthread_unpark -> __kthread_bind -> wait_task_inactive (this got failed, > as current state is running so failed on below call: Aah, yes, I seem to have mis-remembered how wait_task_inactive() works. And it is indeed still a problem.. Let me ponder what the best solution is, it's a bit of a mess.