1. Patchew
  2. linux
  3. [PATCH v5 0/4] x86/cacheinfo: Set the number of leaves per CPU
  4. View patch

[PATCH v5 2/4] cacheinfo: Allocate memory for memory if not done from the primary CPU

Ricardo Neri posted 4 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v5 2/4] cacheinfo: Allocate memory for memory if not done from the primary CPU
Posted by Ricardo Neri 1 year, 3 months ago 
Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
adds functionality that architectures can use to optionally allocate and
build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
arch specific early level initializer") lets secondary CPUs correct (and
reallocate memory) cacheinfo data if needed.

If the early build functionality is not used and cacheinfo does not need
correction, memory for cacheinfo is never allocated. x86 does not use the
early build functionality. Consequently, during the cacheinfo CPU hotplug
callback, last_level_cache_is_valid() attempts to dereference a NULL
pointer:

     BUG: kernel NULL pointer dereference, address: 0000000000000100
     #PF: supervisor read access in kernel mode
     #PF: error_code(0x0000) - not present page
     PGD 0 P4D 0
     Oops: 0000 [#1] PREEPMT SMP NOPTI
     CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
     RIP: 0010: last_level_cache_is_valid+0x95/0xe0a

Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
not done earlier.

Reviewed-by: Radu Rendec <rrendec@redhat.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Cc: Andreas Herrmann <aherrmann@suse.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Nikolay Borisov <nik.borisov@suse.com>
Cc: Radu Rendec <rrendec@redhat.com>
Cc: Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Pu Wen <puwen@hygon.cn>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Will Deacon <will@kernel.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: stable@vger.kernel.org # 6.3+
---
The motivation for commit 5944ce092b97 was to prevent a BUG splat in
PREEMPT_RT kernels during memory allocation. This splat is not observed on
x86 because the memory allocation for cacheinfo happens in
detect_cache_attributes() from the cacheinfo CPU hotplug callback.

The dereference of a NULL pointer is not observed today because
cache_leaves(cpu) is zero until after init_cache_level() is called (also
during the CPU hotplug callback). A subsequent changeset will set the
number of cache leaves earlier and the NULL-pointer dereference will be
observed.
---
Changes since v4:
 * None

Changes since v3:
 * Added Reviewed-by tag from Radu and Sudeep. Thanks!

Changes since v2:
 * Introduced this patch.

Changes since v1:
 * N/A
---
 drivers/base/cacheinfo.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 77f2e0f91589..0332148691f9 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -554,7 +554,11 @@ static inline int init_level_allocate_ci(unsigned int cpu)
 	 */
 	ci_cacheinfo(cpu)->early_ci_levels = false;
 
-	if (cache_leaves(cpu) <= early_leaves)
+	/*
+	 * Some architectures (e.g., x86) do not use early initialization.
+	 * Allocate memory now in such case.
+	 */
+	if (cache_leaves(cpu) <= early_leaves && per_cpu_cacheinfo(cpu))
 		return 0;
 
 	kfree(per_cpu_cacheinfo(cpu));
-- 
2.34.1
Re: [PATCH v5 2/4] cacheinfo: Allocate memory for memory if not done from the primary CPU
Posted by Andreas Herrmann 1 year, 3 months ago 
On Mon, Aug 26, 2024 at 10:16:33PM -0700, Ricardo Neri wrote:
> Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> adds functionality that architectures can use to optionally allocate and
> build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> arch specific early level initializer") lets secondary CPUs correct (and
> reallocate memory) cacheinfo data if needed.
> 
> If the early build functionality is not used and cacheinfo does not need
> correction, memory for cacheinfo is never allocated. x86 does not use the
> early build functionality. Consequently, during the cacheinfo CPU hotplug
> callback, last_level_cache_is_valid() attempts to dereference a NULL
> pointer:
> 
>      BUG: kernel NULL pointer dereference, address: 0000000000000100
>      #PF: supervisor read access in kernel mode
>      #PF: error_code(0x0000) - not present page
>      PGD 0 P4D 0
>      Oops: 0000 [#1] PREEPMT SMP NOPTI
>      CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
>      RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> 
> Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> not done earlier.
> 
> Reviewed-by: Radu Rendec <rrendec@redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Reviewed-by: Andreas Herrmann <aherrmann@suse.de>
Tested-by: Andreas Herrmann <aherrmann@suse.de>

Test was done with a system equipped with AMD Phenom II X6 1055T and
test kernels based on v6.11-rc5-176-g20371ba12063.


Thanks,
Andreas
Re: [PATCH v5 2/4] cacheinfo: Allocate memory for memory if not done from the primary CPU
Posted by Nikolay Borisov 1 year, 3 months ago 

On 27.08.24 г. 8:16 ч., Ricardo Neri wrote:
> Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> adds functionality that architectures can use to optionally allocate and
> build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> arch specific early level initializer") lets secondary CPUs correct (and
> reallocate memory) cacheinfo data if needed.
> 
> If the early build functionality is not used and cacheinfo does not need
> correction, memory for cacheinfo is never allocated. x86 does not use the
> early build functionality. Consequently, during the cacheinfo CPU hotplug
> callback, last_level_cache_is_valid() attempts to dereference a NULL
> pointer:
> 
>       BUG: kernel NULL pointer dereference, address: 0000000000000100
>       #PF: supervisor read access in kernel mode
>       #PF: error_code(0x0000) - not present page
>       PGD 0 P4D 0
>       Oops: 0000 [#1] PREEPMT SMP NOPTI
>       CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
>       RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> 
> Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> not done earlier.

I assume instead of duplicating "memory" in the subject you meant 
"cacheinfo" ?

Otherwise LGTM:

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

> 
> Reviewed-by: Radu Rendec <rrendec@redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Fixes: 6539cffa9495 ("cacheinfo: Add arch specific early level initializer")
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Cc: Andreas Herrmann <aherrmann@suse.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Chen Yu <yu.c.chen@intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Nikolay Borisov <nik.borisov@suse.com>
> Cc: Radu Rendec <rrendec@redhat.com>
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>
> Cc: Pu Wen <puwen@hygon.cn>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: stable@vger.kernel.org # 6.3+
> ---
> The motivation for commit 5944ce092b97 was to prevent a BUG splat in
> PREEMPT_RT kernels during memory allocation. This splat is not observed on
> x86 because the memory allocation for cacheinfo happens in
> detect_cache_attributes() from the cacheinfo CPU hotplug callback.
> 
> The dereference of a NULL pointer is not observed today because
> cache_leaves(cpu) is zero until after init_cache_level() is called (also
> during the CPU hotplug callback). A subsequent changeset will set the
> number of cache leaves earlier and the NULL-pointer dereference will be
> observed.
> ---
> Changes since v4:
>   * None
> 
> Changes since v3:
>   * Added Reviewed-by tag from Radu and Sudeep. Thanks!
> 
> Changes since v2:
>   * Introduced this patch.
> 
> Changes since v1:
>   * N/A
> ---
>   drivers/base/cacheinfo.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> index 77f2e0f91589..0332148691f9 100644
> --- a/drivers/base/cacheinfo.c
> +++ b/drivers/base/cacheinfo.c
> @@ -554,7 +554,11 @@ static inline int init_level_allocate_ci(unsigned int cpu)
>   	 */
>   	ci_cacheinfo(cpu)->early_ci_levels = false;
>   
> -	if (cache_leaves(cpu) <= early_leaves)
> +	/*
> +	 * Some architectures (e.g., x86) do not use early initialization.
> +	 * Allocate memory now in such case.
> +	 */
> +	if (cache_leaves(cpu) <= early_leaves && per_cpu_cacheinfo(cpu))
>   		return 0;
>   
>   	kfree(per_cpu_cacheinfo(cpu));
Re: [PATCH v5 2/4] cacheinfo: Allocate memory for memory if not done from the primary CPU
Posted by Sudeep Holla 1 year, 3 months ago 
On Wed, Aug 28, 2024 at 04:16:00PM +0300, Nikolay Borisov wrote:
> 
> On 27.08.24 г. 8:16 ч., Ricardo Neri wrote:
> > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> > adds functionality that architectures can use to optionally allocate and
> > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> > arch specific early level initializer") lets secondary CPUs correct (and
> > reallocate memory) cacheinfo data if needed.
> > 
> > If the early build functionality is not used and cacheinfo does not need
> > correction, memory for cacheinfo is never allocated. x86 does not use the
> > early build functionality. Consequently, during the cacheinfo CPU hotplug
> > callback, last_level_cache_is_valid() attempts to dereference a NULL
> > pointer:
> > 
> >       BUG: kernel NULL pointer dereference, address: 0000000000000100
> >       #PF: supervisor read access in kernel mode
> >       #PF: error_code(0x0000) - not present page
> >       PGD 0 P4D 0
> >       Oops: 0000 [#1] PREEPMT SMP NOPTI
> >       CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
> >       RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> > 
> > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> > not done earlier.
> 
> I assume instead of duplicating "memory" in the subject you meant
> "cacheinfo" ?
>

Good point, +1 for the $subject change. I clearly missed to notice that.

-- 
Regards,
Sudeep
Re: [PATCH v5 2/4] cacheinfo: Allocate memory for memory if not done from the primary CPU
Posted by Ricardo Neri 1 year, 3 months ago 
On Wed, Aug 28, 2024 at 02:57:19PM +0100, Sudeep Holla wrote:
> On Wed, Aug 28, 2024 at 04:16:00PM +0300, Nikolay Borisov wrote:
> > 
> > On 27.08.24 г. 8:16 ч., Ricardo Neri wrote:
> > > Commit 5944ce092b97 ("arch_topology: Build cacheinfo from primary CPU")
> > > adds functionality that architectures can use to optionally allocate and
> > > build cacheinfo early during boot. Commit 6539cffa9495 ("cacheinfo: Add
> > > arch specific early level initializer") lets secondary CPUs correct (and
> > > reallocate memory) cacheinfo data if needed.
> > > 
> > > If the early build functionality is not used and cacheinfo does not need
> > > correction, memory for cacheinfo is never allocated. x86 does not use the
> > > early build functionality. Consequently, during the cacheinfo CPU hotplug
> > > callback, last_level_cache_is_valid() attempts to dereference a NULL
> > > pointer:
> > > 
> > >       BUG: kernel NULL pointer dereference, address: 0000000000000100
> > >       #PF: supervisor read access in kernel mode
> > >       #PF: error_code(0x0000) - not present page
> > >       PGD 0 P4D 0
> > >       Oops: 0000 [#1] PREEPMT SMP NOPTI
> > >       CPU: 0 PID 19 Comm: cpuhp/0 Not tainted 6.4.0-rc2 #1
> > >       RIP: 0010: last_level_cache_is_valid+0x95/0xe0a
> > > 
> > > Allocate memory for cacheinfo during the cacheinfo CPU hotplug callback if
> > > not done earlier.
> > 
> > I assume instead of duplicating "memory" in the subject you meant
> > "cacheinfo" ?
> >
> 
> Good point, +1 for the $subject change. I clearly missed to notice that.

Ah! Yes, it should read cacheinfo: Allocate memory for cacheinfo if not done from the primary CPU.

Thanks for spotting the error. I will update the subject in the next version.

Thanks and BR,
Ricardo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.