Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933135AbcDTMUT (ORCPT ); Wed, 20 Apr 2016 08:20:19 -0400 Received: from merlin.infradead.org ([205.233.59.134]:37982 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932253AbcDTMUR (ORCPT ); Wed, 20 Apr 2016 08:20:17 -0400 Date: Wed, 20 Apr 2016 14:20:11 +0200 From: Peter Zijlstra To: Thomas Gleixner Cc: xlpang@redhat.com, linux-kernel@vger.kernel.org, Juri Lelli , Ingo Molnar , Steven Rostedt Subject: Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Message-ID: <20160420122011.GX3430@twins.programming.kicks-ass.net> References: <1460633827-345-1-git-send-email-xlpang@redhat.com> <1460633827-345-2-git-send-email-xlpang@redhat.com> <57149E8A.6060701@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Content-Length: 1230 Lines: 34 On Mon, Apr 18, 2016 at 11:02:28AM +0200, Thomas Gleixner wrote: > On Mon, 18 Apr 2016, Xunlei Pang wrote: > > On 2016/04/18 at 16:23, Thomas Gleixner wrote: > > > On Thu, 14 Apr 2016, Xunlei Pang wrote: > > >> We should deboost before waking the high-prio task such that > > >> we don't run two tasks with the 'same' priority. > > > No. This is fundamentaly broken. > > > > > > T1 (prio 0) lock(X) > > > > > > --> preemption > > > > > > T2 (prio 10) lock(X) > > > boost(T1) > > > schedule() > > > > > > T1 (prio 10) unlock(X) > > > > We add a preempt_disable() before deboost to avoid the breakage, > > there's also some comment about this in the patch's code. > > So the changelog is useless and misleading. Neither does it explain what's > wrong with having two tasks with the same priority in running state. So its semantically icky to have the two tasks running off the same state and practically icky when you consider bandwidth inheritance -- where the boosted task wants to explicitly modify the state of the booster. In that latter case you really want to unboost before you let the booster run again. However, you noted we need to deal with this case due to the whole optimistic spinning crap anyway :/