v2:
- Applied renaming/formatting comments from v1.
- Check CACHE_TYPE_VALID flag in pppt.c.
Note:
This patchset requires the following patch to be applied first in
order to avoid the same bug described in the commit message:
https://lore.kernel.org/all/[email protected]/
[1] and [2] build the CPU topology from the cacheinfo information for
both DT/ACPI based systems and remove (struct cpu_topology).llc_id
which was used by ACPI only.
Creating the cacheinfo for secondary CPUs is done during early boot.
Preemption and interrupts are disabled at this stage. On PREEMPT_RT
kernels, allocating memory (and parsing the PPTT table for ACPI based
systems) triggers a:
'BUG: sleeping function called from invalid context' [4]
To prevent this bug, allocate the cacheinfo from the primary CPU when
preemption and interrupts are enabled and before booting secondary
CPUs. The cache levels/leaves are computed from DT/ACPI PPTT information
only, without relying on the arm64 CLIDR_EL1 register.
If no cache information is found in the DT/ACPI PPTT, then fallback
to the current state, triggering [4] on PREEMPT_RT kernels.
Patches to update the arm64 device trees that have incomplete cacheinfo
(mostly for missing the 'cache-level' or 'cache-unified' property)
have been sent at [3].
Tested platforms:
- ACPI + PPTT: Ampere Altra, Ampere eMAG, Cavium ThunderX2,
Kunpeng 920, Juno-r2
- DT: rb5, db845c, Juno-r2
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] On an Ampere Altra, with PREEMPT_RT kernel based on v6.0.0-rc4:
[ 7.560791] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
[ 7.560794] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/111
[ 7.560796] preempt_count: 1, expected: 0
[ 7.560797] RCU nest depth: 1, expected: 1
[ 7.560799] 3 locks held by swapper/111/0:
[ 7.560800] #0: ffff403e406cae98 (&pcp->lock){+.+.}-{3:3}, at: get_page_from_freelist+0x218/0x12c8
[ 7.560811] #1: ffffc5f8ed09f8e8 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x48/0xf0
[ 7.560820] #2: ffff403f400b4fd8 (&zone->lock){+.+.}-{3:3}, at: rmqueue_bulk+0x64/0xa80
[ 7.560824] irq event stamp: 0
[ 7.560825] hardirqs last enabled at (0): [<0000000000000000>] 0x0
[ 7.560827] hardirqs last disabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
[ 7.560830] softirqs last enabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
[ 7.560833] softirqs last disabled at (0): [<0000000000000000>] 0x0
[ 7.560834] Preemption disabled at:
[ 7.560835] [<ffffc5f8e9fd3c28>] migrate_enable+0x30/0x130
[ 7.560838] CPU: 111 PID: 0 Comm: swapper/111 Tainted: G W 6.0.0-rc4-[...]
[ 7.560841] Call trace:
[...]
[ 7.560870] __kmalloc+0xbc/0x1e8
[ 7.560873] detect_cache_attributes+0x2d4/0x5f0
[ 7.560876] update_siblings_masks+0x30/0x368
[ 7.560880] store_cpu_topology+0x78/0xb8
[ 7.560883] secondary_start_kernel+0xd0/0x198
[ 7.560885] __secondary_switched+0xb0/0xb4
Pierre Gondois (5):
cacheinfo: Use RISC-V's init_cache_level() as generic OF
implementation
cacheinfo: Return error code in init_of_cache_level()
ACPI: PPTT: Remove acpi_find_cache_levels()
ACPI: PPTT: Update acpi_find_last_cache_level() to
acpi_get_cache_info()
arch_topology: Build cacheinfo from primary CPU
arch/arm64/kernel/cacheinfo.c | 9 ++-
arch/riscv/kernel/cacheinfo.c | 39 +------------
drivers/acpi/pptt.c | 93 +++++++++++++++++-------------
drivers/base/arch_topology.c | 10 +++-
drivers/base/cacheinfo.c | 104 ++++++++++++++++++++++++++++++----
include/linux/cacheinfo.h | 10 +++-
6 files changed, 170 insertions(+), 95 deletions(-)
--
2.25.1
acpi_find_cache_levels() is used at a single place and is short
enough to be merged into the calling function. The removal allows
an easier renaming of the calling function in the next patch.
Also reorder the local variables in the 'reversed Christmas tree'
order.
Signed-off-by: Pierre Gondois <[email protected]>
Reviewed-by: Sudeep Holla <[email protected]>
Reviewed-by: Jeremy Linton <[email protected]>
---
drivers/acpi/pptt.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index c91342dcbcd6..97c1d33822d1 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -281,19 +281,6 @@ static struct acpi_pptt_processor *acpi_find_processor_node(struct acpi_table_he
return NULL;
}
-static int acpi_find_cache_levels(struct acpi_table_header *table_hdr,
- u32 acpi_cpu_id)
-{
- int number_of_levels = 0;
- struct acpi_pptt_processor *cpu;
-
- cpu = acpi_find_processor_node(table_hdr, acpi_cpu_id);
- if (cpu)
- number_of_levels = acpi_count_levels(table_hdr, cpu);
-
- return number_of_levels;
-}
-
static u8 acpi_cache_type(enum cache_type type)
{
switch (type) {
@@ -613,9 +600,10 @@ static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
*/
int acpi_find_last_cache_level(unsigned int cpu)
{
- u32 acpi_cpu_id;
+ struct acpi_pptt_processor *cpu_node;
struct acpi_table_header *table;
int number_of_levels = 0;
+ u32 acpi_cpu_id;
table = acpi_get_pptt();
if (!table)
@@ -624,7 +612,10 @@ int acpi_find_last_cache_level(unsigned int cpu)
pr_debug("Cache Setup find last level CPU=%d\n", cpu);
acpi_cpu_id = get_acpi_id_for_cpu(cpu);
- number_of_levels = acpi_find_cache_levels(table, acpi_cpu_id);
+ cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
+ if (cpu_node)
+ number_of_levels = acpi_count_levels(table, cpu_node);
+
pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
return number_of_levels;
--
2.25.1
On Mon, Nov 21, 2022 at 6:12 PM Pierre Gondois <[email protected]> wrote:
>
> v2:
> - Applied renaming/formatting comments from v1.
> - Check CACHE_TYPE_VALID flag in pppt.c.
>
> Note:
> This patchset requires the following patch to be applied first in
> order to avoid the same bug described in the commit message:
> https://lore.kernel.org/all/[email protected]/
>
> [1] and [2] build the CPU topology from the cacheinfo information for
> both DT/ACPI based systems and remove (struct cpu_topology).llc_id
> which was used by ACPI only.
>
> Creating the cacheinfo for secondary CPUs is done during early boot.
> Preemption and interrupts are disabled at this stage. On PREEMPT_RT
> kernels, allocating memory (and parsing the PPTT table for ACPI based
> systems) triggers a:
> 'BUG: sleeping function called from invalid context' [4]
>
> To prevent this bug, allocate the cacheinfo from the primary CPU when
> preemption and interrupts are enabled and before booting secondary
> CPUs. The cache levels/leaves are computed from DT/ACPI PPTT information
> only, without relying on the arm64 CLIDR_EL1 register.
> If no cache information is found in the DT/ACPI PPTT, then fallback
> to the current state, triggering [4] on PREEMPT_RT kernels.
>
> Patches to update the arm64 device trees that have incomplete cacheinfo
> (mostly for missing the 'cache-level' or 'cache-unified' property)
> have been sent at [3].
>
> Tested platforms:
> - ACPI + PPTT: Ampere Altra, Ampere eMAG, Cavium ThunderX2,
> Kunpeng 920, Juno-r2
> - DT: rb5, db845c, Juno-r2
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/all/[email protected]/
> [3] https://lore.kernel.org/all/[email protected]/
> [4] On an Ampere Altra, with PREEMPT_RT kernel based on v6.0.0-rc4:
>
>
> [ 7.560791] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
> [ 7.560794] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/111
> [ 7.560796] preempt_count: 1, expected: 0
> [ 7.560797] RCU nest depth: 1, expected: 1
> [ 7.560799] 3 locks held by swapper/111/0:
> [ 7.560800] #0: ffff403e406cae98 (&pcp->lock){+.+.}-{3:3}, at: get_page_from_freelist+0x218/0x12c8
> [ 7.560811] #1: ffffc5f8ed09f8e8 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x48/0xf0
> [ 7.560820] #2: ffff403f400b4fd8 (&zone->lock){+.+.}-{3:3}, at: rmqueue_bulk+0x64/0xa80
> [ 7.560824] irq event stamp: 0
> [ 7.560825] hardirqs last enabled at (0): [<0000000000000000>] 0x0
> [ 7.560827] hardirqs last disabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
> [ 7.560830] softirqs last enabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
> [ 7.560833] softirqs last disabled at (0): [<0000000000000000>] 0x0
> [ 7.560834] Preemption disabled at:
> [ 7.560835] [<ffffc5f8e9fd3c28>] migrate_enable+0x30/0x130
> [ 7.560838] CPU: 111 PID: 0 Comm: swapper/111 Tainted: G W 6.0.0-rc4-[...]
> [ 7.560841] Call trace:
> [...]
> [ 7.560870] __kmalloc+0xbc/0x1e8
> [ 7.560873] detect_cache_attributes+0x2d4/0x5f0
> [ 7.560876] update_siblings_masks+0x30/0x368
> [ 7.560880] store_cpu_topology+0x78/0xb8
> [ 7.560883] secondary_start_kernel+0xd0/0x198
> [ 7.560885] __secondary_switched+0xb0/0xb4
>
> Pierre Gondois (5):
> cacheinfo: Use RISC-V's init_cache_level() as generic OF
> implementation
> cacheinfo: Return error code in init_of_cache_level()
> ACPI: PPTT: Remove acpi_find_cache_levels()
> ACPI: PPTT: Update acpi_find_last_cache_level() to
> acpi_get_cache_info()
> arch_topology: Build cacheinfo from primary CPU
>
> arch/arm64/kernel/cacheinfo.c | 9 ++-
> arch/riscv/kernel/cacheinfo.c | 39 +------------
> drivers/acpi/pptt.c | 93 +++++++++++++++++-------------
> drivers/base/arch_topology.c | 10 +++-
> drivers/base/cacheinfo.c | 104 ++++++++++++++++++++++++++++++----
> include/linux/cacheinfo.h | 10 +++-
> 6 files changed, 170 insertions(+), 95 deletions(-)
>
> --
For the ACPI material in the series:
Acked-by: Rafael J. Wysocki <[email protected]>
and I'm assuming that this series will be merged through a different tree.
On Mon, 21 Nov 2022 09:12:08 PST (-0800), [email protected] wrote:
> v2:
> - Applied renaming/formatting comments from v1.
> - Check CACHE_TYPE_VALID flag in pppt.c.
>
> Note:
> This patchset requires the following patch to be applied first in
> order to avoid the same bug described in the commit message:
> https://lore.kernel.org/all/[email protected]/
>
> [1] and [2] build the CPU topology from the cacheinfo information for
> both DT/ACPI based systems and remove (struct cpu_topology).llc_id
> which was used by ACPI only.
>
> Creating the cacheinfo for secondary CPUs is done during early boot.
> Preemption and interrupts are disabled at this stage. On PREEMPT_RT
> kernels, allocating memory (and parsing the PPTT table for ACPI based
> systems) triggers a:
> 'BUG: sleeping function called from invalid context' [4]
>
> To prevent this bug, allocate the cacheinfo from the primary CPU when
> preemption and interrupts are enabled and before booting secondary
> CPUs. The cache levels/leaves are computed from DT/ACPI PPTT information
> only, without relying on the arm64 CLIDR_EL1 register.
> If no cache information is found in the DT/ACPI PPTT, then fallback
> to the current state, triggering [4] on PREEMPT_RT kernels.
>
> Patches to update the arm64 device trees that have incomplete cacheinfo
> (mostly for missing the 'cache-level' or 'cache-unified' property)
> have been sent at [3].
>
> Tested platforms:
> - ACPI + PPTT: Ampere Altra, Ampere eMAG, Cavium ThunderX2,
> Kunpeng 920, Juno-r2
> - DT: rb5, db845c, Juno-r2
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/all/[email protected]/
> [3] https://lore.kernel.org/all/[email protected]/
> [4] On an Ampere Altra, with PREEMPT_RT kernel based on v6.0.0-rc4:
>
>
> [ 7.560791] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
> [ 7.560794] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/111
> [ 7.560796] preempt_count: 1, expected: 0
> [ 7.560797] RCU nest depth: 1, expected: 1
> [ 7.560799] 3 locks held by swapper/111/0:
> [ 7.560800] #0: ffff403e406cae98 (&pcp->lock){+.+.}-{3:3}, at: get_page_from_freelist+0x218/0x12c8
> [ 7.560811] #1: ffffc5f8ed09f8e8 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x48/0xf0
> [ 7.560820] #2: ffff403f400b4fd8 (&zone->lock){+.+.}-{3:3}, at: rmqueue_bulk+0x64/0xa80
> [ 7.560824] irq event stamp: 0
> [ 7.560825] hardirqs last enabled at (0): [<0000000000000000>] 0x0
> [ 7.560827] hardirqs last disabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
> [ 7.560830] softirqs last enabled at (0): [<ffffc5f8e9f7d594>] copy_process+0x5dc/0x1ab8
> [ 7.560833] softirqs last disabled at (0): [<0000000000000000>] 0x0
> [ 7.560834] Preemption disabled at:
> [ 7.560835] [<ffffc5f8e9fd3c28>] migrate_enable+0x30/0x130
> [ 7.560838] CPU: 111 PID: 0 Comm: swapper/111 Tainted: G W 6.0.0-rc4-[...]
> [ 7.560841] Call trace:
> [...]
> [ 7.560870] __kmalloc+0xbc/0x1e8
> [ 7.560873] detect_cache_attributes+0x2d4/0x5f0
> [ 7.560876] update_siblings_masks+0x30/0x368
> [ 7.560880] store_cpu_topology+0x78/0xb8
> [ 7.560883] secondary_start_kernel+0xd0/0x198
> [ 7.560885] __secondary_switched+0xb0/0xb4
>
> Pierre Gondois (5):
> cacheinfo: Use RISC-V's init_cache_level() as generic OF
> implementation
> cacheinfo: Return error code in init_of_cache_level()
> ACPI: PPTT: Remove acpi_find_cache_levels()
> ACPI: PPTT: Update acpi_find_last_cache_level() to
> acpi_get_cache_info()
> arch_topology: Build cacheinfo from primary CPU
>
> arch/arm64/kernel/cacheinfo.c | 9 ++-
> arch/riscv/kernel/cacheinfo.c | 39 +------------
> drivers/acpi/pptt.c | 93 +++++++++++++++++-------------
> drivers/base/arch_topology.c | 10 +++-
> drivers/base/cacheinfo.c | 104 ++++++++++++++++++++++++++++++----
> include/linux/cacheinfo.h | 10 +++-
> 6 files changed, 170 insertions(+), 95 deletions(-)
Acked-by: Palmer Dabbelt <[email protected]>