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 F309D477987 for ; Wed, 19 Aug 2026 14:37:09 +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=1787150236; cv=none; b=PWYc+4/b/Fkn+UFTy2Hl+Ar55bJcGZ5fNONKVcDYvHqa5RIWiztFsQ5qtHNxcbyJ4aEZq4iDF5m2cu0NldWoVLyF2I14LrH0N62xASkjOiH739gqtcHstWsvOlg07DbLW9yFbAeFFXC8PJif3smNn24TG6uK9fzZzjC2SiQoYzM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787150236; c=relaxed/simple; bh=sfmBvPcQyuJLXsULaC8WNzaOGomd+FtfjCTIrOqQxIs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Jh7UfWiuX3MhrT/639bZz2UhqE7TAfqDzO4UnBP0IyjMveS0OA2mfNq7H8adttH4wHNBTGonXPhPx3KRPJqN1aPlFJpwPM3uN/cUDaTi+kMhLiRfFdR4TH6n3bh1tcLQePDrZScWorAUo4Js/buEfqu9pesJrFLnmuDeP+Fu/aw= 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=a5esvSsN; 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="a5esvSsN" 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=+rMNGRQEgyJluFscpVb8oOQd7diyTfCnlAdMSZZJkpc=; b=a5esvSsNc5WBDx2Tqv33p0oxM7 3RnfiP0TxOEd6tBsJZ6vi9JMr/ZVFNLOfeUHxR9YiTHUSKX/kwGpl5hgJ0V9UMLRFuyXrJPmE4kbM 1D0AUstSzcXT65EmFQ5EowQTwwhy3T1Lh3hFVncxCQu6/tJbZWpjDtJ97D/H4DTy798aOuJukSjd5 yIIjwJ8bEo/bnJY+bV3tHRHSQ3cguugOwuXxR2ns8QvBLca6B0SB9wdTKVHPJ8OBX8LQvzcY30PvS Sz1Di2hMdrqgVjjw+N5WoSqMvMEt86cAnXLKGqooV92jjLQ3DaivAeSY93HqqXZVGF4JOO6U24Kwu yZp+6ceQ==; 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 1wwhPP-00000002TV8-2gTy; Wed, 19 Aug 2026 14:36:51 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 0414030030F; Wed, 19 Aug 2026 16:36:50 +0200 (CEST) Date: Wed, 19 Aug 2026 16:36:49 +0200 From: Peter Zijlstra To: Aaron Lu Cc: mingo@kernel.org, linux-kernel@vger.kernel.org, juri.lelli@redhat.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com, kprateek.nayak@amd.com, tj@kernel.org, williams@redhat.com, jkacur@redhat.com Subject: Re: [PATCH 0/2] sched: Remove sched_class::balance() Message-ID: <20260819143649.GB1248307@noisy.programming.kicks-ass.net> References: <20260624121327.190063948@infradead.org> <20260702114919.GA186418@bytedance.com> <20260819075830.GF1246887@noisy.programming.kicks-ass.net> 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: <20260819075830.GF1246887@noisy.programming.kicks-ass.net> On Wed, Aug 19, 2026 at 09:58:30AM +0200, Peter Zijlstra wrote: > On Thu, Jul 02, 2026 at 07:49:19PM +0800, Aaron Lu wrote: > > > Assume cpuX and cpuY are siblings, it appears the following happened: > > > > cpuX cpuY > > > > pick_next_task() > > goto restart_multi > > > > rqX->core_pick = pick_task(rqX) > > > > pick_task(rqY) > > pick_task_fair(rqY) > > sched_balance_newidle(rqY) > > raw_spin_rq_unlock(rqY) // drops core lock > > > > pick_next_task() > > goto restart_multi > > rqY->core_pick = pick_task(rqY) > > rqX->core_pick = pick_task(rqX) > > > > if (rqX->curr == rqX->core_pick) > > rqX->core_pick = NULL > > > > UNLOCK rq_lockp(rqY) > > > > raw_spin_rq_lock(rqY) > > > > rqY->core_pick = pick_task(rqY) > > > > p = rqX->core_pick // NULL > > cookie_equals(p, cookie) // NULL deref > > Well, damn :/ That's a nice race. So while we did a lock-break, it does > not trigger RETRY_TASK and continues. > > The 'easy' fix is taking a local copy of core_task_seq when we increase > it for the pick, and double checking that is still valid at the end and > then restarting if not. > > Except that is susceptible to live-locks. It doesn't have forward > progress guarantees. For that we need to limit the amount of > lock-breaks/newidle invocations. So TJ did something like that for ext. I'm not entirely sure I get his argument on forward progress though. But the simple thing is something like so, which I think also allows simplifying ext some. --- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 8070a347cf3b..ce8bb5036fe1 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -6232,14 +6232,14 @@ static struct task_struct * pick_next_task(struct rq *rq, struct rq_flags *rf) __must_hold(__rq_lockp(rq)) { + bool core_clock_updated = (rq == rq->core); struct task_struct *next, *p, *max; const struct cpumask *smt_mask; + int i, cpu, seq, occ = 0; bool fi_before = false; - bool core_clock_updated = (rq == rq->core); - unsigned long cookie; - int i, cpu, occ = 0; - struct rq *rq_i; bool need_sync = false; + unsigned long cookie; + struct rq *rq_i; if (!sched_core_enabled(rq)) return __pick_next_task(rq, rf); @@ -6314,7 +6314,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf) * However, preemptions can cause multiple picks on the same task set. * 'Fix' this by also increasing @task_seq for every pick. */ - rq->core->core_task_seq++; + seq = ++rq->core->core_task_seq; /* * Optimize for common case where this CPU has no cookies @@ -6362,7 +6362,8 @@ pick_next_task(struct rq *rq, struct rq_flags *rf) update_rq_clock(rq_i); p = pick_task(rq_i, rf); - if (unlikely(p == RETRY_TASK)) { + if (unlikely(seq != rq->core->core_task_seq || + WARN_ON_ONCE(p == RETRY_TASK))) { /* rq lock may have been dropped, clocks invalidated */ core_clock_updated = false; if (!(rq->clock_update_flags & RQCF_UPDATED)) @@ -6392,7 +6393,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf) if (cookie) p = sched_core_find(rq_i, cookie); if (!p) - p = idle_sched_class.pick_task(rq_i, rf); + p = idle_sched_class.pick_task(rq_i, NULL); } rq_i->core_pick = p;