Messages in this thread | | | Date | Mon, 1 May 2017 15:07:47 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH 1/2] sched/fair: Use task_groups instead of leaf_cfs_rq_list to walk all cfs_rqs |
| |
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
| |