linux-kernel - Re: [RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap

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: <20181214181322.GC10600@bombadil.infradead.org>
Date:   Fri, 14 Dec 2018 10:13:22 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Roman Gushchin <guroan@...il.com>
Cc:     linux-mm@...ck.org, Alexey Dobriyan <adobriyan@...il.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Michal Hocko <mhocko@...e.com>,
        Vlastimil Babka <vbabka@...e.cz>, linux-kernel@...r.kernel.org,
        kernel-team@...com, Roman Gushchin <guro@...com>
Subject: Re: [RFC 2/4] mm: separate memory allocation and actual work in
 alloc_vmap_area()

On Fri, Dec 14, 2018 at 10:07:18AM -0800, Roman Gushchin wrote:
> alloc_vmap_area() is allocating memory for the vmap_area, and
> performing the actual lookup of the vm area and vmap_area
> initialization.
> 
> This prevents us from using a pre-allocated memory for the map_area
> structure, which can be used in some cases to minimize the number
> of required memory allocations.
> 
> Let's keep the memory allocation part in alloc_vmap_area() and
> separate everything else into init_vmap_area().

Oh good, I was considering doing something like this eventually.

> +++ b/mm/vmalloc.c
> @@ -395,16 +395,11 @@ static void purge_vmap_area_lazy(void);
>  
>  static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
>  
> -/*
> - * Allocate a region of KVA of the specified size and alignment, within the
> - * vstart and vend.
> - */
> -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)
> +static int init_vmap_area(struct vmap_area *va, unsigned long size,
> +			  unsigned long align, unsigned long vstart,
> +			  unsigned long vend, int node, gfp_t gfp_mask)
>  {
> -	struct vmap_area *va;
> +

Why the blank line?

>  	struct rb_node *n;
>  	unsigned long addr;
>  	int purged = 0;
> @@ -416,11 +411,6 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  
>  	might_sleep();
>  
> -	va = kmalloc_node(sizeof(struct vmap_area),
> -			gfp_mask & GFP_RECLAIM_MASK, node);
> -	if (unlikely(!va))
> -		return ERR_PTR(-ENOMEM);
> -
>  	/*
>  	 * Only scan the relevant parts containing pointers to other objects
>  	 * to avoid false negatives.
> @@ -512,7 +502,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  	BUG_ON(va->va_start < vstart);
>  	BUG_ON(va->va_end > vend);
>  
> -	return va;
> +	return 0;
>  
>  overflow:
>  	spin_unlock(&vmap_area_lock);
> @@ -534,10 +524,38 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  	if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
>  		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
>  			size);
> -	kfree(va);
> -	return ERR_PTR(-EBUSY);
> +
> +	return -EBUSY;
> +}
> +
> +/*
> + * Allocate a region of KVA of the specified size and alignment, within the
> + * vstart and vend.
> + */
> +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)
> +{
> +	struct vmap_area *va;
> +	int ret;
> +
> +	va = kmalloc_node(sizeof(struct vmap_area),
> +			gfp_mask & GFP_RECLAIM_MASK, node);
> +	if (unlikely(!va))
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
> +	if (ret) {
> +		kfree(va);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return va;
>  }
>  
> +

Another spurious blank line?

With these two fixed,
Reviewed-by: Matthew Wilcox <willy@...radead.org>