[Crash-utility] Re: [PATCH] mem: rename variable for clarity in get_kmem_cache_slub_data()
Hi liuye,
On Fri, Sep 5, 2025 at 12:29 AM <liuye(a)kylinos.cn> wrote:
From 33e22a832bdf79b032dd9d4ceac53fead5c2f552 Mon Sep 17 00:00:00 2001
From: Ye Liu <liuye(a)kylinos.cn>
Date: Wed, 3 Sep 2025 15:18:57 +0800
Subject: [PATCH] mem: rename variable for clarity in
get_kmem_cache_slub_data()
Rename 'cpu_slab_ptr' to 'cpu_slab_page_ptr' to better reflect that
it holds a pointer to a page structure rather than a generic slab pointer.
This improves code readability and consistency with related functions.
No functional changes.
Frankly I don't think the rename is necessary:
1) It is a local variable which only exist within function
get_kmem_cache_slub_data(), the context relating to the r/w of the
variable is simple and clear, such as:
cpu_slab_ptr = ULONG(si->cache_buf +
OFFSET(kmem_cache_cpu_slab) +
OFFSET(kmem_cache_cpu_page));
Easy for people to understand cpu_slab_ptr comes from kmem_cache ->
slab -> page.
2) The cpu_slab_ptr variable have different meaning in different stage, e.g:
stage1:
cpu_slab_ptr = ULONG(si->cache_buf +
OFFSET(kmem_cache_cpu_slab) + (sizeof(void *)*cpu));
readmem(cpu_slab_ptr + OFFSET(kmem_cache_cpu_page),
KVADDR, &page, sizeof(void *),
stage2:
cpu_slab_ptr = page;
your cpu_slab_page_ptr renaming only makes sense for the stage2. To
me, the cpu_slab_ptr acts like a tmp ptr in this case, so it doesn't
matter to me what name it should be. To avoid necessary patches, I'd
prefer not to accept this patch unless you have more persuasive
reasons.
Thanks,
Tao Liu
Signed-off-by: Ye Liu <liuye(a)kylinos.cn>
---
memory.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/memory.c b/memory.c
index 400d31a04cd4..be5c590003a8 100644
--- a/memory.c
+++ b/memory.c
@@ -19426,7 +19426,7 @@ get_kmem_cache_slub_data(long cmd, struct meminfo *si)
{
int i, n, node;
ulong total_objects, total_slabs, free_objects;
- ulong cpu_slab_ptr, node_ptr, cpu_freelist, orig_slab;
+ ulong cpu_slab_page_ptr, node_ptr, cpu_freelist, orig_slab;
ulong node_nr_partial, node_nr_slabs, node_total_objects;
int full_slabs, objects, node_total_avail;
long p;
@@ -19445,12 +19445,12 @@ get_kmem_cache_slub_data(long cmd, struct meminfo *si)
node_total_avail = VALID_MEMBER(kmem_cache_node_total_objects) ? TRUE : FALSE;
for (i = 0; i < kt->cpus; i++) {
- cpu_slab_ptr = get_cpu_slab_ptr(si, i, &cpu_freelist);
+ cpu_slab_page_ptr = get_cpu_slab_ptr(si, i, &cpu_freelist);
- if (!cpu_slab_ptr)
+ if (!cpu_slab_page_ptr)
continue;
- if ((node = page_to_nid(cpu_slab_ptr)) < 0)
+ if ((node = page_to_nid(cpu_slab_page_ptr)) < 0)
goto bailout;
switch (cmd)
@@ -19458,15 +19458,15 @@ get_kmem_cache_slub_data(long cmd, struct meminfo *si)
case GET_SLUB_OBJECTS: {
/* For better error report, set cur slab to si->slab. */
orig_slab = si->slab;
- si->slab = cpu_slab_ptr;
+ si->slab = cpu_slab_page_ptr;
- if (!readmem(cpu_slab_ptr + OFFSET(page_inuse),
+ if (!readmem(cpu_slab_page_ptr + OFFSET(page_inuse),
KVADDR, &inuse, sizeof(short),
"page inuse", RETURN_ON_ERROR)) {
si->slab = orig_slab;
return FALSE;
}
- objects = slub_page_objects(si, cpu_slab_ptr);
+ objects = slub_page_objects(si, cpu_slab_page_ptr);
if (!objects) {
si->slab = orig_slab;
return FALSE;
--
2.43.0
--
Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io
To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki