From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ptr1337.dev (mail.ptr1337.dev [202.61.224.105]) (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 67690355F24 for ; Thu, 26 Feb 2026 17:43:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.61.224.105 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772127806; cv=none; b=j68ZtT2jXc1mKDJCFFIhSA2K77URLjwlK2rywIDTxK4c5hk/LUaKXuXp3E0uKWT4nlcEI4xncywzVva4TezYpn2fwcb1jqBv2CKRKtg7TuxiuoS0Q/lfwXKkgdq/0q8vx6y+ytxdhD+EeoFDlhhKsf3phjlVEAeGnJMD0nCVaFU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772127806; c=relaxed/simple; bh=MOPHlI+U92pUwoQTyyVymAa72XtQIL3DZpyfsihjdE8=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=mErLHhH03yoeHtInAwqsvGqtLal1IH2x9EC+8zDJR1WaNF8F2FuJFPsptZrAz5l9VezpNMt68xT+Hpz/qlfkN7XupomK0jp0XdBZHtMpJL1SyI0v995nwhzJtNf6IwdK5ZAqQtW5a7lV72gnoTxVnFqRTtxkxB0pdghqDBphkUs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=cachyos.org; spf=pass smtp.mailfrom=cachyos.org; dkim=pass (2048-bit key) header.d=cachyos.org header.i=@cachyos.org header.b=JaPOXEHB; arc=none smtp.client-ip=202.61.224.105 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=cachyos.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=cachyos.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=cachyos.org header.i=@cachyos.org header.b="JaPOXEHB" Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id B02C2285D2B; Thu, 26 Feb 2026 18:33:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cachyos.org; s=dkim; t=1772127190; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=HSwrkssGQ2GsT5UdJWCQW0JylWsIV5AhIw0eHcvh1Tg=; b=JaPOXEHBTzizgUtqlzvjZ2SCje9ucGeKyFEESi5Yl//YHQMcmC3cHiVYS/9vFFErRHd/j1 oHLriWWvIDtngJmHrEVPuFJYAWa9ICoqxSU+a8bfJHS4Hrka7rMq4HtJftTOnc1R2VpC4a b8RA3UD2MMycW69jKwmr4PqtTqN2jQ/JDFFO95t3M8wSiwSB9cPQbUcu4CsWpPOSOkpqs8 nMsgYf3dmt8IVd7/nrPxgVLmz0WyI9/KB8Fk6lESo2drXG2Du0Gn7A3h25OL84iJvAZDTB A5uQjAHJrbZX2qWKVfNMnp56s7xjejRW8Ofed8WlEqo95FWjA1i7BFaNaek7QQ== Message-ID: Date: Thu, 26 Feb 2026 17:32:00 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH v5] sched/fair: do not scan twice in detach_tasks() To: Huang Shijie , mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com, vincent.guittot@linaro.org, vschneid@redhat.com Cc: patches@amperecomputing.com, cl@linux.com, Shubhang@os.amperecomputing.com, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, linux-kernel@vger.kernel.org References: <20250722102600.25976-1-shijie@os.amperecomputing.com> From: Eric Naim In-Reply-To: <20250722102600.25976-1-shijie@os.amperecomputing.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Last-TLS-Session-Version: TLSv1.3 On 7/22/25 6:26 PM, Huang Shijie wrote: > detach_tasks() uses struct lb_env.loop_max as an env.src_rq->cfs_tasks > iteration count limit. It is however set without the source RQ lock held. > > This means that env.loop_max and the actual length of env.src_rq->cfs_tasks > as observed within detach_tasks() can differ. This can cause some tasks to > be unnecessarily iterated over more than once, for instance: > > env.loop_max := 4 > detach_tasks() > // Here env.src->cfs_tasks only contains two tasks which can't be > // migrated anywhere, so they're put back in the list each time. > env.src->cfs_tasks := [p1, p0] > // The iteration goes: > p0; cfs_tasks := [p0, p1] > p1; cfs_tasks := [p1, p0] > p0; cfs_tasks := [p0, p1] > p1; cfs_tasks := [p1, p0] > > // IOW we iterate over each task twice > > In the Specjbb test, the similar issues can be caught many times. > (Over 330,000 times in a 30-minites Specjbb test) > > This patch sets env.loop_max only once RQ lock is taken, > and uses busiest->cfs.h_nr_queued for setting the env.loop_max. > > After this patch, I cannot catch any above issue in the Specjbb test. > > Signed-off-by: Huang Shijie > --- > v4 --> v5: > Set the env.loop_max once the rq lock is taken. > v4:https://lore.kernel.org/all/20250721023939.19703-1-shijie@os.amperecomputing.com/ > > v3 --> v4: > Changed the commit message suggested by Valentin Schneider. > v3: https://lore.kernel.org/all/20250718063523.9232-1-shijie@os.amperecomputing.com/ > > v2 --> v3: > Fix a typo in the commit message. > v2: https://lore.kernel.org/all/20250718054709.8781-1-shijie@os.amperecomputing.com/ > > v1 --> v2: > Add more comment from Valentin Schneider > v1: https://lore.kernel.org/all/20250707083636.38380-1-shijie@os.amperecomputing.com/ > --- > kernel/sched/fair.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 7cc9d50e3e11..9c1f21d59b5c 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -11708,12 +11708,15 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq, > * still unbalanced. ld_moved simply stays zero, so it is > * correctly treated as an imbalance. > */ > - env.loop_max = min(sysctl_sched_nr_migrate, busiest->nr_running); > - > more_balance: > rq_lock_irqsave(busiest, &rf); > update_rq_clock(busiest); > > + if (!env.loop_max) > + env.loop_max = min(sysctl_sched_nr_migrate, busiest->cfs.h_nr_queued); > + else > + env.loop_max = min(env.loop_max, busiest->cfs.h_nr_queued); > + > /* > * cur_ld_moved - load moved in current iteration > * ld_moved - cumulative load moved across iterations Hi maintainers, this patch seems to have gotten lost in the mailbox, are there any plans on picking it up? -- Regards, Eric