From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754014AbbJAGhk (ORCPT ); Thu, 1 Oct 2015 02:37:40 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:59214 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751073AbbJAGhg (ORCPT ); Thu, 1 Oct 2015 02:37:36 -0400 Date: Thu, 1 Oct 2015 08:37:32 +0200 From: Peter Zijlstra To: "Meyer, Mike" Cc: "linux-kernel@vger.kernel.org" , "mingo@redhat.com" Subject: Re: [PATCH] sched: fix task and run queue run_delay inconsistencies Message-ID: <20151001063732.GM2881@worktop.programming.kicks-ass.net> References: <72105DE171429F468B83CE64C279BFFA1760DC12@SUSHDC8002.TD.TERADATA.COM> <20150930154413.GO3604@twins.programming.kicks-ass.net> <72105DE171429F468B83CE64C279BFFA17615A68@SUSHDC8002.TD.TERADATA.COM> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <72105DE171429F468B83CE64C279BFFA17615A68@SUSHDC8002.TD.TERADATA.COM> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 30, 2015 at 08:28:41PM +0000, Meyer, Mike wrote: > > From: Peter Zijlstra [mailto:peterz@infradead.org] > > > > On Wed, Sep 23, 2015 at 12:37:18AM +0000, Meyer, Mike wrote: > > > > > > The proposed patch addresses the issue by calling > > > sched_info_reset_dequeued(thread) following the call to > > > enqueue_task(rq, thread) for running threads in situations in which > > > thread->sched_info.last_queued should remain 0. > > > > Would something like the below; which avoids calling > > sched_info_{de,}queued() for these sites also work? > > > > It even shrinks the code (due to inlining {en,de}queue_task()): > > > > $ size defconfig-build/kernel/sched/core.o defconfig- > > build/kernel/sched/core.o.orig > > text data bss dec hex filename > > 64019 23378 2344 89741 15e8d defconfig-build/kernel/sched/core.o > > 64149 23378 2344 89871 15f0f defconfig-build/kernel/sched/core.o.orig > > > Yes that will also address the issue. > > The reason I approached the way I did was to avoid adding code path to > the far more common uses of {en,de}queue_task() but I doubt anyone is > going to notice a difference with the addition of some register > save/restores and a compare in that path. Overall the code does > shrink with the alternative which is good. In most cases the flags should be compile time constants, and with the inline we can determine the branch at compile time, avoiding emitting that branch instruction entirely. But let me double check the asm for a few important sites. > My only comment is I am not sure about the naming of the flag > ENQUEUE_TEMP which implies (to me) the enqueue is temporary which > clearly it isn't. Maybe something like DEQUEUE_MOVE/ENQUEUE_MOVE > would be a bit more descriptive of the use case. Yes, I ran out of creative juices, let me attempt a better name once I've woken up a bit.