From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2A3DC6379D for ; Thu, 26 Nov 2020 10:18:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6E83B21741 for ; Thu, 26 Nov 2020 10:18:19 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="nnymFjQm" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387672AbgKZKSC (ORCPT ); Thu, 26 Nov 2020 05:18:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55174 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726099AbgKZKSB (ORCPT ); Thu, 26 Nov 2020 05:18:01 -0500 Received: from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com [IPv6:2a00:1450:4864:20::22c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B561C0613D4 for ; Thu, 26 Nov 2020 02:18:01 -0800 (PST) Received: by mail-lj1-x22c.google.com with SMTP id s9so1710901ljo.11 for ; Thu, 26 Nov 2020 02:18:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=F3gSDfUsKRUY3FZX1HOALC3kUdm90Fc5VLcRsSCj8q8=; b=nnymFjQm/mN5O96fAZhiLaPH+OGvEJvz1VHlRqbEinYSBTvXnZkVwKrixyYtBJpFlS kqTM3h+dkgoBwHFIoAqXypp1t9SjuJS7cZrowMZ9RzzbPHDMSIqvL0QvOTbXxkXLdYRx WOMwGRDYQx8ch+Zwj/SumPp7/nFz/aYjcvh4uKNhPuGxOC9EViOruQJfd70HCrfOXxqI fSkFtkVJ867xiNKI8etNYZDRpTvOiwjj2A75SFoB8ZzYVhPN3L65P+X6OshhNBP20+1J H5GUmHDhoQV6Es7Eh2ydxN5qSEumiK9WrYEV6DRMPC2kJSRquJdlZnqNjKjtdlLI3Nq2 o04w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=F3gSDfUsKRUY3FZX1HOALC3kUdm90Fc5VLcRsSCj8q8=; b=c2TrZAD+WQq1Msqe/5jN/D8BNsF+rVJTm2K7ARlqQJAHEsGyYdcCTVOReU7DhKKuLJ 3pV7UzA7WW7wqPKiUCb7fFl6Tt/jlyknooaqKG8e2DmdxqpraQgnK42xn0zv9XLdloh1 IePTkMDSYl2W2lGEcMbdKJnsx4qaJht6FT9KVX13fw4nL9ZgBtS4AZSSmsc4PzzrueIc zc81PHlifACFcZC+buGNdj8WBsvNrKSZssqV6HBLDE904iMBn8ycf3FeiaNdNGUpBVsE MaswArVDV1bznCEKFx+QNy0rdnsMXUCAKmiRzh6hFZYaHIV6/bsZzsMzMGm0cBo0TIq+ pa2A== X-Gm-Message-State: AOAM533XoiUkNngAG0yLgD63jP+6ZyggqYvomS37fCk68vRB/5iBbI0q G/CLKy+/WtmhL1Paks+ycCmDBg5WJ49tQIjlxS8Xpw== X-Google-Smtp-Source: ABdhPJy0r2Z5FCyd6we23GNdP82wO1uZf80pvaDk+2eQ4HsDD04AokZ/BjTZ9xPYNyvXA6xtmN9JOXxCDznoBLYUKz0= X-Received: by 2002:a2e:8050:: with SMTP id p16mr1024017ljg.69.1606385879690; Thu, 26 Nov 2020 02:17:59 -0800 (PST) MIME-Version: 1.0 References: <20201117232003.3580179-1-joel@joelfernandes.org> <20201117232003.3580179-3-joel@joelfernandes.org> <20201126090710.GF2414@hirez.programming.kicks-ass.net> In-Reply-To: <20201126090710.GF2414@hirez.programming.kicks-ass.net> From: Vincent Guittot Date: Thu, 26 Nov 2020 11:17:48 +0100 Message-ID: Subject: Re: [PATCH -tip 02/32] sched: Introduce sched_class::pick_task() To: Peter Zijlstra Cc: "Joel Fernandes (Google)" , Nishanth Aravamudan , Julien Desfossez , Tim Chen , Vineeth Pillai , Aaron Lu , Aubrey Li , Thomas Gleixner , linux-kernel , Ingo Molnar , Linus Torvalds , Frederic Weisbecker , Kees Cook , Greg Kerr , Phil Auld , Valentin Schneider , Mel Gorman , Pawan Gupta , Paolo Bonzini , vineeth@bitbyteword.org, Chen Yu , Christian Brauner , Agata Gruza , Antonio Gomez Iglesias , graf@amazon.com, konrad.wilk@oracle.com, dfaggioli@suse.com, Paul Turner , Steven Rostedt , Patrick Bellasi , Jiang Biao , Alexandre Chartre , James Bottomley , OWeisse@umich.edu, Dhaval Giani , Junaid Shahid , jsbarnes@google.com, "Hyser,Chris" , Ben Segall , Josh Don , Hao Luo , Tom Lendacky , Aubrey Li , "Paul E. McKenney" , Tim Chen Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 26 Nov 2020 at 10:07, Peter Zijlstra wrote: > > On Wed, Nov 25, 2020 at 05:28:36PM +0100, Vincent Guittot wrote: > > On Wed, 18 Nov 2020 at 00:20, Joel Fernandes (Google) > > > > +#ifdef CONFIG_SMP > > > +static struct task_struct *pick_task_fair(struct rq *rq) > > > +{ > > > + struct cfs_rq *cfs_rq = &rq->cfs; > > > + struct sched_entity *se; > > > + > > > + if (!cfs_rq->nr_running) > > > + return NULL; > > > + > > > + do { > > > + struct sched_entity *curr = cfs_rq->curr; > > > + > > > + se = pick_next_entity(cfs_rq, NULL); > > > > Calling pick_next_entity clears buddies. This is fine without > > coresched because the se will be the next one. But calling > > pick_task_fair doesn't mean that the se will be used > > Urgh, nice one :/ > > > > + > > > + if (curr) { > > > + if (se && curr->on_rq) > > > + update_curr(cfs_rq); > > > + > > > > Shouldn't you check if cfs_rq is throttled ? > > Hmm,... I suppose we do. > > > > + if (!se || entity_before(curr, se)) > > > + se = curr; > > > + } > > > + > > > + cfs_rq = group_cfs_rq(se); > > > + } while (cfs_rq); > > > + > > > + return task_of(se); > > > +} > > > +#endif > > Something like so then? yes. it seems ok > > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -4354,6 +4354,8 @@ check_preempt_tick(struct cfs_rq *cfs_rq > static void > set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se) > { > + clear_buddies(cfs_rq, se); > + > /* 'current' is not kept within the tree. */ > if (se->on_rq) { > /* > @@ -4440,8 +4442,6 @@ pick_next_entity(struct cfs_rq *cfs_rq, > se = cfs_rq->last; > } > > - clear_buddies(cfs_rq, se); > - > return se; > } > > @@ -6982,20 +6982,29 @@ static void check_preempt_wakeup(struct > #ifdef CONFIG_SMP > static struct task_struct *pick_task_fair(struct rq *rq) > { > - struct cfs_rq *cfs_rq = &rq->cfs; > struct sched_entity *se; > - > + struct cfs_rq *cfs_rq; > + > +again: > + cfs_rq = &rq->cfs; > if (!cfs_rq->nr_running) > return NULL; > > do { > struct sched_entity *curr = cfs_rq->curr; > > - if (curr && curr->on_rq) > - update_curr(cfs_rq); > + /* When we pick for a remote RQ, we'll not have done put_prev_entity() */ > + if (curr) { > + if (curr->on_rq) > + update_curr(cfs_rq); > + else > + curr = NULL; > > - se = pick_next_entity(cfs_rq, curr); > + if (unlikely(check_cfs_rq_runtime(cfs_rq))) > + goto again; > + } > > + se = pick_next_entity(cfs_rq, curr); > cfs_rq = group_cfs_rq(se); > } while (cfs_rq); >