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

Vlastimil Babka
2020-12-10T11:39:50Z

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