[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 riscv's init_cache_level() as generic OF implem
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 | 86 +++++++++++++++++------------
drivers/base/arch_topology.c | 10 +++-
drivers/base/cacheinfo.c | 101 ++++++++++++++++++++++++++++++----
include/linux/cacheinfo.h | 10 +++-
6 files changed, 164 insertions(+), 91 deletions(-)
--
2.25.1
commit 3fcbf1c77d08 ("arch_topology: Fix cache attributes detection
in the CPU hotplug path")
adds a call to detect_cache_attributes() to populate the cacheinfo
before updating the siblings mask. detect_cache_attributes() allocates
memory and can take the PPTT mutex (on ACPI platforms). On PREEMPT_RT
kernels, on secondary CPUs, this triggers a:
'BUG: sleeping function called from invalid context' [1]
as the code is executed with preemption and interrupts disabled.
The primary CPU was previously storing the cache information using
the now removed (struct cpu_topology).llc_id:
commit 5b8dc787ce4a ("arch_topology: Drop LLC identifier stash from
the CPU topology")
allocate_cache_info() tries to build the cacheinfo from the primary
CPU prior secondary CPUs boot, if the DT/ACPI description
contains cache information.
If allocate_cache_info() fails, then fallback to the current state
for the cacheinfo allocation. [1] will be triggered in such case.
When unplugging a CPU, the cacheinfo memory cannot be freed. If it
was, then the memory would be allocated early by the re-plugged
CPU and would trigger [1].
[1]:
[ 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-rt6-[...]
[ 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
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/base/arch_topology.c | 10 ++++++-
drivers/base/cacheinfo.c | 53 ++++++++++++++++++++++++++++--------
include/linux/cacheinfo.h | 1 +
3 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index d07a7cfa389a..4222a6d1e07b 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -825,7 +825,7 @@ __weak int __init parse_acpi_topology(void)
#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
void __init init_cpu_topology(void)
{
- int ret;
+ int cpu, ret;
reset_cpu_topology();
ret = parse_acpi_topology();
@@ -840,6 +840,14 @@ void __init init_cpu_topology(void)
reset_cpu_topology();
return;
}
+
+ for_each_possible_cpu(cpu) {
+ ret = allocate_cache_info(cpu);
+ if (ret) {
+ pr_err("Early cacheinfo failed, ret = %d\n", ret);
+ break;
+ }
+ }
}
void store_cpu_topology(unsigned int cpuid)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 6f6cd120c4f1..889d94d89e18 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -371,10 +371,6 @@ static void free_cache_attributes(unsigned int cpu)
return;
cache_shared_cpu_map_remove(cpu);
-
- kfree(per_cpu_cacheinfo(cpu));
- per_cpu_cacheinfo(cpu) = NULL;
- cache_leaves(cpu) = 0;
}
int __weak init_cache_level(unsigned int cpu)
@@ -387,18 +383,53 @@ int __weak populate_cache_leaves(unsigned int cpu)
return -ENOENT;
}
+int allocate_cache_info(unsigned int cpu)
+{
+ struct cpu_cacheinfo *this_cpu_ci;
+ unsigned int levels, split_levels;
+ int ret;
+
+ if (acpi_disabled)
+ ret = init_of_cache_level(cpu);
+ else {
+ ret = acpi_get_cache_info(cpu, &levels, &split_levels);
+ this_cpu_ci = get_cpu_cacheinfo(cpu);
+ this_cpu_ci->num_levels = levels;
+ /*
+ * This assumes that:
+ * - there cannot be any split caches (data/instruction)
+ * above a unified cache
+ * - data/instruction caches come by pair
+ */
+ this_cpu_ci->num_leaves = levels + split_levels;
+ }
+ if (ret < 0)
+ return ret;
+ else if (!cache_leaves(cpu))
+ return -ENOENT;
+
+ per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
+ sizeof(struct cacheinfo), GFP_ATOMIC);
+ if (per_cpu_cacheinfo(cpu) == NULL) {
+ cache_leaves(cpu) = 0;
+ return -ENOMEM;
+ }
+
+ return ret;
+}
+
int detect_cache_attributes(unsigned int cpu)
{
int ret;
- /* Since early detection of the cacheinfo is allowed via this
- * function and this also gets called as CPU hotplug callbacks via
- * cacheinfo_cpu_online, the initialisation can be skipped and only
- * CPU maps can be updated as the CPU online status would be update
- * if called via cacheinfo_cpu_online path.
+ /* Since early initialization/allocation of the cacheinfo is allowed
+ * via allocate_cache_info() and this also gets called as CPU hotplug
+ * callbacks via cacheinfo_cpu_online, the init/alloc can be skipped
+ * as it will happen only once (the cacheinfo memory is never freed).
+ * Just populate the cacheinfo.
*/
if (per_cpu_cacheinfo(cpu))
- goto update_cpu_map;
+ goto populate_leaves;
if (init_cache_level(cpu) || !cache_leaves(cpu))
return -ENOENT;
@@ -410,6 +441,7 @@ int detect_cache_attributes(unsigned int cpu)
return -ENOMEM;
}
+populate_leaves:
/*
* populate_cache_leaves() may completely setup the cache leaves and
* shared_cpu_map or it may leave it partially setup.
@@ -418,7 +450,6 @@ int detect_cache_attributes(unsigned int cpu)
if (ret)
goto free_ci;
-update_cpu_map:
/*
* For systems using DT for cache hierarchy, fw_token
* and shared_cpu_map will be set up here only if they are
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index f992d81d211f..7d390806b788 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -85,6 +85,7 @@ int populate_cache_leaves(unsigned int cpu);
int cache_setup_acpi(unsigned int cpu);
bool last_level_cache_is_valid(unsigned int cpu);
bool last_level_cache_is_shared(unsigned int cpu_x, unsigned int cpu_y);
+int allocate_cache_info(unsigned int cpu);
int detect_cache_attributes(unsigned int cpu);
#ifndef CONFIG_ACPI_PPTT
/*
--
2.25.1
Make init_of_cache_level() return an error code when the cache
information parsing fails to help detecting missing information.
init_of_cache_level() is only called for riscv. Returning an error
code instead of 0 will prevent detect_cache_attributes() to allocate
memory if an incomplete DT is parsed.
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/base/cacheinfo.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index a4308b48dd3e..6f6cd120c4f1 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -246,11 +246,11 @@ int init_of_cache_level(unsigned int cpu)
of_node_put(prev);
prev = np;
if (!of_device_is_compatible(np, "cache"))
- break;
+ goto err_out;
if (of_property_read_u32(np, "cache-level", &level))
- break;
+ goto err_out;
if (level <= levels)
- break;
+ goto err_out;
if (of_property_read_bool(np, "cache-size"))
++leaves;
if (of_property_read_bool(np, "i-cache-size"))
@@ -265,6 +265,10 @@ int init_of_cache_level(unsigned int cpu)
this_cpu_ci->num_leaves = leaves;
return 0;
+
+err_out:
+ of_node_put(np);
+ return -EINVAL;
}
#else
--
2.25.1
+ Rob Herring
+ Jeremy Linton
On 11/8/22 12:04, Pierre Gondois wrote:
> [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 riscv's init_cache_level() as generic OF implem
> 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 | 86 +++++++++++++++++------------
> drivers/base/arch_topology.c | 10 +++-
> drivers/base/cacheinfo.c | 101 ++++++++++++++++++++++++++++++----
> include/linux/cacheinfo.h | 10 +++-
> 6 files changed, 164 insertions(+), 91 deletions(-)
>
acpi_find_last_cache_level() allows to find the last level of cache
for a given CPU. The function is only called on arm64 ACPI based
platforms to check for cache information that would be missing in
the CLIDR_EL1 register.
To allow populating (struct cpu_cacheinfo).num_leaves by only parsing
a PPTT, update acpi_find_last_cache_level() to get the 'split_levels',
i.e. the number of cache levels being split in data/instruction
caches.
It is assumed that there will not be data/instruction caches above a
unified cache.
If a split level consist of one data cache and no instruction cache
(or opposite), then the missing cache will still be populated
by default with minimal cache information, and maximal cpumask
(all non-existing caches have the same fw_token).
Suggested-by: Jeremy Linton <[email protected]>
Signed-off-by: Pierre Gondois <[email protected]>
---
arch/arm64/kernel/cacheinfo.c | 9 +++--
drivers/acpi/pptt.c | 69 ++++++++++++++++++++++++-----------
include/linux/cacheinfo.h | 8 ++--
3 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index 97c42be71338..164255651d64 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -46,7 +46,7 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
int init_cache_level(unsigned int cpu)
{
unsigned int ctype, level, leaves;
- int fw_level;
+ int fw_level, ret;
struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
for (level = 1, leaves = 0; level <= MAX_CACHE_LEVEL; level++) {
@@ -61,8 +61,11 @@ int init_cache_level(unsigned int cpu)
if (acpi_disabled)
fw_level = of_find_last_cache_level(cpu);
- else
- fw_level = acpi_find_last_cache_level(cpu);
+ else {
+ ret = acpi_get_cache_info(cpu, &fw_level, NULL);
+ if (ret < 0)
+ return ret;
+ }
if (fw_level < 0)
return fw_level;
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 97c1d33822d1..72a6ddc1c555 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -81,6 +81,7 @@ static inline bool acpi_pptt_match_type(int table_type, int type)
* acpi_pptt_walk_cache() - Attempt to find the requested acpi_pptt_cache
* @table_hdr: Pointer to the head of the PPTT table
* @local_level: passed res reflects this cache level
+ * @split_levels: Number of split cache levels (data/instruction).
* @res: cache resource in the PPTT we want to walk
* @found: returns a pointer to the requested level if found
* @level: the requested cache level
@@ -100,6 +101,7 @@ static inline bool acpi_pptt_match_type(int table_type, int type)
*/
static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
unsigned int local_level,
+ unsigned int *split_levels,
struct acpi_subtable_header *res,
struct acpi_pptt_cache **found,
unsigned int level, int type)
@@ -113,6 +115,12 @@ static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
while (cache) {
local_level++;
+ if (split_levels && (acpi_pptt_match_type(cache->attributes,
+ ACPI_PPTT_CACHE_TYPE_DATA) ||
+ acpi_pptt_match_type(cache->attributes,
+ ACPI_PPTT_CACHE_TYPE_INSTR)))
+ *split_levels = local_level;
+
if (local_level == level &&
cache->flags & ACPI_PPTT_CACHE_TYPE_VALID &&
acpi_pptt_match_type(cache->attributes, type)) {
@@ -135,8 +143,8 @@ static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
static struct acpi_pptt_cache *
acpi_find_cache_level(struct acpi_table_header *table_hdr,
struct acpi_pptt_processor *cpu_node,
- unsigned int *starting_level, unsigned int level,
- int type)
+ unsigned int *starting_level, unsigned int *split_levels,
+ unsigned int level, int type)
{
struct acpi_subtable_header *res;
unsigned int number_of_levels = *starting_level;
@@ -149,7 +157,8 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
resource++;
local_level = acpi_pptt_walk_cache(table_hdr, *starting_level,
- res, &ret, level, type);
+ split_levels, res, &ret,
+ level, type);
/*
* we are looking for the max depth. Since its potentially
* possible for a given node to have resources with differing
@@ -165,29 +174,33 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
}
/**
- * acpi_count_levels() - Given a PPTT table, and a CPU node, count the caches
+ * acpi_count_levels() - Given a PPTT table, and a CPU node, count the cache
+ * levels and split cache levels (data/instruction).
* @table_hdr: Pointer to the head of the PPTT table
* @cpu_node: processor node we wish to count caches for
+ * @levels: Number of levels if success.
+ * @split_levels: Number of split cache levels (data/instruction) if success.
+ * Can by NULL.
*
* Given a processor node containing a processing unit, walk into it and count
* how many levels exist solely for it, and then walk up each level until we hit
* the root node (ignore the package level because it may be possible to have
- * caches that exist across packages). Count the number of cache levels that
- * exist at each level on the way up.
+ * caches that exist across packages). Count the number of cache levels and
+ * split cache levels (data/instruction) that exist at each level on the way
+ * up.
*
- * Return: Total number of levels found.
+ * Return: 0 on success.
*/
static int acpi_count_levels(struct acpi_table_header *table_hdr,
- struct acpi_pptt_processor *cpu_node)
+ struct acpi_pptt_processor *cpu_node,
+ unsigned int *levels, unsigned int *split_levels)
{
- int total_levels = 0;
-
do {
- acpi_find_cache_level(table_hdr, cpu_node, &total_levels, 0, 0);
+ acpi_find_cache_level(table_hdr, cpu_node, levels, split_levels, 0, 0);
cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
} while (cpu_node);
- return total_levels;
+ return 0;
}
/**
@@ -321,7 +334,7 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
while (cpu_node && !found) {
found = acpi_find_cache_level(table_hdr, cpu_node,
- &total_levels, level, acpi_type);
+ &total_levels, NULL, level, acpi_type);
*node = cpu_node;
cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
}
@@ -589,36 +602,48 @@ static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
}
/**
- * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
+ * acpi_get_cache_info() - Determine the number of cache levels and
+ * split cache levels (data/instruction) and for a PE.
* @cpu: Kernel logical CPU number
+ * @levels: Number of levels if success.
+ * @split_levels: Number of levels being split (i.e. data/instruction)
+ * if success. Can by NULL.
*
* Given a logical CPU number, returns the number of levels of cache represented
* in the PPTT. Errors caused by lack of a PPTT table, or otherwise, return 0
* indicating we didn't find any cache levels.
*
- * Return: Cache levels visible to this core.
+ * Return: -ENOENT if no PPTT table or no PPTT processor struct found.
+ * 0 on success.
*/
-int acpi_find_last_cache_level(unsigned int cpu)
+int acpi_get_cache_info(unsigned int cpu, unsigned int *levels,
+ unsigned int *split_levels)
{
struct acpi_pptt_processor *cpu_node;
struct acpi_table_header *table;
- int number_of_levels = 0;
u32 acpi_cpu_id;
+ *levels = 0;
+ if (split_levels)
+ *split_levels = 0;
+
table = acpi_get_pptt();
if (!table)
return -ENOENT;
- pr_debug("Cache Setup find last level CPU=%d\n", cpu);
+ pr_debug("Cache Setup: find cache levels for CPU=%d\n", cpu);
acpi_cpu_id = get_acpi_id_for_cpu(cpu);
cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
- if (cpu_node)
- number_of_levels = acpi_count_levels(table, cpu_node);
+ if (!cpu_node)
+ return -ENOENT;
- pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
+ acpi_count_levels(table, cpu_node, levels, split_levels);
- return number_of_levels;
+ pr_debug("Cache Setup: last_level=%d split_levels=%d\n",
+ *levels, split_levels ? *split_levels : -1);
+
+ return 0;
}
/**
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index ff0328f3fbb0..f992d81d211f 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -88,19 +88,21 @@ bool last_level_cache_is_shared(unsigned int cpu_x, unsigned int cpu_y);
int detect_cache_attributes(unsigned int cpu);
#ifndef CONFIG_ACPI_PPTT
/*
- * acpi_find_last_cache_level is only called on ACPI enabled
+ * acpi_get_cache_info() is only called on ACPI enabled
* platforms using the PPTT for topology. This means that if
* the platform supports other firmware configuration methods
* we need to stub out the call when ACPI is disabled.
* ACPI enabled platforms not using PPTT won't be making calls
* to this function so we need not worry about them.
*/
-static inline int acpi_find_last_cache_level(unsigned int cpu)
+static inline int acpi_get_cache_info(unsigned int cpu,
+ unsigned int *levels, unsigned int *split_levels)
{
return 0;
}
#else
-int acpi_find_last_cache_level(unsigned int cpu);
+int acpi_get_cache_info(unsigned int cpu,
+ unsigned int *levels, unsigned int *split_levels);
#endif
const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);
--
2.25.1
On Tue, Nov 08, 2022 at 12:04:16PM +0100, Pierre Gondois wrote:
> [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].
FWIW checkpatch whinges a bit at your final two patches [a]. The RISC-V
code movement looks okay though, no new warnings and the like.
[a] - https://patchwork.kernel.org/project/linux-riscv/list/?series=693175
>
> 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 riscv's init_cache_level() as generic OF implem
> 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 | 86 +++++++++++++++++------------
> drivers/base/arch_topology.c | 10 +++-
> drivers/base/cacheinfo.c | 101 ++++++++++++++++++++++++++++++----
> include/linux/cacheinfo.h | 10 +++-
> 6 files changed, 164 insertions(+), 91 deletions(-)
>
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Nov 08, 2022 at 12:04:18PM +0100, Pierre Gondois wrote:
> Make init_of_cache_level() return an error code when the cache
> information parsing fails to help detecting missing information.
>
> init_of_cache_level() is only called for riscv. Returning an error
> code instead of 0 will prevent detect_cache_attributes() to allocate
> memory if an incomplete DT is parsed.
>
Reviewed-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
Hi Pierre,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on driver-core/driver-core-next driver-core/driver-core-linus arm64/for-next/core rafael-pm/linux-next linus/master v6.1-rc4 next-20221108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Pierre-Gondois/arch_topology-Build-cacheinfo-from-primary-CPU/20221108-190738
patch link: https://lore.kernel.org/r/20221108110424.166896-6-pierre.gondois%40arm.com
patch subject: [PATCH 5/5] arch_topology: Build cacheinfo from primary CPU
config: ia64-randconfig-r024-20221108
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1d2759f5c9fe043ae00116c153312eb6de3f4748
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Pierre-Gondois/arch_topology-Build-cacheinfo-from-primary-CPU/20221108-190738
git checkout 1d2759f5c9fe043ae00116c153312eb6de3f4748
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/base/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/base/cacheinfo.c: In function 'allocate_cache_info':
>> drivers/base/cacheinfo.c:404:50: warning: 'levels' is used uninitialized [-Wuninitialized]
404 | this_cpu_ci->num_leaves = levels + split_levels;
| ~~~~~~~^~~~~~~~~~~~~~
drivers/base/cacheinfo.c:389:22: note: 'levels' was declared here
389 | unsigned int levels, split_levels;
| ^~~~~~
>> drivers/base/cacheinfo.c:404:50: warning: 'split_levels' is used uninitialized [-Wuninitialized]
404 | this_cpu_ci->num_leaves = levels + split_levels;
| ~~~~~~~^~~~~~~~~~~~~~
drivers/base/cacheinfo.c:389:30: note: 'split_levels' was declared here
389 | unsigned int levels, split_levels;
| ^~~~~~~~~~~~
vim +/levels +404 drivers/base/cacheinfo.c
385
386 int allocate_cache_info(unsigned int cpu)
387 {
388 struct cpu_cacheinfo *this_cpu_ci;
389 unsigned int levels, split_levels;
390 int ret;
391
392 if (acpi_disabled)
393 ret = init_of_cache_level(cpu);
394 else {
395 ret = acpi_get_cache_info(cpu, &levels, &split_levels);
396 this_cpu_ci = get_cpu_cacheinfo(cpu);
397 this_cpu_ci->num_levels = levels;
398 /*
399 * This assumes that:
400 * - there cannot be any split caches (data/instruction)
401 * above a unified cache
402 * - data/instruction caches come by pair
403 */
> 404 this_cpu_ci->num_leaves = levels + split_levels;
405 }
406 if (ret < 0)
407 return ret;
408 else if (!cache_leaves(cpu))
409 return -ENOENT;
410
411 per_cpu_cacheinfo(cpu) = kcalloc(cache_leaves(cpu),
412 sizeof(struct cacheinfo), GFP_ATOMIC);
413 if (per_cpu_cacheinfo(cpu) == NULL) {
414 cache_leaves(cpu) = 0;
415 return -ENOMEM;
416 }
417
418 return ret;
419 }
420
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi,
Thanks for fixing this!
Aside from one somewhat significant comment below this is all looks fine
to me:
Reviewed-by: Jeremy Linton <[email protected]>
On 11/8/22 05:04, Pierre Gondois wrote:
> acpi_find_last_cache_level() allows to find the last level of cache
> for a given CPU. The function is only called on arm64 ACPI based
> platforms to check for cache information that would be missing in
> the CLIDR_EL1 register.
> To allow populating (struct cpu_cacheinfo).num_leaves by only parsing
> a PPTT, update acpi_find_last_cache_level() to get the 'split_levels',
> i.e. the number of cache levels being split in data/instruction
> caches.
>
> It is assumed that there will not be data/instruction caches above a
> unified cache.
> If a split level consist of one data cache and no instruction cache
> (or opposite), then the missing cache will still be populated
> by default with minimal cache information, and maximal cpumask
> (all non-existing caches have the same fw_token).
>
> Suggested-by: Jeremy Linton <[email protected]>
> Signed-off-by: Pierre Gondois <[email protected]>
> ---
> arch/arm64/kernel/cacheinfo.c | 9 +++--
> drivers/acpi/pptt.c | 69 ++++++++++++++++++++++++-----------
> include/linux/cacheinfo.h | 8 ++--
> 3 files changed, 58 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
> index 97c42be71338..164255651d64 100644
> --- a/arch/arm64/kernel/cacheinfo.c
> +++ b/arch/arm64/kernel/cacheinfo.c
> @@ -46,7 +46,7 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
> int init_cache_level(unsigned int cpu)
> {
> unsigned int ctype, level, leaves;
> - int fw_level;
> + int fw_level, ret;
> struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
>
> for (level = 1, leaves = 0; level <= MAX_CACHE_LEVEL; level++) {
> @@ -61,8 +61,11 @@ int init_cache_level(unsigned int cpu)
>
> if (acpi_disabled)
> fw_level = of_find_last_cache_level(cpu);
> - else
> - fw_level = acpi_find_last_cache_level(cpu);
> + else {
> + ret = acpi_get_cache_info(cpu, &fw_level, NULL);
> + if (ret < 0)
> + return ret;
> + }
>
> if (fw_level < 0)
> return fw_level;
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> index 97c1d33822d1..72a6ddc1c555 100644
> --- a/drivers/acpi/pptt.c
> +++ b/drivers/acpi/pptt.c
> @@ -81,6 +81,7 @@ static inline bool acpi_pptt_match_type(int table_type, int type)
> * acpi_pptt_walk_cache() - Attempt to find the requested acpi_pptt_cache
> * @table_hdr: Pointer to the head of the PPTT table
> * @local_level: passed res reflects this cache level
> + * @split_levels: Number of split cache levels (data/instruction).
> * @res: cache resource in the PPTT we want to walk
> * @found: returns a pointer to the requested level if found
> * @level: the requested cache level
> @@ -100,6 +101,7 @@ static inline bool acpi_pptt_match_type(int table_type, int type)
> */
> static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
> unsigned int local_level,
> + unsigned int *split_levels,
> struct acpi_subtable_header *res,
> struct acpi_pptt_cache **found,
> unsigned int level, int type)
> @@ -113,6 +115,12 @@ static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
> while (cache) {
> local_level++;
>
> + if (split_levels && (acpi_pptt_match_type(cache->attributes,
> + ACPI_PPTT_CACHE_TYPE_DATA) ||
> + acpi_pptt_match_type(cache->attributes,
> + ACPI_PPTT_CACHE_TYPE_INSTR)))
> + *split_levels = local_level;
> +
I think for maximum defense/spec compliance you need to validate the
CACHE_TYPE_VALID flag before checking this. As its done in the next line
too, it might be cleaner to consolidate the two checks.
> if (local_level == level &&
> cache->flags & ACPI_PPTT_CACHE_TYPE_VALID &&
> acpi_pptt_match_type(cache->attributes, type)) {
> @@ -135,8 +143,8 @@ static unsigned int acpi_pptt_walk_cache(struct acpi_table_header *table_hdr,
> static struct acpi_pptt_cache *
> acpi_find_cache_level(struct acpi_table_header *table_hdr,
> struct acpi_pptt_processor *cpu_node,
> - unsigned int *starting_level, unsigned int level,
> - int type)
> + unsigned int *starting_level, unsigned int *split_levels,
> + unsigned int level, int type)
> {
> struct acpi_subtable_header *res;
> unsigned int number_of_levels = *starting_level;
> @@ -149,7 +157,8 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
> resource++;
>
> local_level = acpi_pptt_walk_cache(table_hdr, *starting_level,
> - res, &ret, level, type);
> + split_levels, res, &ret,
> + level, type);
> /*
> * we are looking for the max depth. Since its potentially
> * possible for a given node to have resources with differing
> @@ -165,29 +174,33 @@ acpi_find_cache_level(struct acpi_table_header *table_hdr,
> }
>
> /**
> - * acpi_count_levels() - Given a PPTT table, and a CPU node, count the caches
> + * acpi_count_levels() - Given a PPTT table, and a CPU node, count the cache
> + * levels and split cache levels (data/instruction).
> * @table_hdr: Pointer to the head of the PPTT table
> * @cpu_node: processor node we wish to count caches for
> + * @levels: Number of levels if success.
> + * @split_levels: Number of split cache levels (data/instruction) if success.
> + * Can by NULL.
nitpick: I think usually you want to indent/align multiple line argument
comments so its clear your talking about the argument.
See:
https://elixir.bootlin.com/linux/v6.1-rc4/source/Documentation/doc-guide/kernel-doc.rst#L107
> *
> * Given a processor node containing a processing unit, walk into it and count
> * how many levels exist solely for it, and then walk up each level until we hit
> * the root node (ignore the package level because it may be possible to have
> - * caches that exist across packages). Count the number of cache levels that
> - * exist at each level on the way up.
> + * caches that exist across packages). Count the number of cache levels and
> + * split cache levels (data/instruction) that exist at each level on the way
> + * up.
> *
> - * Return: Total number of levels found.
> + * Return: 0 on success.
> */
> static int acpi_count_levels(struct acpi_table_header *table_hdr,
> - struct acpi_pptt_processor *cpu_node)
> + struct acpi_pptt_processor *cpu_node,
> + unsigned int *levels, unsigned int *split_levels)
> {
> - int total_levels = 0;
> - > do {
> - acpi_find_cache_level(table_hdr, cpu_node, &total_levels, 0, 0);
> + acpi_find_cache_level(table_hdr, cpu_node, levels, split_levels, 0, 0);
> cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
> } while (cpu_node);
>
> - return total_levels;
> + return 0;
> }
another nitpick: You might consider just making this "void" if the
return value is always 0, and not checked later.
>
> /**
> @@ -321,7 +334,7 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
>
> while (cpu_node && !found) {
> found = acpi_find_cache_level(table_hdr, cpu_node,
> - &total_levels, level, acpi_type);
> + &total_levels, NULL, level, acpi_type);
> *node = cpu_node;
> cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
> }
> @@ -589,36 +602,48 @@ static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
> }
>
> /**
> - * acpi_find_last_cache_level() - Determines the number of cache levels for a PE
> + * acpi_get_cache_info() - Determine the number of cache levels and
> + * split cache levels (data/instruction) and for a PE.
> * @cpu: Kernel logical CPU number
> + * @levels: Number of levels if success.
> + * @split_levels: Number of levels being split (i.e. data/instruction)
> + * if success. Can by NULL.nitpick: same as above
> *
> * Given a logical CPU number, returns the number of levels of cache represented
> * in the PPTT. Errors caused by lack of a PPTT table, or otherwise, return 0
> * indicating we didn't find any cache levels.
> *
> - * Return: Cache levels visible to this core.
> + * Return: -ENOENT if no PPTT table or no PPTT processor struct found.
> + * 0 on success.
> */
> -int acpi_find_last_cache_level(unsigned int cpu)
> +int acpi_get_cache_info(unsigned int cpu, unsigned int *levels,
> + unsigned int *split_levels)
> {
> struct acpi_pptt_processor *cpu_node;
> struct acpi_table_header *table;
> - int number_of_levels = 0;
> u32 acpi_cpu_id;
>
> + *levels = 0;
> + if (split_levels)
> + *split_levels = 0;
> +
> table = acpi_get_pptt();
> if (!table)
> return -ENOENT;
>
> - pr_debug("Cache Setup find last level CPU=%d\n", cpu);
> + pr_debug("Cache Setup: find cache levels for CPU=%d\n", cpu);
>
> acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
> - if (cpu_node)
> - number_of_levels = acpi_count_levels(table, cpu_node);
> + if (!cpu_node)
> + return -ENOENT;
>
> - pr_debug("Cache Setup find last level level=%d\n", number_of_levels);
> + acpi_count_levels(table, cpu_node, levels, split_levels);
>
> - return number_of_levels;
> + pr_debug("Cache Setup: last_level=%d split_levels=%d\n",
> + *levels, split_levels ? *split_levels : -1);
> +
> + return 0;
> }
>
> /**
> diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
> index ff0328f3fbb0..f992d81d211f 100644
> --- a/include/linux/cacheinfo.h
> +++ b/include/linux/cacheinfo.h
> @@ -88,19 +88,21 @@ bool last_level_cache_is_shared(unsigned int cpu_x, unsigned int cpu_y);
> int detect_cache_attributes(unsigned int cpu);
> #ifndef CONFIG_ACPI_PPTT
> /*
> - * acpi_find_last_cache_level is only called on ACPI enabled
> + * acpi_get_cache_info() is only called on ACPI enabled
> * platforms using the PPTT for topology. This means that if
> * the platform supports other firmware configuration methods
> * we need to stub out the call when ACPI is disabled.
> * ACPI enabled platforms not using PPTT won't be making calls
> * to this function so we need not worry about them.
> */
> -static inline int acpi_find_last_cache_level(unsigned int cpu)
> +static inline int acpi_get_cache_info(unsigned int cpu,
> + unsigned int *levels, unsigned int *split_levels)
> {
> return 0;
> }
> #else
> -int acpi_find_last_cache_level(unsigned int cpu);
> +int acpi_get_cache_info(unsigned int cpu,
> + unsigned int *levels, unsigned int *split_levels);
And as a final comment your free to ignore: It might reduce some patch
churn to just add split_levels and leave the return behavior alone.
> #endif
>
> const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);