Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1525470 > unrolled thread

Re: [RFC v3 2/6] Improve the tracking of active utilisation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC v3 2/6] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-11-18 16:50 +0100
    Re: [RFC v3 2/6] Improve the tracking of active utilisation luca abeni <luca.abeni@unitn.it> - 2016-11-18 17:10 +0100
      Re: [RFC v3 2/6] Improve the tracking of active utilisation Peter Zijlstra <peterz@infradead.org> - 2016-11-18 20:00 +0100

#1525470 — Re: [RFC v3 2/6] Improve the tracking of active utilisation

On Mon, Oct 24, 2016 at 04:06:34PM +0200, Luca Abeni 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);
> +	}
> +	raw_spin_unlock(&rq->lock);

Its a bit sad having to take rq->lock here...

Also, what happens when hrtimer_try_to_cancel() fails?

> +
>  out:
>  	return cpu;
>  }

[toc] | [next] | [standalone]

#1525478

On Fri, 18 Nov 2016 16:47:48 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Oct 24, 2016 at 04:06:34PM +0200, Luca Abeni 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);
> > +	}
> > +	raw_spin_unlock(&rq->lock);  
> 
> Its a bit sad having to take rq->lock here...

I think I can move the locking inside the if() (so that rq->lock is not
taken if the inactive timer is not active); apart from this, the only
solution I can think about is to modify select_task_rq_dl() not to
change the cpu if the timer is active... (I think the task will be
migrated by a following push() if needed). What do you think? Any other
solution I am not seeing?


> Also, what happens when hrtimer_try_to_cancel() fails?

This is something I am working on... My original idea was that nothing
bad happens, because the timer handler will see the task as RUNNING and
will not decrease the running bw... But this is wrong.
My new idea is to add a "dl_contending" flag in the scheduling entity,
that indicates if the running bw has already been subtracted or not.
With this, the issue should be solved (if anyone sees additional
issues, or a better solution that does not require an additional flag,
let me know).

BTW, this code also missed a put_task_struct() for the case in which
hrtimer_try_to_cancel() does not fail :(


			Thanks,
				Luca
> 
> > +
> >  out:
> >  	return cpu;
> >  }  

[toc] | [prev] | [next] | [standalone]

#1525636

On Fri, Nov 18, 2016 at 05:06:50PM +0100, luca abeni wrote:
> 
> > Also, what happens when hrtimer_try_to_cancel() fails?
> 
> This is something I am working on... My original idea was that nothing
> bad happens, because the timer handler will see the task as RUNNING and
> will not decrease the running bw... But this is wrong.
> My new idea is to add a "dl_contending" flag in the scheduling entity,
> that indicates if the running bw has already been subtracted or not.
> With this, the issue should be solved (if anyone sees additional
> issues, or a better solution that does not require an additional flag,
> let me know).

Right. My suggestion would be to make it obvious, use that flag if
that's what it takes. We can always try and be clever later.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web