linux-kernel - Re: [PATCH 1/4] mm/vmalloc: Remove unnecessary size ALIGN in __vmalloc_node_range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8giNq5CMtbYnlo-@pc636>
Date: Wed, 5 Mar 2025 11:06:46 +0100
From: Uladzislau Rezki <urezki@...il.com>
To: Baoquan He <bhe@...hat.com>, liuye <liuye@...inos.cn>
Cc: liuye <liuye@...inos.cn>, Uladzislau Rezki <urezki@...il.com>,
	akpm@...ux-foundation.org, hch@...radead.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] mm/vmalloc: Remove unnecessary size ALIGN in
 __vmalloc_node_range_noprof

On Wed, Mar 05, 2025 at 06:02:19PM +0800, Baoquan He wrote:
> On 03/05/25 at 09:46am, liuye wrote:
> > 
> > 在 2025/3/4 02:30, Uladzislau Rezki 写道:
> > > On Mon, Mar 03, 2025 at 05:44:07PM +0800, Liu Ye wrote:
> > >> The same operation already exists in the function __get_vm_area_node,
> > >> so delete the duplicate operation to simplify the code.
> > >>
> > >> Signed-off-by: Liu Ye <liuye@...inos.cn>
> > >> ---
> > >>  mm/vmalloc.c | 1 -
> > >>  1 file changed, 1 deletion(-)
> > >>
> > >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > >> index dc658d4af181..20d9b9de84b1 100644
> > >> --- a/mm/vmalloc.c
> > >> +++ b/mm/vmalloc.c
> > >> @@ -3798,7 +3798,6 @@ void *__vmalloc_node_range_noprof(unsigned long size, unsigned long align,
> > >>  			shift = arch_vmap_pte_supported_shift(size);
> > >>  
> > >>  		align = max(real_align, 1UL << shift);
> > >> -		size = ALIGN(real_size, 1UL << shift);
> > >>  	}
> > >>  
> > >>  again:
> > >> -- 
> > >> 2.25.1
> > >>
> > > There is a mess with:
> > >
> > >  unsigned long real_size = size;
> > >  unsigned long real_align = align;
> > >
> > > "real_size" and "real_align". Those are useless. What is about:
> > 
> > Sorry, the order of the patches may be misleading.
> > 
> > The correct order is as follows：
> > 
> > PATCH1.  mm/vmalloc: Size should be used instead of real_size "
> > PATCH2.  mm/vmalloc: Remove unnecessary size ALIGN in __vmalloc_node_range_noprof                 
> > PATCH3.  mm/vmalloc: Remove the real_size variable to simplify the code "
> > PATCH4.  mm/vmalloc: Rename the variable real_align to original_align to prevent misunderstanding
> > 
> > If PATCH1 is the correct fix, then consider PATCH2, PATCH3, and PATCH4.
> 
> Well, seems the patch split is done too subtly. It's only about the
> size/align inside one function, maybe one patch is enough in this case.
> My personal opinion.
> 
I agree. One patch would be enough.

--
Uladzislau Rezki