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

Groups > linux.kernel > #1633734

Re: [PATCH 1/2] sched/fair: Use task_groups instead of leaf_cfs_rq_list to walk all cfs_rqs

Show all headers | View raw

On Tue, Apr 25, 2017 at 05:40:39PM -0700, Tejun Heo wrote:

> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4644,23 +4644,32 @@ static void destroy_cfs_bandwidth(struct
>  
>  static void __maybe_unused update_runtime_enabled(struct rq *rq)
>  {
> -	struct cfs_rq *cfs_rq;
> +	struct task_group *tg;
>  
> -	for_each_leaf_cfs_rq(rq, cfs_rq) {
> -		struct cfs_bandwidth *cfs_b = &cfs_rq->tg->cfs_bandwidth;
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(tg, &task_groups, list) {
> +		struct cfs_bandwidth *cfs_b = &tg->cfs_bandwidth;
> +		struct cfs_rq *cfs_rq = tg->cfs_rq[cpu_of(rq)];
> +
> +		if (!cfs_rq->online)
> +			continue;
>  
>  		raw_spin_lock(&cfs_b->lock);
>  		cfs_rq->runtime_enabled = cfs_b->quota != RUNTIME_INF;
>  		raw_spin_unlock(&cfs_b->lock);
>  	}
> +	rcu_read_unlock();
>  }
>  
>  static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
>  {
> -	struct cfs_rq *cfs_rq;
> +	struct task_group *tg;
>  
> -	for_each_leaf_cfs_rq(rq, cfs_rq) {
> -		if (!cfs_rq->runtime_enabled)
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(tg, &task_groups, list) {
> +		struct cfs_rq *cfs_rq = tg->cfs_rq[cpu_of(rq)];
> +
> +		if (!cfs_rq->online || !cfs_rq->runtime_enabled)
>  			continue;
>  
>  		/*
> @@ -4677,6 +4686,7 @@ static void __maybe_unused unthrottle_of
>  		if (cfs_rq_throttled(cfs_rq))
>  			unthrottle_cfs_rq(cfs_rq);
>  	}
> +	rcu_read_unlock();
>  }

Note that both these are called with rq->lock held. I don't think we
need to actually add rcu_read_lock() here, as it will fully serialize
against your ->online = 0 that holds rq->lock.

Also, arguably you can keep using for_each_leaf_cfs_rq() for unthrottle,
because I don't think we can (should?) remove a throttled group from the
leaf list -- its not empty after all.

Then again, this is CPU hotplug code and nobody cares about its
performance much, and its better to be safe than sorry, so yes, use
task_groups list to make sure to reach all groups.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] sched/fair: Use task_groups instead of leaf_cfs_rq_list  to walk all cfs_rqs Tejun Heo <tj@kernel.org> - 2017-04-26 02:50 +0200
  Re: [PATCH 1/2] sched/fair: Use task_groups instead of  leaf_cfs_rq_list to walk all cfs_rqs Peter Zijlstra <peterz@infradead.org> - 2017-05-01 18:30 +0200
  Re: [PATCH 1/2] sched/fair: Use task_groups instead of  leaf_cfs_rq_list to walk all cfs_rqs Peter Zijlstra <peterz@infradead.org> - 2017-05-01 19:10 +0200
    Re: [PATCH 1/2] sched/fair: Use task_groups instead of  leaf_cfs_rq_list to walk all cfs_rqs Peter Zijlstra <peterz@infradead.org> - 2017-05-01 19:10 +0200
      Re: [PATCH 1/2] sched/fair: Use task_groups instead of  leaf_cfs_rq_list to walk all cfs_rqs Tejun Heo <tj@kernel.org> - 2017-05-01 21:10 +0200
  Re: [PATCH 1/2] sched/fair: Use task_groups instead of  leaf_cfs_rq_list to walk all cfs_rqs Peter Zijlstra <peterz@infradead.org> - 2017-05-04 15:40 +0200
    Re: [PATCH 1/2] sched/fair: Use task_groups instead of  leaf_cfs_rq_list to walk all cfs_rqs Tejun Heo <tj@kernel.org> - 2017-05-04 19:40 +0200

csiph-web