Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752108AbdC0Hgw (ORCPT ); Mon, 27 Mar 2017 03:36:52 -0400 Received: from mail.santannapisa.it ([193.205.80.99]:17385 "EHLO mail.santannapisa.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752038AbdC0Hgq (ORCPT ); Mon, 27 Mar 2017 03:36:46 -0400 Date: Mon, 27 Mar 2017 09:36:28 +0200 From: Luca Abeni To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Juri Lelli , Claudio Scordino , Steven Rostedt , Tommaso Cucinotta , Daniel Bristot de Oliveira , Joel Fernandes , Mathieu Poirier Subject: Re: [RFC v5 2/9] sched/deadline: improve the tracking of active utilization Message-ID: <20170327093628.29725038@luca> In-Reply-To: <20170324224715.4098dbfb@nowhere> References: <1490327582-4376-1-git-send-email-luca.abeni@santannapisa.it> <1490327582-4376-3-git-send-email-luca.abeni@santannapisa.it> <20170324132041.6fxayfe3id4af3n5@hirez.programming.kicks-ass.net> <20170324224715.4098dbfb@nowhere> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2759 Lines: 61 On Fri, 24 Mar 2017 22:47:15 +0100 luca abeni wrote: [...] > > > + } else { > > > + /* > > > + * Since "dl_non_contending" is not set, the > > > + * task's utilization has already been removed > > > from > > > + * active utilization (either when the task > > > blocked, > > > + * when the "inactive timer" fired). > > > + * So, add it back. > > > + */ > > > + add_running_bw(dl_se->dl_bw, dl_rq); > > > + } > > > +} > > > > In general I feel it would be nice to have a state diagram included > > somewhere near these two functions. It would be nice to not have to > > dig out the PDF every time. > > Ok... Since I am not good at ascii art, would it be ok to add a > textual description? If yes, I'll add a comment like: > " > The utilization of a task is added to the runqueue's active > utilization when the task becomes active (is enqueued in the > runqueue), and is removed when the task becomes inactive. A task does > not become immediately inactive when it blocks, but becomes inactive > at the so called "0 lag time"; so, we setup the "inactive timer" to > fire at the "0 lag time". When the "inactive timer" fires, the task > utilization is removed from the runqueue's active utilization. If the > task wakes up again on the same runqueue before the "0 lag time", the > active utilization must not be changed and the "inactive timer" must > be cancelled. If the task wakes up again on a different runqueue > before the "0 lag time", then the task's utilization must be removed > from the previous runqueue's active utilization and must be added to > the new runqueue's active utilization. > In order to avoid races between a task waking up on a runqueue while > the "inactive timer" is running on a different CPU, the > "dl_non_contending" flag is used to indicate that a task is not on a > runqueue but is active (so, the flag is set when the task blocks and > is cleared when the "inactive timer" fires or when the task wakes > up). " After re-reading all of this, I realized two things: 1) The comment I added before the definition of the dl_non_contending flag in sched.h is confusing. I'll try to rephrase it 2) The "dl_non_contending" name might be part of the issue, here. It tries to map concepts from the GRUB paper to the implementation, so it is understandable and makes things clear (I hope) for people who know the paper... But it is not the best name for people not knowing the GRUB paper (and pretending that someone studies the paper to understand the code is not good :). So, what about "inactive_timer_armed" (or similar)? This would immediately clarify what the flag is about... What do you think? Would this renaming be useful? Thanks, Luca