linux-kernel - Re: [RFC 4/8] Improve the tracking of active utilisation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 15 Jan 2016 07:48:41 +0100
From:	Luca Abeni <luca.abeni@...tn.it>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	Juri Lelli <juri.lelli@....com>
Subject: Re: [RFC 4/8] Improve the tracking of active utilisation

Hi Peter,


On Thu, 14 Jan 2016 18:16:19 +0100
Peter Zijlstra <peterz@...radead.org> wrote:

[...]
> > +	/* If the "inactive timer" is still active, stop it adn
> > leave
> > +	 * the active utilisation unchanged.
> > +	 * If it is running, increase the active utilisation
> > +	 */
> > +	if (hrtimer_active(&dl_se->inactive_timer)) {
> > +		hrtimer_try_to_cancel(&dl_se->inactive_timer);
> 
> what if cancel fails?
Eh, this is a tricky point :)
In this case, the "if (p->state == TASK_RUNNING) {" in
inactive_task_timer() should detect what happened, and avoid decreasing
the active utilization. So, we should be safe... At least, this was my
plan, maybe I missed something.


> > @@ -1248,8 +1370,6 @@ static void task_fork_dl(struct task_struct
> > *p) static void task_dead_dl(struct task_struct *p)
> >  {
> >  	struct dl_bw *dl_b = dl_bw_of(task_cpu(p));
> > -	struct dl_rq *dl_rq = dl_rq_of_se(&p->dl);
> > -	struct rq *rq = rq_of_dl_rq(dl_rq);
> >  
> >  	/*
> >  	 * Since we are TASK_DEAD we won't slip out of the domain!
> > @@ -1258,10 +1378,6 @@ static void task_dead_dl(struct task_struct
> > *p) /* XXX we should retain the bw until 0-lag */
> >  	dl_b->total_bw -= p->dl.dl_bw;
> >  	raw_spin_unlock_irq(&dl_b->lock);
> > -
> > -	if (task_on_rq_queued(p)) {
> > -		clear_running_bw(&p->dl, &rq->dl);
> > -	}
> 
> what happens if the timer is still active here? then we get the timer
> storage freed while enqueued?
I think here (and in the successive comment) we are safe because of the
get_task_struct() you mention in another email, right? Or am I missing
something else?


			Thanks,
				Luca