From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754406AbbIHJtv (ORCPT ); Tue, 8 Sep 2015 05:49:51 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:60761 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751658AbbIHJtt (ORCPT ); Tue, 8 Sep 2015 05:49:49 -0400 Date: Tue, 8 Sep 2015 11:49:42 +0200 From: Peter Zijlstra To: Wanpeng Li Cc: Ingo Molnar , linux-kernel@vger.kernel.org, byungchul.park@lge.com, yuyang.du@intel.com Subject: Re: [PATCH] sched: fix lose fair sleeper bonus in switch_to_fair() Message-ID: <20150908094942.GB3644@twins.programming.kicks-ass.net> References: <20150907140221.GZ3644@twins.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.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 08, 2015 at 06:48:43AM +0800, Wanpeng Li wrote: > On 9/7/15 10:02 PM, Peter Zijlstra wrote: > >Please always Cc at least the person who wrote the lines you modify. > > > >On Mon, Sep 07, 2015 at 05:45:20PM +0800, Wanpeng Li wrote: > >>The sleeper task will be normalized when moved from fair_sched_class, in > >>order that vruntime will be adjusted either the task is running or sleeping > >>when moved back. The nomalization in switch_to_fair for sleep task will > >>result in lose fair sleeper bonus in place_entity() once the vruntime - > >>cfs_rq->min_vruntime is big when moved from fair_sched_class. > >> > >>This patch fix it by adjusting vruntime just during migrating as original > >>codes since the vruntime of the task has usually NOT been normalized in > >>this case. > >Sorry, I cannot follow that at all. Maybe its me being sleep deprived, > >but could you try that again? > > When changing away from the fair class while sleeping, relative vruntime is > calculated to handle the case sleep when moved from fair_sched_class and > running when moved to fair_sched_class. That, or the task being migrated to a different cgroup / cpu while being outside of the fair class. Also, the 'relative vruntime' as you call it, is an approximation for lag. Because we do not compute the 0-lag point (too expensive) we use min_vruntime as a conservative approximation. Lag is something you can transfer between runqueues, so that is the natural state for something that is not associated with a rq. > The absolute vruntime will be > calculated in enqueue_entity() either the task is running or sleeping when > moved back. Incorrect, enqueue_entity() will only do that conditionally, in the other cases it will assume se->vruntime is already absolute as you call it. attach_task_cfs_rq() must deal with the other cases. > The fair sleeper bonus should be gained in place_entity() if the > task is still sleeping. And this is still true. place_entity() assumes 'absolute vruntime', no matter how it got there. If we went to relative/lag, someone needs to go back to 'absolute'. > However, after recent commit ( 23ec30ddd7c1306: > 'sched: add two functions for att(det)aching a task to(from) a cfs_rq'), the > absolute vruntime will be calculated in switched_to_fair(), Also, conditionally, to complement the other places. > so the > max_vruntime() which is called in place_entity() will select the absolute > vruntime which is calculated in switched_to_fair() as the se->vruntime and > lose the fair sleeper bonus. You cannot loose your sleeper bonus by going to and from relative/lag (or rather you can, but that's due to min_vruntime being a poor substitute for the 0-lag point). But since we do this for all rq transfers we should not make exemptions. Afaict, the only possibly place for a bug to be here is vruntime_normalized(), if that somehow gets the conditions wrong we could fail-to/incorrectly subtract/add min_vruntime, creating a mess.