From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BC61D392C21 for ; Thu, 18 Jun 2026 10:56:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.92.199 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781780206; cv=none; b=FCzkW+Cf4vnBGNWx52p7FQvQNi7D+si3/vSzt0btEtC/hfExOI+DLjOkmKr7oPmkmkZ7tYPXEcyWXn/etG/cnMl8dv+CzqjrhrkrPGNwBsa8RqIpUcEL2Tv/AXggxoah/1yjKAIn8lc3dZ8tTth20WWNvkGT40dlXbLxP4JKKdA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781780206; c=relaxed/simple; bh=2pJsTGNA+i/ab7LpVvM8Rh7p87lOYvNhKQofDzqF8Ak=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YCqHz7HbHv4LTvdsN8JcZj2/CpMUWq9EF/ZRhPJL1pCeSlE9GePPqw+Dikw/Uq7EPvrPyaMOKcMWw83B1zj8LVYBQXSt21C1gl2shFIXO/vfCSHVBa686qcDgBbeTYU8fwKtRIWbRgaQWsAJ8e9AsFa8Nus0ngIjY/GE0xIieC8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=pass smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=RgLslydv; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="RgLslydv" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=8X7wA8ToOk8/i6eB5km8kuuvlR/XgTK9SJsn4ZBWRu0=; b=RgLslydv+T6UXZfR5isbJjWK/5 UxTVIEJ9iYOQRwaUw5dBtRno2CAdBbOLvn7f8uVW6kqLCBjc/v3bSdkB6MKKaJtnDFV+azTnOMMAs MDvFllzeCrjb9wkAIMh/kOpOnyDZX88auEOu/hheJq7TEfMwiPuqIr8wD++xrvEnawuNw9k2bX8ad 2XR7UGdCMOFrp2Yhmo0fjr4zr+RemSo1R7FtvYJ4reQgf3NAPOInbrruq6vxMab7c32Ex1Urtbk74 SIRPxGz2Zo9SCZW2qSi4h9KYGEyknr7KXGagubWom+EcKcWiOUewDbKPujCRNFEf7e5cBzD9BWjqU DH9etbTg==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.99.2 #2 (Red Hat Linux)) id 1waAQ9-0000000DEGW-0NfF; Thu, 18 Jun 2026 10:56:29 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 7E4A4300339; Thu, 18 Jun 2026 12:56:27 +0200 (CEST) Date: Thu, 18 Jun 2026 12:56:27 +0200 From: Peter Zijlstra To: Xin Zhao Cc: vschneid@redhat.com, mingo@redhat.com, juri.lelli@redhat.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, kprateek.nayak@amd.com, pauld@redhat.com, aiqun.yu@oss.qualcomm.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 0/2] sched/fair: Optimize some active balance logic Message-ID: <20260618105627.GP49951@noisy.programming.kicks-ass.net> References: <20260617072151.1173416-1-jackzxcui1989@163.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260617072151.1173416-1-jackzxcui1989@163.com> And since I've been staring at this code far too long, I accidentally did the below cleanup on top. --- Subject: sched/fair: Reflow sched_balance_rq() From: Peter Zijlstra Date: Thu Jun 18 10:51:49 CEST 2026 Reflow to reduce indenting. Signed-off-by: Peter Zijlstra (Intel) --- kernel/sched/fair.c | 136 ++++++++++++++++++++++++--------------------------- kernel/sched/sched.h | 19 ++++++- 2 files changed, 82 insertions(+), 73 deletions(-) --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -13437,82 +13437,78 @@ static int sched_balance_rq(int this_cpu } } - if (!ld_moved) { - schedstat_inc(sd->lb_failed[idle]); + if (ld_moved) { + sd->nr_balance_failed = 0; + goto out_unbalanced; + } + + schedstat_inc(sd->lb_failed[idle]); + /* + * Increment the failure counter only on periodic balance. + * We do not want newidle balance, which can be very + * frequent, pollute the failure counter causing + * excessive cache_hot migrations and active balances. + * + * Similarly for migration_misfit which is not related to + * load/util migration, don't pollute nr_balance_failed. + * + * The same for cache aware scheduling's allowance for + * load imbalance. If regular load balance does not + * migrate task due to LLC locality, it is a expected + * behavior and don't pollute nr_balance_failed. + * See can_migrate_task(). + */ + if (idle != CPU_NEWLY_IDLE && + env.migration_type != migrate_misfit && + !(env.flags & LBF_LLC_PINNED)) + sd->nr_balance_failed++; + + if (!need_active_balance(&env)) + goto out_unbalanced; + + scoped_guard (raw_spin_rq_lock_irqsave, busiest) { /* - * Increment the failure counter only on periodic balance. - * We do not want newidle balance, which can be very - * frequent, pollute the failure counter causing - * excessive cache_hot migrations and active balances. - * - * Similarly for migration_misfit which is not related to - * load/util migration, don't pollute nr_balance_failed. - * - * The same for cache aware scheduling's allowance for - * load imbalance. If regular load balance does not - * migrate task due to LLC locality, it is a expected - * behavior and don't pollute nr_balance_failed. - * See can_migrate_task(). + * Don't kick the active_load_balance_cpu_stop, + * if the curr task on busiest CPU can't be + * moved to this_cpu: */ - if (idle != CPU_NEWLY_IDLE && - env.migration_type != migrate_misfit && - !(env.flags & LBF_LLC_PINNED)) - sd->nr_balance_failed++; - - if (need_active_balance(&env)) { - unsigned long flags; - - raw_spin_rq_lock_irqsave(busiest, flags); - - /* - * Don't kick the active_load_balance_cpu_stop, - * if the curr task on busiest CPU can't be - * moved to this_cpu: - */ - if (!cpumask_test_cpu(this_cpu, busiest->curr->cpus_ptr)) { - raw_spin_rq_unlock_irqrestore(busiest, flags); - goto out_one_pinned; - } - - /* Record that we found at least one task that could run on this_cpu */ - env.flags &= ~LBF_ALL_PINNED; - - /* - * ->active_balance synchronizes accesses to - * ->active_balance_work. Once set, it's cleared - * only after active load balance is finished. - */ - if (busiest->active_balance) - goto no_active_balance; - - /* - * @busiest dropped its rq_lock in the middle of - * scheduling out its ->curr task (->on_rq := 0), no - * need to forcefully punt it away with active balance. - */ - if (!busiest->curr->on_rq) - goto no_active_balance; - - busiest->active_balance = 1; - busiest->push_cpu = this_cpu; - active_balance = 1; -no_active_balance: - preempt_disable(); - raw_spin_rq_unlock_irqrestore(busiest, flags); - if (active_balance) { - stop_one_cpu_nowait(cpu_of(busiest), - active_load_balance_cpu_stop, busiest, - &busiest->active_balance_work); - } - preempt_enable(); - } - } else { - sd->nr_balance_failed = 0; + if (!cpumask_test_cpu(this_cpu, busiest->curr->cpus_ptr)) + goto out_one_pinned; + + /* Record that we found at least one task that could run on this_cpu */ + env.flags &= ~LBF_ALL_PINNED; + + /* + * ->active_balance synchronizes accesses to + * ->active_balance_work. Once set, it's cleared + * only after active load balance is finished. + */ + if (busiest->active_balance) + goto out_unbalanced; + + /* + * @busiest dropped its rq_lock in the middle of + * scheduling out its ->curr task (->on_rq := 0), no + * need to forcefully punt it away with active balance. + */ + if (!busiest->curr->on_rq) + goto out_unbalanced; + + busiest->active_balance = 1; + busiest->push_cpu = this_cpu; + active_balance = 1; + preempt_disable(); } + if (active_balance) { + stop_one_cpu_nowait(cpu_of(busiest), + active_load_balance_cpu_stop, busiest, + &busiest->active_balance_work); + } + preempt_enable(); +out_unbalanced: /* We were unbalanced, so reset the balancing interval */ sd->balance_interval = sd->min_interval; - goto out; out_balanced: --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -2018,7 +2018,8 @@ DEFINE_LOCK_GUARD_1(rq_lock, struct rq, rq_unlock(_T->lock, &_T->rf), struct rq_flags rf) -DECLARE_LOCK_GUARD_1_ATTRS(rq_lock, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T))); +DECLARE_LOCK_GUARD_1_ATTRS(rq_lock, __acquires(__rq_lockp(_T)), + __releases(__rq_lockp(*(struct rq **)_T))); #define class_rq_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock, _T) DEFINE_LOCK_GUARD_1(rq_lock_irq, struct rq, @@ -2026,7 +2027,8 @@ DEFINE_LOCK_GUARD_1(rq_lock_irq, struct rq_unlock_irq(_T->lock, &_T->rf), struct rq_flags rf) -DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irq, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T))); +DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irq, __acquires(__rq_lockp(_T)), + __releases(__rq_lockp(*(struct rq **)_T))); #define class_rq_lock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock_irq, _T) DEFINE_LOCK_GUARD_1(rq_lock_irqsave, struct rq, @@ -2034,9 +2036,20 @@ DEFINE_LOCK_GUARD_1(rq_lock_irqsave, str rq_unlock_irqrestore(_T->lock, &_T->rf), struct rq_flags rf) -DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T))); +DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, __acquires(__rq_lockp(_T)), + __releases(__rq_lockp(*(struct rq **)_T))); #define class_rq_lock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, _T) +DEFINE_LOCK_GUARD_1(raw_spin_rq_lock_irqsave, struct rq, + raw_spin_rq_lock_irqsave(_T->lock, _T->flags), + raw_spin_rq_unlock_irqrestore(_T->lock, _T->flags), + unsigned long flags) + +DECLARE_LOCK_GUARD_1_ATTRS(raw_spin_rq_lock_irqsave, __acquires(__rq_lockp(_T)), + __releases(__rq_lockp(*(struct rq **)_T))); +#define class_raw_spin_rq_lock_irqsave_constructor(_T) \ + WITH_LOCK_GUARD_1_ATTRS(raw_spin_rq_lock_irqsave, _T) + #define this_rq_lock_irq(...) __acquire_ret(_this_rq_lock_irq(__VA_ARGS__), __rq_lockp(__ret)) static inline struct rq *_this_rq_lock_irq(struct rq_flags *rf) __acquires_ret {