[Crash-utility] [PATCH 1/2] Fix cpu_slab freelist handling on SLUB

[OGAWA Hirofumi]

Dave Anderson <anderson at redhat.com> writes:

> ----- Original Message -----
>> OGAWA Hirofumi <hirofumi at mail.parknet.co.jp> 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 <hirofumi at mail.parknet.co.jp>