On 02/11/16 03:35, Luca Abeni wrote: > On Tue, 1 Nov 2016 22:46:33 +0100 > luca abeni <[email protected]> wrote: > [...] > > > > @@ -1074,6 +1161,14 @@ select_task_rq_dl(struct task_struct *p, int > > > > cpu, int sd_flag, int flags) > > > > } > > > > rcu_read_unlock(); > > > > > > > > + rq = task_rq(p); > > > > + raw_spin_lock(&rq->lock); > > > > + if (hrtimer_active(&p->dl.inactive_timer)) { > > > > + sub_running_bw(&p->dl, &rq->dl); > > > > + hrtimer_try_to_cancel(&p->dl.inactive_timer); > > > > > > Can't we subtract twice if it happens that after we grabbed rq_lock the > > > timer > > > fired, so it's now waiting for that lock and it goes ahead and > > > sub_running_bw > > > again after we release the lock? > > Uhm... I somehow convinced myself that this could not happen, but I do not > > remember the details, sorry :( > I think I remember the answer now: pi_lock is acquired before invoking > select_task_rq > and is released after invoking enqueue_task... So, if there is a pending > inactive > timer, its handler will be executed after the task is enqueued... It will see > the task > as RUNNING, and will not decrease the active utilisation. >
Oh, because we do task_rq_lock() inactive_task_timer(). So, that should save us from the double subtract. Would you mind adding something along the line of what you said above as a comment for next version? Thanks, - Juri
