linux-kernel - Re: [PATCH 2/6] memblock: Introduce bottom-up allocation mode

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: <20130924121725.GC2366@htj.dyndns.org>
Date:	Tue, 24 Sep 2013 08:17:25 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Zhang Yanfei <zhangyanfei@...fujitsu.com>
Cc:	"Rafael J . Wysocki" <rjw@...k.pl>, lenb@...nel.org,
	Thomas Gleixner <tglx@...utronix.de>, mingo@...e.hu,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Toshi Kani <toshi.kani@...com>,
	Wanpeng Li <liwanp@...ux.vnet.ibm.com>,
	Thomas Renninger <trenn@...e.de>,
	Yinghai Lu <yinghai@...nel.org>,
	Jiang Liu <jiang.liu@...wei.com>,
	Wen Congyang <wency@...fujitsu.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	isimatu.yasuaki@...fujitsu.com, izumi.taku@...fujitsu.com,
	Mel Gorman <mgorman@...e.de>, Minchan Kim <minchan@...nel.org>,
	mina86@...a86.com, gong.chen@...ux.intel.com,
	vasilis.liaskovitis@...fitbricks.com, lwoodman@...hat.com,
	Rik van Riel <riel@...hat.com>, jweiner@...hat.com,
	prarit@...hat.com, "x86@...nel.org" <x86@...nel.org>,
	linux-doc@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linux MM <linux-mm@...ck.org>, linux-acpi@...r.kernel.org,
	imtangchen@...il.com, Zhang Yanfei <zhangyanfei.yes@...il.com>
Subject: Re: [PATCH 2/6] memblock: Introduce bottom-up allocation mode

On Tue, Sep 24, 2013 at 06:05:03PM +0800, Zhang Yanfei wrote:
> +/* Allocation direction */
> +enum {
> +	MEMBLOCK_DIRECTION_TOP_DOWN,
> +	MEMBLOCK_DIRECTION_BOTTOM_UP,
> +	NR_MEMLBOCK_DIRECTIONS
> +};
> +
>  struct memblock_region {
>  	phys_addr_t base;
>  	phys_addr_t size;
> @@ -35,6 +42,7 @@ struct memblock_type {
>  };
>  
>  struct memblock {
> +	int current_direction;  /* current allocation direction */

Just use boolean bottom_up here too?  No need for the constants.

> @@ -20,6 +20,8 @@
>  #include <linux/seq_file.h>
>  #include <linux/memblock.h>
>  
> +#include <asm-generic/sections.h>
> +

Why is the above added by this patch?

>  /**
> + * __memblock_find_range - find free area utility
> + * @start: start of candidate range
> + * @end: end of candidate range, can be %MEMBLOCK_ALLOC_{ANYWHERE|ACCESSIBLE}
> + * @size: size of free area to find
> + * @align: alignment of free area to find
> + * @nid: nid of the free area to find, %MAX_NUMNODES for any node
> + *
> + * Utility called from memblock_find_in_range_node(), find free area bottom-up.
> + *
> + * RETURNS:
> + * Found address on success, %0 on failure.

I don't think we prefix numeric literals with %.

...
> @@ -127,6 +162,10 @@ __memblock_find_range_rev(phys_addr_t start, phys_addr_t end,
>   *
>   * Find @size free area aligned to @align in the specified range and node.
>   *
> + * When allocation direction is bottom-up, the @start should be greater
> + * than the end of the kernel image. Otherwise, it will be trimmed. And also,
> + * if bottom-up allocation failed, will try to allocate memory top-down.

It'd be nice to explain that bottom-up allocation is limited to above
kernel image and what it's used for here.

> + *
>   * RETURNS:
>   * Found address on success, %0 on failure.
>   */
> @@ -134,6 +173,8 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
>  					phys_addr_t end, phys_addr_t size,
>  					phys_addr_t align, int nid)
>  {
> +	int ret;
> +
>  	/* pump up @end */
>  	if (end == MEMBLOCK_ALLOC_ACCESSIBLE)
>  		end = memblock.current_limit;
> @@ -142,6 +183,28 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
>  	start = max_t(phys_addr_t, start, PAGE_SIZE);
>  	end = max(start, end);
>  
> +	if (memblock_bottom_up()) {
> +		phys_addr_t bottom_up_start;
> +
> +		/* make sure we will allocate above the kernel */
> +		bottom_up_start = max_t(phys_addr_t, start, __pa_symbol(_end));
> +
> +		/* ok, try bottom-up allocation first */
> +		ret = __memblock_find_range(bottom_up_start, end,
> +					    size, align, nid);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * we always limit bottom-up allocation above the kernel,
> +		 * but top-down allocation doesn't have the limit, so
> +		 * retrying top-down allocation may succeed when bottom-up
> +		 * allocation failed.
> +		 */
> +		pr_warn("memblock: Failed to allocate memory in bottom up "
> +			"direction. Now try top down direction.\n");

Maybe just print warning only on the first failure?

Otherwise, looks good to me.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/