From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750918AbdEATHx (ORCPT ); Mon, 1 May 2017 15:07:53 -0400 Received: from mail-yw0-f179.google.com ([209.85.161.179]:36828 "EHLO mail-yw0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750733AbdEATHt (ORCPT ); Mon, 1 May 2017 15:07:49 -0400 Date: Mon, 1 May 2017 15:07:47 -0400 From: Tejun Heo To: Peter Zijlstra Cc: Ingo Molnar , =?utf-8?B?4oCcbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZ+KAnQ==?= , Linus Torvalds , Mike Galbraith , Paul Turner , Chris Mason , =?utf-8?B?4oCca2VybmVsLXRlYW1AZmIuY29t4oCd?= , Paul McKenney Subject: Re: [PATCH 1/2] sched/fair: Use task_groups instead of leaf_cfs_rq_list to walk all cfs_rqs Message-ID: <20170501190747.GC8921@htj.duckdns.org> References: <20170426004039.GA3222@wtj.duckdns.org> <20170501165939.ujfhr5oi35bozsnr@hirez.programming.kicks-ass.net> <20170501170241.3qjckawnvwzktsdp@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170501170241.3qjckawnvwzktsdp@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.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