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>]

Hello,

On Mon, May 01, 2017 at 07:02:41PM +0200, Peter Zijlstra wrote:
> 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

So, the rcu_read_lock() thing protects against sched_free_group() and
thanks to the order of operations, all online cfs_rq's are guaranteed
to be visbile in the two callbacks; however, nothing prevents the code
paths from seeing already dead cfs_rqs, which *may* be okay if the
code paths are safe to run on dead and unlinked cfs_rqs, but it's
still nasty and fragile.

The new ->online condition which is synchronized by rq->lock
guarantees that both functions only process live ones.  We need
something synchronized by rq->lock to guarantee this whether that's a
new list entry or a flag like ->online.  It'd be better to encapsulate
and document this iteration.

Thanks.

-- 
tejun