Date: Tue, 5 Apr 2016 20:11:25 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: luca abeni <luca.abeni@unitn.it>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@arm.com>
Subject: Re: [RFC v2 3/7] Improve the tracking of active utilisation
Message-ID: <20160405181125.GQ3408@twins.programming.kicks-ass.net>
References: <1459523553-29089-1-git-send-email-luca.abeni@unitn.it>
 <1459523553-29089-4-git-send-email-luca.abeni@unitn.it>
 <20160405150036.GA3430@twins.programming.kicks-ass.net>
 <20160405195657.586e8c97@utopia>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160405195657.586e8c97@utopia>
User-Agent: Mutt/1.5.21 (2012-12-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2070
Lines: 46

On Tue, Apr 05, 2016 at 07:56:57PM +0200, luca abeni wrote:
> On Tue, 5 Apr 2016 17:00:36 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Fri, Apr 01, 2016 at 05:12:29PM +0200, Luca Abeni wrote:
> > > +static void task_go_inactive(struct task_struct *p)
> > > +{
> > > +	struct sched_dl_entity *dl_se = &p->dl;
> > > +	struct hrtimer *timer = &dl_se->inactive_timer;
> > > +	struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
> > > +	struct rq *rq = rq_of_dl_rq(dl_rq);
> > > +	ktime_t now, act;
> > > +	s64 delta;
> > > +	u64 zerolag_time;
> > > +
> > > +	WARN_ON(dl_se->dl_runtime == 0);
> > > +
> > > +	/* If the inactive timer is already armed, return immediately */
> > > +	if (hrtimer_active(&dl_se->inactive_timer))
> > > +		return;
> > 
> > So while we start the timer on the local cpu, we don't migrate the timer
> > when we migrate the task, so the callback can happen on a remote cpu,
> > right?
> > 
> > Therefore, the timer function might still be running, but just have done
> > task_rq_unlock(), which would have allowed our cpu to acquire the
> > rq->lock and get here.
> > 
> > Then the above check is true, we'll quit, but effectively the inactive
> > timer will not run 'again'.
> Uhm... So the problem is:
> - Task T wakes up, but cannot cancel its inactive timer, because it is running
> 	+ This should not be a problem: inactive_task_timer() will return without
>           doing anything
> - Before inactive_task_timer() can actually run, task T migrates to a different CPU
> - Befere the timer finishes to run, the task blocks again... So, task_go_inactive()
>   sees the timer as active and returns immediately. But the timer has already
>   executed (without doing anything). So noone decreases the rq utilisation.
> 
> I did not think about this issue, and I never managed to trigger it in my
> tests... I'll try to see how it can be addressed. Do you have any suggestions?

So my brain is about to give out, but it might be easiest to simply
track if the current tasks' bandwidth is added with a per task variable
under pi and rq lock.