linux-kernel - Re: [PATCH 1/2] sched/fair: Use task_groups instead of leaf_cfs_rq_list to walk all cfs

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]

Message-ID: <20170501170241.3qjckawnvwzktsdp@hirez.programming.kicks-ass.net>
Date:   Mon, 1 May 2017 19:02:41 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Tejun Heo <tj@...nel.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        “linux-kernel@...r.kernel.org” 
        <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mike Galbraith <efault@....de>, Paul Turner <pjt@...gle.com>,
        Chris Mason <clm@...com>,
        “kernel-team@...com” <kernel-team@...com>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>
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