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

[Dave Anderson]

----- Original Message -----
> 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).

OK great, that works nicely -- let's go with that.  

Both patches are now queued for crash-7.1.5:

  https://github.com/crash-utility/crash/commit/f6918567d72bd1ca98dd4119092e25157961e3c2
  https://github.com/crash-utility/crash/commit/7f324a41449728fb0a1c36b2264a69aeb65a87b1
  
Thanks,
  Dave




> 
> 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>
>