Re: + kasan-avoid-sleepable-page-allocation-from-atomic-context-v7.patch added to mm-hotfixes-unstable branch

[Harry Yoo <harry.yoo@oracle.com>]

On Mon, May 12, 2025 at 03:23:07PM -0700, Andrew Morton wrote:
> 
> The patch titled
>      Subject: kasan-avoid-sleepable-page-allocation-from-atomic-context-v7
> has been added to the -mm mm-hotfixes-unstable branch.  Its filename is
>      kasan-avoid-sleepable-page-allocation-from-atomic-context-v7.patch
> 
> This patch will shortly appear at
>      https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/kasan-avoid-sleepable-page-allocation-from-atomic-context-v7.patch__;!!ACWV5N9M2RV99hQ!IDZGs5LvdWUbyEOsjCGQ9n4Rk5nHMYzHfbAvvr0U07eEBuITVQjUmFA0nvwv6w4E_2SDwb99rlhHW1qS4ZW84Q$ 
> 
> This patch will later appear in the mm-hotfixes-unstable branch at
>     git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> 
> Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
> 
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
> 
> The -mm tree is included into linux-next via the mm-everything
> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there every 2-3 working days
> 
> ------------------------------------------------------
> From: Alexander Gordeev <agordeev@linux.ibm.com>
> Subject: kasan-avoid-sleepable-page-allocation-from-atomic-context-v7
> Date: Mon, 12 May 2025 16:27:06 +0200
> 
> do not unnecessary free pages across iterations
> 
> Link: https://urldefense.com/v3/__https://lkml.kernel.org/r/c8eeeb146382bcadabce5b5dcf92e6176ba4fb04.1747059374.git.agordeev@linux.ibm.com__;!!ACWV5N9M2RV99hQ!IDZGs5LvdWUbyEOsjCGQ9n4Rk5nHMYzHfbAvvr0U07eEBuITVQjUmFA0nvwv6w4E_2SDwb99rlhHW1oEPUM6Aw$ 
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> Suggested-by: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> Cc: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---

Andrew, please drop this patch                                                  
(kasan-avoid-sleepable-page-allocation-from-atomic-context-v7.patch).           
                                                                                
v7 has a memory leak and after discussing with Alexander this                   
'optimization' doesn't seem worth here.                                         
                                                                                
But as Alexander mentioned v6 also has a memory leak in an error case,
and I believe a proper fix to that should be (lightly tested on my box)
below.

And with the change below on top of v6, it looks good to me.
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index 660cc2148575..8212a7007b02 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -339,7 +339,7 @@ static int __kasan_populate_vmalloc(unsigned long start, unsigned long end)
 {
 	unsigned long nr_populated, nr_pages, nr_total = PFN_UP(end - start);
 	struct vmalloc_populate_data data;
-	int ret;
+	int ret = 0;
 
 	data.pages = (struct page **)__get_free_page(GFP_KERNEL | __GFP_ZERO);
 	if (!data.pages)
@@ -350,8 +350,8 @@ static int __kasan_populate_vmalloc(unsigned long start, unsigned long end)
 		nr_populated = alloc_pages_bulk(GFP_KERNEL, nr_pages, data.pages);
 		if (nr_populated != nr_pages) {
 			free_pages_bulk(data.pages, nr_populated);
-			free_page((unsigned long)data.pages);
-			return -ENOMEM;
+			ret = -ENOMEM;
+			break;
 		}
 
 		data.start = start;
@@ -359,7 +359,7 @@ static int __kasan_populate_vmalloc(unsigned long start, unsigned long end)
 					  kasan_populate_vmalloc_pte, &data);
 		free_pages_bulk(data.pages, nr_pages);
 		if (ret)
-			return ret;
+			break;
 
 		start += nr_pages * PAGE_SIZE;
 		nr_total -= nr_pages;
@@ -367,7 +367,7 @@ static int __kasan_populate_vmalloc(unsigned long start, unsigned long end)
 
 	free_page((unsigned long)data.pages);
 
-	return 0;
+	return ret;
 }
 
 int kasan_populate_vmalloc(unsigned long addr, unsigned long size)

-- 
Cheers,
Harry / Hyeonggon