Re: [PATCH v2 sl-b 3/5] mm: Make mem_dump_obj() handle vmalloc() memory

[Vlastimil Babka <vbabka@suse.cz>]

On 12/10/20 12:23 AM, Paul E. McKenney wrote:
> On Wed, Dec 09, 2020 at 06:51:20PM +0100, Vlastimil Babka wrote:
>> On 12/9/20 2:13 AM, paulmck@kernel.org wrote:
>> > From: "Paul E. McKenney" <paulmck@kernel.org>
>> > 
>> > This commit adds vmalloc() support to mem_dump_obj().  Note that the
>> > vmalloc_dump_obj() function combines the checking and dumping, in
>> > contrast with the split between kmem_valid_obj() and kmem_dump_obj().
>> > The reason for the difference is that the checking in the vmalloc()
>> > case involves acquiring a global lock, and redundant acquisitions of
>> > global locks should be avoided, even on not-so-fast paths.
>> > 
>> > Note that this change causes on-stack variables to be reported as
>> > vmalloc() storage from kernel_clone() or similar, depending on the degree
>> > of inlining that your compiler does.  This is likely more helpful than
>> > the earlier "non-paged (local) memory".
>> > 
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>> > Cc: <linux-mm@kvack.org>
>> > Reported-by: Andrii Nakryiko <andrii@kernel.org>
>> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>> 
>> ...
>> 
>> > --- a/mm/vmalloc.c
>> > +++ b/mm/vmalloc.c
>> > @@ -3431,6 +3431,18 @@ void pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
>> >  }
>> >  #endif	/* CONFIG_SMP */
>> >  
>> > +bool vmalloc_dump_obj(void *object)
>> > +{
>> > +	struct vm_struct *vm;
>> > +	void *objp = (void *)PAGE_ALIGN((unsigned long)object);
>> > +
>> > +	vm = find_vm_area(objp);
>> > +	if (!vm)
>> > +		return false;
>> > +	pr_cont(" vmalloc allocated at %pS\n", vm->caller);
>> 
>> Would it be useful to print the vm area boundaries too?
> 
> Like this?

Yeah, thanks!

> I also considered instead using vm->size, but that always seems to include
> an extra page, so a 4-page span is listed as having 20480 bytes and a
> one-page span is 8192 bytes.  This might be more accurate in some sense,
> but would be quite confusing to someone trying to compare this size with
> that requested in the vmalloc() call.

Right.

> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> commit 33e0469c289c2f78e5f0d0c463c8ee3357d273c0
> Author: Paul E. McKenney <paulmck@kernel.org>
> Date:   Wed Dec 9 15:15:27 2020 -0800
> 
>     mm: Make mem_obj_dump() vmalloc() dumps include start and length
>     
>     This commit adds the starting address and number of pages to the vmalloc()
>     information dumped by way of vmalloc_dump_obj().
>     
>     Cc: Andrew Morton <akpm@linux-foundation.org>
>     Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>     Cc: <linux-mm@kvack.org>
>     Reported-by: Andrii Nakryiko <andrii@kernel.org>
>     Suggested-by: Vlastimil Babka <vbabka@suse.cz>
>     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 7421719..77b1100 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3439,7 +3439,8 @@ bool vmalloc_dump_obj(void *object)
>  	vm = find_vm_area(objp);
>  	if (!vm)
>  		return false;
> -	pr_cont(" vmalloc allocated at %pS\n", vm->caller);
> +	pr_cont(" %u-page vmalloc region starting at %#lx allocated at %pS\n",
> +		vm->nr_pages, (unsigned long)vm->addr, vm->caller);
>  	return true;
>  }
>  
>