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

[Tejun Heo]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/