Messages in this thread | | | Date | Mon, 1 May 2017 19:02:41 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/2] sched/fair: Use task_groups instead of leaf_cfs_rq_list to walk all cfs_rqs |
| |
On Mon, May 01, 2017 at 06:59:39PM +0200, Peter Zijlstra wrote: > On Tue, Apr 25, 2017 at 05:40:39PM -0700, Tejun Heo wrote: > > @@ -9355,24 +9366,18 @@ void online_fair_sched_group(struct task > > void unregister_fair_sched_group(struct task_group *tg) > > { > > unsigned long flags; > > - struct rq *rq; > > int cpu; > > > > for_each_possible_cpu(cpu) { > > + struct rq *rq = cpu_rq(cpu); > > + struct cfs_rq *cfs_rq = tg->cfs_rq[cpu]; > > + > > if (tg->se[cpu]) > > remove_entity_load_avg(tg->se[cpu]); > > > > - /* > > - * Only empty task groups can be destroyed; so we can speculatively > > - * check on_list without danger of it being re-added. > > - */ > > - if (!tg->cfs_rq[cpu]->on_list) > > - continue; > > - > > - rq = cpu_rq(cpu); > > - > > raw_spin_lock_irqsave(&rq->lock, flags); > > - list_del_leaf_cfs_rq(tg->cfs_rq[cpu]); > > + list_del_leaf_cfs_rq(cfs_rq); > > + cfs_rq->online = 0; > > raw_spin_unlock_irqrestore(&rq->lock, flags); > > } > > } > > It would be nice to be able to retain that on_list check and avoid > taking all those rq->lock's. > > Since per the previous mail, those online/offline loops are protected by > rq->lock, all we need is to ensure an rcu_sched GP passes between here > and sched_free_group(). > > Which I think we can do differently, see below. Then we don't need the > ->online thing and can keep using the ->on_list check.
n/m, I need to stop staring at a screen. Wrapping those two sites in rcu_read_lock() achieves the very same.
So we want the rcu_read_lock() to serialize against sched_free_group, but don't need the new ->online thing and can retain the ->on_list stuff. Or I've completely lost the plot (which is entirely possible...)
I'll stare at this again tomorrow
| |