mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Chen, Yu C" <yu.c.chen@intel.com>
To: Davi Chaves Azevedo <davichazbh@gmail.com>
Cc: <peterz@infradead.org>, <mingo@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Valentin Schneider <vschneid@redhat.com>,
	"K Prateek Nayak" <kprateek.nayak@amd.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	<driver-core@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	"chen.yu@linux.dev" <chen.yu@linux.dev>
Subject: Re: [PATCH] sched/cache: Refresh LLC capacity across CPU hotplug
Date: Sat, 12 Sep 2026 02:24:38 +0800	[thread overview]
Message-ID: <a3433e6a-0d1f-44a8-99bd-bc63d1a15913@intel.com> (raw)
In-Reply-To: <20260911134825.420748-1-davichazbh@gmail.com>

Hi Davi,

On 9/11/2026 9:48 PM, Davi Chaves Azevedo wrote:
> The scheduler scales LLC capacity by the fraction of cache-sharing CPUs
> covered by a domain:
> 
>    llc_bytes = cache_size * span_weight / shared_weight
> 
> During CPU teardown, sched_cpu_deactivate() rebuilds scheduler domains
> before cacheinfo_cpu_pre_down() removes the CPU from shared_cpu_map. The
> new domains therefore use the old sharing weight. The later call to
> sched_update_llc_bytes() looks up the departing CPU's sd_llc, which has
> already been detached, and returns without correcting the surviving CPUs.
> 
> On a Ryzen 5 7535U with twelve logical CPUs sharing a 16 MiB LLC,
> offlining one SMT sibling left the remaining CPUs with:
> 
>    llc_bytes = floor(16777216 * 11 / 12) = 15379114 bytes
> 
> The correct capacity is still 16777216 bytes. On systems with active
> cache-aware scheduling, an underestimated capacity can cause
> exceed_llc_capacity() to reject aggregation for a process whose footprint
> would fit. Unchanged cpuset partitions sharing the physical cache can
> also retain stale capacity when a CPU comes online in another partition.
> 

Ah right, thanks very much for catching this. The current 
sched_update_llc_bytes()
only considers the bootup sequence for build_sched_domains() and 
cacheinfo_cpu_online(),
but misses the runtime CPU hotplug scenario, where it incorrectly 
ignores all the surviving
CPUs due to an offline CPU.

