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 58C0A20B80D for ; Fri, 10 Jan 2025 11:57:26 +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=1736510250; cv=none; b=OQjN+BBbMkx5B2hTcptPY9ni/KDfESN+HijczNTEUeK8u+u8SmtBLATArFWjiks8qHEp4FRKWTQwHPzTcGYQALE+jVrbiOs1LembnLvr1Mk+CMpZS3F3S7mu34OQkEsR/CdCeNR+cFjz7guy9rLV/tkVA6DNl/F6PQxu2ASaw78= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736510250; c=relaxed/simple; bh=XPBFInmyGTryLZZ0YzY0K5RbNhs/ClG1CLQWBml9lIk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=r7hwkrSEPMcpeESZWl9rjS/6P14Bl2k/YjvqL5SP6sueQ6UCuje9xEqUeE+Kb/rB3JRUpCG5rW89BzpIAK28QMRwLzKm2f4wmew9vPm9yWRFnf0lrb+IoArwJ6ormKexXO7CIskpR7RaRhl4oWQzVd9zmBq9XBJ2Y4hejS8yHBk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=ZpVF0gnM; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none 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="ZpVF0gnM" 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=hqT770SO6aBEeQz7B9BgXReUBwpWxIG3PW9NjXOE7WQ=; b=ZpVF0gnMpR6fBVG0JdO4rppKeA KYmn4O/yVqYOLr78vkT9UWZ1eqg+zplI23E2/WsKP/fuuJmWaFoquebrC9vNRw8sXHcsByyS5HNgH aaj5uFqYZ20ZHilvlgNPbTiPafXSJw5POTCcODMx8TNAqwZ6p6ALjHqGbByfY9rQ167FjjyobvFf+ TdglQJYcreNoR8hZnm2v7H2aL3NkUVVxWtGgbslXLRuSFV65lEJoO7SP/kRGUPSAjvsKs/gxpFDSU NPxFPgHA83zq+vrSF68cLlJEPTn4Ts0tGpL4LQ1ECUHGDseKcPHUnZbl/4DUl7hi/LG2mW24+Ay1O hMxk25Aw==; Received: from 77-249-17-89.cable.dynamic.v4.ziggo.nl ([77.249.17.89] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.98 #2 (Red Hat Linux)) id 1tWDdh-00000009khJ-3Vnw; Fri, 10 Jan 2025 11:57:22 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id F05A730063F; Fri, 10 Jan 2025 12:57:20 +0100 (CET) Date: Fri, 10 Jan 2025 12:57:20 +0100 From: Peter Zijlstra To: Doug Smythies Cc: linux-kernel@vger.kernel.org, vincent.guittot@linaro.org, 'Ingo Molnar' , wuyun.abel@bytedance.com Subject: Re: [REGRESSION] Re: [PATCH 00/24] Complete EEVDF Message-ID: <20250110115720.GA17405@noisy.programming.kicks-ass.net> References: <20250106165932.GG20870@noisy.programming.kicks-ass.net> <20250106170455.GB22191@noisy.programming.kicks-ass.net> <001b01db608a$56d3dc40$047b94c0$@telus.net> <20250107112606.GN20870@noisy.programming.kicks-ass.net> <20250107192340.GB36003@noisy.programming.kicks-ass.net> <001501db618c$67bd8170$37388450$@telus.net> <20250108131205.GO20870@noisy.programming.kicks-ass.net> <001b01db61e4$c3ce5a40$4b6b0ec0$@telus.net> <20250109105959.GA2981@noisy.programming.kicks-ass.net> <002f01db631d$d265a600$7730f200$@telus.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: <002f01db631d$d265a600$7730f200$@telus.net> On Thu, Jan 09, 2025 at 09:09:26PM -0800, Doug Smythies wrote: > Hi Peter, > > Thanks for all your hard work on this. > > On 2025.01.09 03:00 Peter Zijlstra wrote: > > ... > > > This made me have a very hard look at reweight_entity(), and > > specifically the ->on_rq case, which is more prominent with > > DELAY_DEQUEUE. > > > > And indeed, it is all sorts of broken. While the computation of the new > > lag is correct, the computation for the new vruntime, using the new lag > > is broken for it does not consider the logic set out in place_entity(). > > > > With the below patch, I now see things like: > > > > migration/12-55 [012] d..3. 309.006650: reweight_entity: (ffff8881e0e6f600-ffff88885f235f40-12) > > { weight: 977582 avg_vruntime: 4860513347366 vruntime: 4860513347908 (-542) deadline: 4860516552475 > } -> > > { weight: 2 avg_vruntime: 4860528915984 vruntime: 4860793840706 (-264924722) deadline: 6427157349203 > } > > migration/14-62 [014] d..3. 309.006698: reweight_entity: (ffff8881e0e6cc00-ffff88885f3b5f40-15) > > { weight: 2 avg_vruntime: 4874472992283 vruntime: 4939833828823 (-65360836540) deadline: > 6316614641111 } -> > > { weight: 967149 avg_vruntime: 4874217684324 vruntime: 4874217688559 (-4235) deadline: 4874220535650 > } > > > > Which isn't perfect yet, but much closer. > > Agreed. > I tested the patch. Attached is a repeat of a graph I had sent before, with different y axis scale and old data deleted. > It still compares to the "b12" kernel (the last good one in the kernel bisection). > It was a 2 hour and 31 minute duration test, and the maximum CPU migration time was 24 milliseconds, > verses 6 seconds without the patch. Progress! > I left things running for many hours and will let it continue overnight. > There seems to have been an issue at one spot in time: Right, so by happy accident I also left mine running overnight and I think I caught one of those weird spikes. It took a bit of staring to figure out what went wrong this time, but what I now think is the thing that sets off the chain of fail is a combination of DELAY_DEQUEUE and the cgroup reweight stuff. Specifically, when a cgroup's CPU queue becomes empty, calc_group_shares() will drop its weight to the floor. Now, normally, when a queue goes empty, it gets dequeued from its parent and its weight is immaterial. However, with DELAY_DEQUEUE the thing will stick around for a while -- at a very low weight. What makes this esp. troublesome is that even for an active cgroup (like the 'yes' group) the per-cpu weight will be relatively small (~1/nr_cpus like). Worse, the avg_vruntime thing uses load_scale_down() because u64 just isn't all that big :/ (if only all 64bit machines could do 128bit divisions as cheaply as x86_64) This means that on a 16 CPU machine, the weight of a 'normal' all busy queue will be 64, and the weight of an empty queue will be 2, which means the effect of the ginormous lag on the avg_vruntime is fairly significant, pushing it wildly off balance and affecting placement of new tasks. Also, this violates the spirit of DELAY_DEQUEUE, that wants to continue competition as the entity was. As such, we should not adjust the weight of an empty queue. I've started a new run, and some 15 minutes of runtime show nothing interesting, I'll have to let it run for a while again. --- diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 3e9ca38512de..93644b3983d4 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4065,7 +4019,11 @@ static void update_cfs_group(struct sched_entity *se) struct cfs_rq *gcfs_rq = group_cfs_rq(se); long shares; - if (!gcfs_rq) + /* + * When a group becomes empty, preserve its weight. This matters for + * DELAY_DEQUEUE. + */ + if (!gcfs_rq || !gcfs_rq->load.weight) return; if (throttled_hierarchy(gcfs_rq))