From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753774AbbE1Lvg (ORCPT ); Thu, 28 May 2015 07:51:36 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:35712 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753615AbbE1Lv0 (ORCPT ); Thu, 28 May 2015 07:51:26 -0400 Date: Thu, 28 May 2015 13:51:18 +0200 From: Peter Zijlstra To: Mike Galbraith Cc: LKML , Ingo Molnar , ktkhai@parallels.com Subject: Re: sched_setscheduler() vs idle_balance() race Message-ID: <20150528115118.GG3644@twins.programming.kicks-ass.net> References: <1432799032.3237.119.camel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1432799032.3237.119.camel@gmail.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 28, 2015 at 09:43:52AM +0200, Mike Galbraith wrote: > Hi Peter, > > I'm not seeing what prevents pull_task() from yanking a task out from > under __sched_setscheduler(). A box sprinkling smoldering 3.0 kernel > wreckage all over my bugzilla mbox isn't seeing it either ;-) > > Scenario: rt task forks, wakes child to CPU foo, immediately tries to > change child to fair class, calls switched_from_rt(), that leads to > pull_rt_task() -> double_lock_balance() which momentarily drops child's > rq->lock, letting some prick doing idle balancing over on CPU bar in to > migrate the child. Rt parent then calls switched_to_fair(), and box > explodes when we use the passed rq as if the child still lived there. > > I sent a patchlet to verify that the diagnosis is really really correct > (can_migrate_task() says no if ->pi_lock is held), but I think it is, > the 8x10 color glossy with circles and arrows clearly shows both tasks > with their grubby mitts on that child at the same time, each thinking it > has that child locked down tight. > > Not seeing what should prevent that in mainline either, I'll just ask > while I wait to (hopefully) hear "yup, all better". The last patch to come close is 67dfa1b756f2 ("sched/deadline: Implement cancel_dl_timer() to use in switched_from_dl()") Which places the comment /* Possible rq-lock hole */ between switched_from() and switched_to(). Which is exactly the hole you mean, right? And that commit talks about how all that is 'safe' because all scheduler operations take ->pi_lock, which is true, except for load-balancing, which only uses rq->lock. Furthermore, we call check_class_changed() _after_ we enqueue the task on the new class, so balancing can indeed occur. Lemme go stare at this; ideally we'd call check_class_changed() at __setscheduler() time where the task is off all rqs, but I suspect there's 'obvious' problems with that..