Dave Anderson <[email protected]> writes:

> ----- Original Message -----
>> OGAWA Hirofumi <[email protected]> writes:
>> 
>> OK. More simpler proof, the following is enough to convince you?

[...]

> But going back to the error report, the "slab: 0" is kind of confusing:
>
>   crash> kmem -s kmalloc-32
>   CACHE            NAME                 OBJSIZE  ALLOCATED     TOTAL  SLABS  
> SSIZE
>   kmem: kmalloc-32: slab: 0 invalid freepointer: ffff001090e33f80
>   ffff880333001c00 kmalloc-32                32     122658    125440    980   
>   4k
>   crash>

[...]

> I'm thinking we should clarify that error message, perhaps by storing the cpu
> number in si->cpu, and displaying it when si->slab is NULL?

Just a idea for now though (means not tested error path at all), how
about the following?

We know already slab pointer at that point. So, this sets si->slab
temporarily. (and slub_page_objects() just uses slab in argument, not
si->slab).

Thanks.

---

 memory.c |   23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff -puN memory.c~crash-slub-freelist-error-fix memory.c
--- crash-64/memory.c~crash-slub-freelist-error-fix     2016-04-21 
05:04:11.660627253 +0900
+++ crash-64-hirofumi/memory.c  2016-04-21 05:20:01.673010107 +0900
@@ -17947,12 +17947,12 @@ static ushort slub_page_objects(struct m
                if (CRASHDEBUG(1) && (objects != si->objects))
                        error(NOTE, "%s: slab: %lx oo objects: %ld "
                              "slab objects: %d\n",
-                             si->curname, si->slab,
+                             si->curname, page,
                              si->objects, objects);
 
                if (objects == (ushort)(-1)) {
                        error(INFO, "%s: slab: %lx invalid page.objects: -1\n",
-                             si->curname, si->slab);
+                             si->curname, page);
                        return 0;
                }
        } else
@@ -18039,14 +18039,22 @@ get_kmem_cache_slub_data(long cmd, struc
 
                switch (cmd)
                {
-               case GET_SLUB_OBJECTS:
+               case GET_SLUB_OBJECTS: {
+                       /* For better error report, set cur slab to si->slab. */
+                       ulong orig_slab = si->slab;
+                       si->slab = cpu_slab_ptr;
+
                        if (!readmem(cpu_slab_ptr + OFFSET(page_inuse), 
-                           KVADDR, &inuse, sizeof(short), 
-                           "page inuse", RETURN_ON_ERROR))
+                                    KVADDR, &inuse, sizeof(short),
+                                    "page inuse", RETURN_ON_ERROR)) {
+                               si->slab = orig_slab;
                                return FALSE;
+                       }
                        objects = slub_page_objects(si, cpu_slab_ptr);
-                       if (!objects)
+                       if (!objects) {
+                               si->slab = orig_slab;
                                return FALSE;
+                       }
 
                        free_objects += objects - inuse;
                        free_objects += count_free_objects(si, cpu_freelist);
@@ -18055,6 +18063,9 @@ get_kmem_cache_slub_data(long cmd, struc
                        if (!node_total_avail)
                                total_objects += inuse;
                        total_slabs++;
+
+                       si->slab = orig_slab;
+               }
                        break;
 
                case GET_SLUB_SLABS:
_
-- 
OGAWA Hirofumi <[email protected]>

--
Crash-utility mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to