From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.11]) (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 E661030CD80 for ; Wed, 15 Jul 2026 20:44:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.11 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784148244; cv=none; b=VhTBpQuZHC2ecKPuZCdz6CNfH7BqGDMlE6hfb4AorgIw6XR8rtoBhv4N6Pra6Rn+K3SpZmzNwtsUiBAOzOmNLOqWRi35h3gJAjaNI3s3NN2J4YC0NNKHLNndJ+L+rV03xEyM3w4FhaQQE5czbp/SDh5qWFIqzLvUe/YlAKyhw44= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784148244; c=relaxed/simple; bh=ed6Sam2h54ANWnrdi5aKYPiJXrTbEUmKbamMZEttmYw=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=lLLamYfzrbDno8VbY0+p5ddqdu9wuOoWqrDbqY+e6WhewwQbb9lu2ZzZJ2ns+TdiVtaST/m0pJ3KDBHYeB3E5abj8EWi9v7c54Z4idGc2baq57YitDYHA3nIMfqk64iaTSz/xZ6z519WyBxffokyb9IB3fKH0JbWgCyQAJu3iY4= 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=MTHZYnKf; arc=none smtp.client-ip=192.198.163.11 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="MTHZYnKf" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1784148242; x=1815684242; h=message-id:subject:from:to:cc:date:in-reply-to: references:content-transfer-encoding:mime-version; bh=ed6Sam2h54ANWnrdi5aKYPiJXrTbEUmKbamMZEttmYw=; b=MTHZYnKfA15l3GWC68wNuinlIhl3Ce4fegtPYenqbbw8tJgfZz7nBN+q unBr3Swkn8oXTF7eokLVXC/2OnYHH1eUu6aRjYEwe9YV9eXKcy7zpTR7R o2mvStcNoyV4HgW8XjgNtS8f2WZW+vvwzNJFyNEmfoFcZNZ3luuIhtkw7 Ekmc1L6A3QDv3BmgZo+kZd4LbWBEnTAchzcTiBv6Q9VwYu70IMwUebrvf DQUjIE77PNXCuHJEcHvZIBuhp7rabZr8kY4rfHgpPfQeSYIQchuw/1xm4 bUkx2PxNUTqUKvbdcUgkuJ98MIttQyBhHpkEUaKBTCBdFneSrP380NIRI Q==; X-CSE-ConnectionGUID: wR3Wf1NQSl6B4EYHanmNBA== X-CSE-MsgGUID: q/gsB48JQJC8ONJZSdIpjg== X-IronPort-AV: E=McAfee;i="6800,10657,11847"; a="95406996" X-IronPort-AV: E=Sophos;i="6.25,166,1779174000"; d="scan'208";a="95406996" Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jul 2026 13:44:01 -0700 X-CSE-ConnectionGUID: fXeckhJ6TWCQfzXys+jJKQ== X-CSE-MsgGUID: XbQSgaaNSGmYeVK8gti1iQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,166,1779174000"; d="scan'208";a="280729929" Received: from unknown (HELO [10.241.243.185]) ([10.241.243.185]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jul 2026 13:44:00 -0700 Message-ID: <4c43b34fd3134cc60d84640a4c70e38e6373dae5.camel@linux.intel.com> Subject: Re: [PATCH v5 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 C" Cc: dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, peterz@infradead.org, vschneid@redhat.com, kprateek.nayak@amd.com, linux-kernel@vger.kernel.org, juri.lelli@redhat.com, mingo@redhat.com, vincent.guittot@linaro.org, "chen.yu@linux.dev" Date: Wed, 15 Jul 2026 13:44:00 -0700 In-Reply-To: <7f67f8f6-7b01-40fc-93fc-180d2f006c0a@huawei.com> References: <20260709130053.2749834-1-luogengkun2@huawei.com> <20260709130053.2749834-2-luogengkun2@huawei.com> <95dc124ed604ff568dc01ba502adf8d4e35c255b.camel@linux.intel.com> <7f67f8f6-7b01-40fc-93fc-180d2f006c0a@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-15 at 14:41 +0800, Luo Gengkun wrote: >=20 > On 2026/7/15 6:40, Tim Chen wrote: > > On Thu, 2026-07-09 at 23:01 +0800, Chen, Yu C wrote: > > > Hi Gengkun, > > >=20 > > > thanks for the update, > > >=20 > > > On 7/9/2026 9:00 PM, Luo Gengkun wrote: > > >=20 > > > > +static unsigned long fraction_mm_sched(int cpu, > > > > + struct mm_struct *mm) > > > > { > > > > + struct sched_cache_time *pcpu_sched =3D > > > > + per_cpu_ptr(mm->sc_stat.pcpu_sched, cpu); > > > > + struct rq *rq =3D cpu_rq(cpu); > > > > + > > > > guard(raw_spinlock_irqsave)(&rq->cpu_epoch_lock); > > > > =20 > > > > + /* Skip the rq that has not been hit for a long time */ > > > > + if ((rq->cpu_epoch - pcpu_sched->epoch_timeout) > llc_epoch_affin= ity_timeout) { > > > > + cpumask_clear_cpu(cpu, &mm->sc_stat.visited_cpus); > > > > + return 0; > > > > + } > > > > + > > >=20 > > > I was wondering if there is a race condition in the scenario > > > above: If the rq has just updated its cpu_epoch field, and the > > > CPU subsequently remains idle, then rq->cpu_epoch stays unchanged. > > > That is to say, a race could occur as follows: if CPU1 becomes idle, > > > CPU2's task_cache_work() might compute > > > rq->cpu_epoch - pcpu_sched->epoch_timeout =3D=3D 0 and consequently f= ail > > > to clear CPU1 from visit_cpus. > > >=20 > > > CPU_1 CPU_2 > > > ------------------------------------ ---------------------------= ------ > > > account_mm_sched(rq_X) > > > set CPU1 in visited_cpus > > >=20 > > >=20 > > > (idle: fair task runs here, > > > CPU1.cpu_epoch frozen afterwards, > > > pcpu_sched->epoch_timeout =3D CPU1.epoch) > > > fraction_mm_sched() > > > CPU1->cpu_epoch - > > > pcpu_sched->epoch_timeout =3D 0 > > > -> bit CPU1 not cleared > > > __update_mm_sched() > > > ->update cpu_epoch, too late > > > I wonder if we should let __update_mm_sched() be invoked before > > > if ((rq->cpu_epoch - pcpu_sched->epoch_timeout) > > > > llc_epoch_affinity_timeout)? > >=20 > > Also, I am not sure why we need to create this new field pcpu_sched->ep= och_timeout? > > Wouldn't rq->cpu_epoch - pcpu_sched->epoch give the elapsed > > epochs since the last time that cpu was visited for this mm > > (as in __update_mm_sched())? > >=20 > Good question. > During previous testing, I found that pcpu_sched->epoch was being flushed > by calls to fraction_mm_sched(), even though the task was not running on > that specific CPU. Because the flush frequency was higher than the evicti= on > frequency, the eviction mechanism failed. The newly introduced epoch_time= out > ensures that the epoch_timeout is only flushed when the task is actually > running on the corresponding CPU, thereby preventing the aforementioned i= ssue. Okay. pcpu_sched->epoch is refreshed when the accounting is done on the cpu= and pcpu_sched->epoch_timeout is refreshed when a task in the process was last = run on the CPU. I think renaming to pcpu_sched->epoch_last_visit makes things c= learer. >=20 > > There are some races possible > > when cpu_epoch changes before the whole accounting got done. > > A CPU could get visited after you think that it hasn't got visited. > > But you will also have race if you use epoch_timeout >=20 > If a CPU was just visited, there are two possible conditions: >=20 > 1. The CPU is not yet set in visited_cpus, meaning account_mm_sched() has= not been > called yet. In this case, fraction_mm_sched() won't be triggered either. = This is > acceptable because it implies pcpu_sched->runtime hasn't been updated for= a long > time too, and this negligible contribution can be safely ignored. >=20 > 2. The CPU is already set in visited_cpus. In this case, we can simply ac= cept this > contribution instead of performing an early return. We can just clear the= CPU > from the mask and proceed, as our primary goal is simply to limit the num= ber of > scanned CPUs. As shown in the code below. What do you think? >=20 > @@ -1651,7 +1651,6 @@ static unsigned long fraction_mm_sched(int cpu, > if (sched_feat(SC_VISIT) && > (rq->cpu_epoch - pcpu_sched->epoch_timeout) > llc_epoch_affi= nity_timeout) { > cpumask_clear_cpu(cpu, mm->sc_stat.visited_cpus); > - return 0; > } You need to call __update_mm_sched(rq, pcpu_sched) to have rq->epoch update= d to current time before you perform the above comparison and see if we exceed llc_epoch_affinity_timeout. I think that's what Chen Yu pointed out in his previous mail. Tim > =20 > /* > @@ -1913,8 +1912,6 @@ static void task_cache_work(struct callback_head *w= ork) > =20 > scanned++; > occ =3D fraction_mm_sched(i, mm); > - if (occ =3D=3D 0) > - continue; > =20 > a_occ +=3D occ; >=20 >=20 > thanks, > Gengkun >=20 > >=20 > > Tim > > =20 > >=20 > > >=20 > > > BTW, in your test data: > > > sched_cache_scan: comm=3Dredis-server pid=3D24660 scan=3D384 > > > Is it because NUMA balancing is disabled on your platform that it has > > > to scan all the CPUs? It would be helpful to know what the result wou= ld > > > be with NUMA balancing enabled. > > > I found some reports from sashiko here that are worth taking a look a= t: > > > https://sashiko.dev/#/patchset/20260709130053.2749834-1-luogengkun2%4= 0huawei.com > > >=20 > > > thanks, > > > Chenyu > > >=20