mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tim Chen <tim.c.chen@linux.intel.com>
To: Davi Chaves Azevedo <davichazbh@gmail.com>,
	peterz@infradead.org,  mingo@redhat.com
Cc: Chen Yu <yu.c.chen@intel.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>,
	chen.yu@linux.dev, driver-core@lists.linux.dev,
		linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] sched/cache: Refresh LLC capacity across CPU hotplug
Date: Fri, 11 Sep 2026 17:55:30 -0700	[thread overview]
Message-ID: <8da2c1b91baf26b89b37e9cb6ea38af8c4f807cf.camel@linux.intel.com> (raw)
In-Reply-To: <20260911220229.1368887-1-davichazbh@gmail.com>

On Fri, 2026-09-11 at 19:02 -0300, 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.
> 
> 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.
> 


Thanks.  The patch looks good to me.

Tim

> Fixes: 7030513a0877 ("sched/cache: Calculate the LLC size and store it in sched_domain")
> Signed-off-by: Davi Chaves Azevedo <davichazbh@gmail.com>
> Reviewed-by: Chen Yu <yu.c.chen@intel.com>
> ---
> Changes in v2:
>   - Restore the original boot-time shared_cpu_map explanation, as Chen Yu
>     suggested, alongside the CPU-offline and cpuset-partition rationale.
>     No functional changes from v1.
>   - Add Chen Yu's Reviewed-by tag and document his multi-LLC testing.
> 
> v1:
> https://lore.kernel.org/r/20260911134825.420748-1-davichazbh@gmail.com
> Review:
> https://lore.kernel.org/r/a3433e6a-0d1f-44a8-99bd-bc63d1a15913@intel.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.
> 
> Local validation performed for v1 (no functional changes in v2):
>   - 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.
> 
> Thanks to Chen Yu <yu.c.chen@intel.com> for the additional verification
> and review. He reproduced the issue and confirmed that v1 restored the
> expected sd->llc_bytes on:
>   - AMD Ryzen 8945HX: two LLCs, eight cores per LLC.
>   - Xeon: four LLCs per node.
> 
> The local tests used a single-LLC host; Chen Yu reported the multi-LLC
> results above. These checks validate LLC accounting.
> Builds and hotplug tests were not rerun for this comment revision.
> 
>  drivers/base/cacheinfo.c       | 11 ++++++-----
>  include/linux/sched/topology.h |  4 ++--
>  kernel/sched/topology.c        | 22 +++++++++++++---------
>  3 files changed, 21 insertions(+), 16 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..3dab0253976f 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,17 +996,13 @@ 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
> @@ -1014,14 +1010,22 @@ void sched_update_llc_bytes(unsigned int cpu)
>  	 * get_effective_llc_bytes().  Re-evaluating every LLC
>  	 * sibling on each online event corrects this once the full
>  	 * shared_cpu_map is known.
> +	 *
> +	 * 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

      reply	other threads:[~2026-09-12  0:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 13:48 [PATCH] " Davi Chaves Azevedo
2026-09-11 18:24 ` Chen, Yu C
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 [this message]

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=8da2c1b91baf26b89b37e9cb6ea38af8c4f807cf.camel@linux.intel.com \
    --to=tim.c.chen@linux.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=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=yu.c.chen@intel.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®