From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (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 960533B0584 for ; Wed, 29 Jul 2026 18:29:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.10 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785349772; cv=none; b=Srgr/f1u/SNytpQnbI3GQREo6qih9SRUbedkWY8p6ySTLWrcv5ydNhjNl8KoTg44uyvhMzo5p5MizJMnhtDVZYXtbX3uk6KNrzfqrTh/uUuCOb9vroQEgLVNNxO9H4Cf3x0O2UQU01rI+WA33h3o+SR8RlnZrhC3/tfWl6v6n7g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785349772; c=relaxed/simple; bh=0pUg73M1QXQjgq9HyftBlEZP7W4RryRQwBM/jJt3Cl0=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=HDvPYQEvtZvdVqsYKYWvLGcmH5BNrGogmkIv9SBVMU8O5UOaRVsU/oqsIK997WB83MkE9qaWsJwkeYeDTNSm4ZEOz9F/LD5AiIDDfy+kZECkFSp5EY8FBWqnVRaKiSj+ZZ0wiS7pY83oG6Hg7QoIVlTZsn5t7K7+P4xog7iokdw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=QgY7ECMN; arc=none smtp.client-ip=198.175.65.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="QgY7ECMN" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1785349770; x=1816885770; h=message-id:subject:from:to:cc:date:in-reply-to: references:content-transfer-encoding:mime-version; bh=0pUg73M1QXQjgq9HyftBlEZP7W4RryRQwBM/jJt3Cl0=; b=QgY7ECMNgrqiK5gB5V2KId9kxKdem2VEOyekAUJ3aT4O73V5e80vPg3F 5gPl1ZL2cSXmS44GBES1vj1kVKJa02Xnu+ZN5F42qkc9dIRzL42beIQyH 2PkhFuVxaBZyfffJ8feOLYaeLkUwzKlXALAq67ffbxrxZzkc71rlRDsrX Jatw1U92ReevMDTDrjfZD5FRL/5WBBKT36zvsogVUHP7VacoQl9ELXjuM fIwBFuATx1CtnAApy2hUg+/Y+6C60RvaZhkSt+EocZslNt+qBf8v2t6cZ xRGDkUFmHcZ1grP+If0/6Z7uOqyewKGrPn2K+f669k8y/LeA6/WLlOeHd g==; X-CSE-ConnectionGUID: LLb7plKbQOaLc1yEqWtmIQ== X-CSE-MsgGUID: Hp3XPvadSD6OKsxuV8G1zA== X-IronPort-AV: E=McAfee;i="6800,10657,11859"; a="103369957" X-IronPort-AV: E=Sophos;i="6.25,192,1779174000"; d="scan'208";a="103369957" Received: from orviesa003.jf.intel.com ([10.64.159.143]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Jul 2026 11:29:30 -0700 X-CSE-ConnectionGUID: Kmc0+WfCSryBDpl0jj75Ww== X-CSE-MsgGUID: fyobZacAQ4+0Cb7jBVvbbA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,193,1779174000"; d="scan'208";a="263574581" Received: from unknown (HELO [10.241.243.185]) ([10.241.243.185]) by ORVIESA003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Jul 2026 11:29:30 -0700 Message-ID: <7d1ae7709bbe052f1d5737b6f0ef6cddf2f5e541.camel@linux.intel.com> Subject: Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus From: Tim Chen To: Luo Gengkun , Chen Yu Cc: "Chen, Yu C" , dietmar.eggemann@arm.com, rostedt@goodmis.org, peterz@infradead.org, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com, kprateek.nayak@amd.com, linux-kernel@vger.kernel.org, mingo@redhat.com, juri.lelli@redhat.com, vincent.guittot@linaro.org Date: Wed, 29 Jul 2026 11:29:29 -0700 In-Reply-To: <8b7cd508-3b89-4fc0-85fd-5a8d35ed06ca@huawei.com> References: <20260723040429.630176-1-luogengkun2@huawei.com> <20260723040429.630176-2-luogengkun2@huawei.com> <1dc03c84-9bc2-4db1-bab4-3f603fba54cd@huawei.com> <8b7cd508-3b89-4fc0-85fd-5a8d35ed06ca@huawei.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.1 (3.58.1-1.fc43) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Wed, 2026-07-29 at 17:19 +0800, Luo Gengkun wrote: >=20 > On 2026/7/28 20:08, Chen Yu wrote: > > On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: > > >=20 > > > On 2026/7/27 9:09, Chen, Yu C wrote: > > > > On 7/23/2026 12:04 PM, Luo Gengkun wrote: > > > >=20 > > > > [ ... ] > > > >=20 > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 guard(raw_spinlock_irqsave)(&rq->= cpu_epoch_lock); > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 __update_mm_sched(rq, pcpu_sched)= ; > > > > > +=C2=A0=C2=A0=C2=A0 /* Skip the rq that has not been hit for a lo= ng time */ > > > > > +=C2=A0=C2=A0=C2=A0 if ((rq->cpu_epoch - pcpu_sched->epoch_last_v= isit) > llc_epoch_affinity_timeout) { > > > >=20 > > > > In v2 there is a check if the cpu has been set before writing: > > > > cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) > > > > https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@hua= wei.com/ > > > > do we need to bring that back? > > > >=20 > > > I don't think we need it back. Here is why: > > >=20 > > > In v2, for_each_cpu was used instead of for_each_cpu_and in the inner= loop, > > > meaning some CPUs being checked might not have been set. Therefore, > > > cpumask_test_cpu was necessary to filter out those cases. > > >=20 > > > Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.vis= ited_cpus), > > > we can ensure each scanned CPU is set, so the issue no longer exists. > > > Furthermore, the only place where the visited_cpus bits are cleared i= s > > > task_cache_work(), which is only called once per scan period, there i= s no > > > risk of the bit being cleared concurrently mid-loop. > > >=20 > >=20 > > Make sense. > > =20 > > > However, is there a possibility that the current task_cache_work() ex= ecution > > > hasn't finished yet when the next scan window arrives? For instance, = if the > > > current task work is heavily delayed or preempted by unexpected inter= rupt, > > > jiffies could advance past next_scan before the loop completes. > > > If we move the `work->next =3D work;` to the very end of task_cache_w= ork(), > > > would that resolve this issue? By doing so, the existing `work->next = =3D=3D work` > > > check in task_tick_cache() should fail and no new task work will be s= ubmitted. > > >=20 > > > Please let me know if I'm missing something. > > >=20 > >=20 > > There are two layers of protection: first a cheap timeout gate (time_be= fore) > > that skips scanning until the next period, and then a try_cmpxchg that = atomically > > picks a single winner among the threads that pass the timeout =E2=80=94= this actually > > guarantees only one scanner per mm at a time, no? >=20 > What I am worried about is the following scenario: >=20 > Thread A (CPU 0) Thread B (CPU 1) > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > task_cache_work() > | > +-> try_cmpxchg() =3D=3D true > | (Sets next_scan =3D now + 10) > | > +-> Enters Scanning Loop (jiffies =3D 100) > | [ Delayed / Preempted ] > | jiffies advances 100 -> 115. > | Thread A STILL in the loop! > | task_cache_work() (jiffies = =3D 115) >=20 > | | > | +-> next_scan =3D=3D 110 = (pass timeout check) >=20 > | | > | +-> try_cmpxchg() =3D=3D = true >=20 > | | (Sets next_scan =3D 1= 15 + 10) > | | >=20 > In other words, concurrent execution of task_cache_work() from two adjace= nt periods can > occur under extreme conditions; do we need to take this scenario into con= sideration? >=20 > Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to = ensure > that the previous task_cache_work() execution has fully completed before = allowing > a new thread to proceed, regardless of whether the next scan window has a= rrived. > What do you think? First, the EPOCH_PERIOD is fairly long (10 msec) so it is unlikely that Thr= ead A hasn't completed its work. Second, suppose the above scenario happened, the two thread above are serialized in the work on updating occupancy and visited cpus under the cpus_read_lock when looping through the cpus in task_cache_work(). So I think we should be okay without an explicit state flag. That said, I think we should use mm->sc_stat.lock instead of cpus_read_lock in task_cache_work()'s cpu loop. That will improve scalability. Tim >=20 > thanks, > Gengkun >=20 > >=20 > > thanks, > > Chenyu