> Pass the cache-sharing mask already retained by cacheinfo to the
> scheduler update. Refresh every surviving CPU using its own LLC domain
> so that each partition receives the correct share. This also preserves
> the correction needed as cache-sharing maps grow during boot.
> 
> Keep the existing CPU-hotplug and scheduler-domain synchronization. The
> update remains on the hotplug path; no steady-state scheduling operation
> or persistent allocation is added.
> 
> Fixes: 7030513a0877 ("sched/cache: Calculate the LLC size and store it in sched_domain")
> Signed-off-by: Davi Chaves Azevedo <davichazbh@gmail.com>
> ---
> The issue was identified by tracing the scheduler/cacheinfo teardown
> ordering, then checking live llc_bytes values using the running kernel's
> BTF layout and /proc/kcore.
> 
> Validation:
>    - Reproduced the stale value on 7.2.3-arch1-3 on the Ryzen system above.
>      The patched kernel retained 16777216 bytes on every surviving CPU.
>    - Ten SMT-thread and ten whole-core hotplug cycles passed on the patched
>      kernel, including capacity checks after each removal and restoration.
>      The existing limited CPU-hotplug selftest also passed.
>    - Source-level state fixtures: five failures in eight scenarios before
>      the fix, eight passes after it. These cover partition changes, unequal
>      spans, sparse CPU IDs and boot-time map growth, but not concurrency.
>    - Full x86-64 baseline and patched bzImage/modules builds passed with
>      matched configs apart from LOCALVERSION. Focused ARM64, x86 without
>      CONFIG_SCHED_CACHE, and x86 UP builds also passed.
> 
> This host has only one LLC, so the live checks establish the accounting
> correction, not an aggregation speedup. No controlled same-version
> performance comparison or multi-LLC hardware result is claimed.
> 
>   drivers/base/cacheinfo.c       | 11 ++++++-----
>   include/linux/sched/topology.h |  4 ++--
>   kernel/sched/topology.c        | 27 ++++++++++++---------------
>   3 files changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> index 9f9c72727a05..7a47a392568a 100644
> --- a/drivers/base/cacheinfo.c
> +++ b/drivers/base/cacheinfo.c
> @@ -1040,9 +1040,10 @@ static int cacheinfo_cpu_online(unsigned int cpu)
>   	rc = cache_add_dev(cpu);
>   	if (rc)
>   		goto err;
> -	if (cpu_map_shared_cache(true, cpu, &cpu_map))
> +	if (cpu_map_shared_cache(true, cpu, &cpu_map)) {
>   		update_per_cpu_data_slice_size(true, cpu, cpu_map);
> -	sched_update_llc_bytes(cpu);
> +		sched_update_llc_bytes(cpu_map);
> +	}
>   	return 0;
>   err:
>   	free_cache_attributes(cpu);
> @@ -1059,10 +1060,10 @@ static int cacheinfo_cpu_pre_down(unsigned int cpu)
>   		cpu_cache_sysfs_exit(cpu);
>   
>   	free_cache_attributes(cpu);
> -	if (nr_shared > 1)
> +	if (nr_shared > 1) {
>   		update_per_cpu_data_slice_size(false, cpu, cpu_map);
> -
> -	sched_update_llc_bytes(cpu);
> +		sched_update_llc_bytes(cpu_map);
> +	}
>   
>   	return 0;
>   }
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index b5d9d7c2b8ad..f96812d71c51 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -281,9 +281,9 @@ static inline int task_node(const struct task_struct *p)
>   }
>   
>   #ifdef CONFIG_SCHED_CACHE
> -extern void sched_update_llc_bytes(unsigned int cpu);
> +extern void sched_update_llc_bytes(const struct cpumask *cpus);
>   #else
> -static inline void sched_update_llc_bytes(unsigned int cpu) { }
> +static inline void sched_update_llc_bytes(const struct cpumask *cpus) { }
>   #endif
>   
>   #endif /* _LINUX_SCHED_TOPOLOGY_H */
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 0248227d983a..a10eecc49d51 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -985,8 +985,8 @@ void sched_cache_active_set(void)
>   }
>   
>   /*
> - * Update the bottom sched_domain's llc_bytes for @cpu and all its
> - * LLC siblings. Called from cacheinfo_cpu_online() or
> + * Update the bottom sched_domain's llc_bytes for @cpus sharing a physical
> + * LLC. Called from cacheinfo_cpu_online() or
>    * cacheinfo_cpu_pre_down() with cpu hotplug lock held.
>    *
>    * Note: get_effective_llc_bytes() returns 0 on PowerPC.
> @@ -996,32 +996,29 @@ void sched_cache_active_set(void)
>    * and does not populates the per-CPU struct cpu_cacheinfo array
>    * that get_cpu_cacheinfo_llc() reads.
>    */
> -void sched_update_llc_bytes(unsigned int cpu)
> +void sched_update_llc_bytes(const struct cpumask *cpus)
>   {
>   	struct sched_domain *sd, *sdp;
>   	unsigned int i;
>   
>   	sched_domains_mutex_lock();
>   
> -	sdp = rcu_dereference_sched_domain(per_cpu(sd_llc, cpu));
> -	if (!sdp)
> -		goto unlock;
> -
>   	/*
> -	 * ci->shared_cpu_map is built incrementally as CPUs come
> -	 * online, so the first CPU in an LLC initially sees
> -	 * hw_weight == 1 and computes an inflated llc_bytes in
> -	 * get_effective_llc_bytes().  Re-evaluating every LLC
> -	 * sibling on each online event corrects this once the full
> -	 * shared_cpu_map is known.
Maybe the above comments can be kept, because they describe the bootup
scenario, with your offline case/domain partition added — just in case
in the future the reader might wonder why we iterate every online CPU
again and again during cacheinfo_cpu_online()/offline().
> +	 * The departing CPU's domains have already been detached when
> +	 * cacheinfo removes it. Use the surviving cache siblings instead.
> +	 * They may belong to different cpuset partitions, so use each CPU's
> +	 * own LLC domain to scale its share of the physical cache.
>   	 */
> -	for_each_cpu(i, sched_domain_span(sdp)) {
> +	for_each_cpu(i, cpus) {
> +		sdp = rcu_dereference_sched_domain(per_cpu(sd_llc, i));
> +		if (!sdp)
> +			continue;
> +
>   		sd = rcu_dereference_sched_domain(cpu_rq(i)->sd);
>   		if (sd)
>   			sd->llc_bytes = get_effective_llc_bytes(i, sdp);
>   	}
>   
> -unlock:
>   	sched_domains_mutex_unlock();
>   }
>   
> base-commit: 50d05c7c76c96b90462f24debacca971d2e86713

I have reproduced this issue on an AMD Ryzen 8945HX machine, which has 2 
LLCs
and 8 cores/LLC. It is also reproduced on a Xeon platform with 4 LLCs 
per node.
With the patch applied, sd->llc_bytes is back to normal. This patch 
looks reasonable
to me.

Reviewed-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Chenyu

  reply	other threads:[~2026-09-11 18:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 13:48 Davi Chaves Azevedo
2026-09-11 18:24 ` Chen, Yu C [this message]
2026-09-11 22:10   ` Davi Chaves Azevedo
2026-09-11 22:02 ` [PATCH v2] " Davi Chaves Azevedo
2026-09-12  0:55   ` Tim Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a3433e6a-0d1f-44a8-99bd-bc63d1a15913@intel.com \
    --to=yu.c.chen@intel.com \
    --cc=chen.yu@linux.dev \
    --cc=dakr@kernel.org \
    --cc=davichazbh@gmail.com \
    --cc=driver-core@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®