linux-kernel - Re: [PATCH v7 1/2] mm/vmalloc: Moved macros with no functional change happened

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZevrkCCDG/7WKBYm@MiWiFi-R3L-srv>
Date: Sat, 9 Mar 2024 12:54:40 +0800
From: Baoquan He <bhe@...hat.com>
To: Uladzislau Rezki <urezki@...il.com>
Cc: rulinhuang <rulin.huang@...el.com>, akpm@...ux-foundation.org,
	colin.king@...el.com, hch@...radead.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	lstoakes@...il.com, tianyou.li@...el.com, tim.c.chen@...el.com,
	wangyang.guo@...el.com, zhiguo.zhou@...el.com
Subject: Re: [PATCH v7 1/2] mm/vmalloc: Moved macros with no functional
 change happened

On 03/08/24 at 11:28am, Uladzislau Rezki wrote:
> > > I would remove it, because it is really hard to mess it, there is only
> > > one place also BUG_ON() is really a show stopper. I really appreciate
> > > what rulinhuang <rulin.huang@...el.com> is doing and i understand that
> > > it might be not so easy.
> > 
> > I agree, I was hesitant, now it firms up my mind.
> > 
> > > 
> > > So, if we can avoid of moving the code, that looks to me that we can do,
> > > if we can pass less arguments into alloc_vmap_area() since it is overloaded 
> > > that would be great.
> > 
> > Agree too, less arguments is much better. While I personnally prefer the open
> > coding a little bit like below. There is suspicion of excessive packaging in
> > __pre/__post_setup_vmalloc_vm() wrapping. They are very simple and few
> > assignments after all. 
> > 
> > ---
> >  mm/vmalloc.c | 20 ++++++++++++--------
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 0fd8ebaad17b..0c738423976d 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -1924,8 +1924,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> >  				unsigned long align,
> >  				unsigned long vstart, unsigned long vend,
> >  				int node, gfp_t gfp_mask,
> > -				unsigned long va_flags, struct vm_struct *vm,
> > -				unsigned long flags, const void *caller)
> > +				unsigned long va_flags, struct vm_struct *vm)
> >  {
> >  	struct vmap_node *vn;
> >  	struct vmap_area *va;
> > @@ -1988,8 +1987,11 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> >  	va->vm = NULL;
> >  	va->flags = (va_flags | vn_id);
> >  
> > -	if (vm)
> > -		setup_vmalloc_vm(vm, va, flags, caller);
> > +	if (vm) {
> > +		vm->addr = (void *)va->va_start;
> > +		vm->size = va->va_end - va->va_start;
> > +		va->vm = vm;
> > +	}
> >  
> >  	vn = addr_to_node(va->va_start);
> >  
> > @@ -2565,8 +2567,7 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
> >  	va = alloc_vmap_area(VMAP_BLOCK_SIZE, VMAP_BLOCK_SIZE,
> >  					VMALLOC_START, VMALLOC_END,
> >  					node, gfp_mask,
> > -					VMAP_RAM|VMAP_BLOCK, NULL,
> > -					0, NULL);
> > +					VMAP_RAM|VMAP_BLOCK, NULL);
> >  	if (IS_ERR(va)) {
> >  		kfree(vb);
> >  		return ERR_CAST(va);
> > @@ -2924,7 +2925,7 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node)
> >  		va = alloc_vmap_area(size, PAGE_SIZE,
> >  				VMALLOC_START, VMALLOC_END,
> >  				node, GFP_KERNEL, VMAP_RAM,
> > -				NULL, 0, NULL);
> > +				NULL);
> >  		if (IS_ERR(va))
> >  			return NULL;
> >  
> > @@ -3063,7 +3064,10 @@ static struct vm_struct *__get_vm_area_node(unsigned long size,
> >  	if (!(flags & VM_NO_GUARD))
> >  		size += PAGE_SIZE;
> >  
> > -	va = alloc_vmap_area(size, align, start, end, node, gfp_mask, 0, area, flags, caller);
> > +	area->flags = flags;
> > +	area->caller = caller;
> > +
> > +	va = alloc_vmap_area(size, align, start, end, node, gfp_mask, 0, area);
> >  	if (IS_ERR(va)) {
> >  		kfree(area);
> >  		return NULL;
> > -- 
> > 2.41.0
> > 
> Reviewed-by: Uladzislau Rezki (Sony) <urezki@...il.com>
> 
> Looks even better :) It can be applied on on top of:
> 
> [PATCH v8] mm/vmalloc: Eliminated the lock contention from twice to once
> 
> We are a bit ahead since v8 will be taken later. Anyway please use the
> reviewed-by tag once you send a complete patch. 

Thanks, have posted.