Re: [PATCH 1/1] module: check if memory leak by module.

From: Pavel Machek <pavel@ucw.cz>
To: Maninder Singh <maninder1.s@samsung.com>
Cc: jeyu@redhat.com, rusty@rustcorp.com.au,
	akpm@linux-foundation.org, chris@chris-wilson.co.uk,
	aryabinin@virtuozzo.com, joonas.lahtinen@linux.intel.com,
	mhocko@suse.com, keescook@chromium.org, jinb.park7@gmail.com,
	anisse@astier.eu, rafael.j.wysocki@intel.com, zijun_hu@htc.com,
	mingo@kernel.org, mawilcox@microsoft.com, thgarnie@google.com,
	joelaf@google.com, kirill.shutemov@linux.intel.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	pankaj.m@samsung.com, ajeet.y@samsung.com,
	hakbong5.lee@samsung.com, a.sahrawat@samsung.com,
	lalit.mohan@samsung.com, cpgs@samsung.com,
	Vaneet Narang <v.narang@samsung.com>
Subject: Re: [PATCH 1/1] module: check if memory leak by module.
Date: Mon, 27 Mar 2017 08:43:25 +0200	[thread overview]
Message-ID: <20170327064325.GA27625@amd> (raw)
In-Reply-To: <1490355028-13292-1-git-send-email-maninder1.s@samsung.com>

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

Hi!

> This patch adds new config VMALLOC_MEMORY_LEAK to check if any
> module which is going to be unloaded is doing vmalloc memory leak.
> 
> Logs:-
> [  129.336368] Module vmalloc is getting unloaded before doing vfree
> [  129.336371] Memory still allocated: addr:0xffffc90001461000 - 0xffffc900014c7000, pages 101
> [  129.336376] Allocating function kernel_init+0x1c/0x20 [vmalloc]
> 
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>

Let me see...

> diff --git a/kernel/module.c b/kernel/module.c
> index 529efae..b492f34 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2082,9 +2082,37 @@ void __weak module_arch_freeing_init(struct module *mod)
>  {
>  }
>  
> +#ifdef CONFIG_VMALLOC_MEMORY_LEAK

I'd not make this optional -- the performance cost is not all that
big, right?

> +static void check_memory_leak(struct module *mod)
> +{
> +	struct vmap_area *va;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(va, &vmap_area_list, list) {
> +		if (!(va->flags & VM_VM_AREA))
> +			continue;
> +		if ((mod->core_layout.base < va->vm->caller) &&
> +			(mod->core_layout.base +  mod->core_layout.size) > va->vm->caller) {

Two spaces after "+".

> +			pr_alert("Module %s is getting unloaded before doing vfree\n", mod->name);
> +			pr_alert("Memory still allocated: addr:0x%lx - 0x%lx, pages %u\n",
> +				va->va_start, va->va_end, va->vm->nr_pages);
> +			pr_alert("Allocating function %pS\n", va->vm->caller);
> +		}

Plain pr_err() would be preffered. Its just a memory leak.

Otherwise looks good to me..
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]