diff mbox

[v3,1/5] memblock: Introduce allocation direction to memblock.

Message ID 1379064655-20874-2-git-send-email-tangchen@cn.fujitsu.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

tangchen Sept. 13, 2013, 9:30 a.m. UTC
The Linux kernel cannot migrate pages used by the kernel. As a result, kernel
pages cannot be hot-removed. So we cannot allocate hotpluggable memory for
the kernel.

ACPI SRAT (System Resource Affinity Table) contains the memory hotplug info.
But before SRAT is parsed, memblock has already started to allocate memory
for the kernel. So we need to prevent memblock from doing this.

In a memory hotplug system, any numa node the kernel resides in should
be unhotpluggable. And for a modern server, each node could have at least
16GB memory. So memory around the kernel image is highly likely unhotpluggable.

So the basic idea is: Allocate memory from the end of the kernel image and
to the higher memory. Since memory allocation before SRAT is parsed won't
be too much, it could highly likely be in the same node with kernel image.

The current memblock can only allocate memory from high address to low.
So this patch introduces the allocation direct to memblock. It could be
used to tell memblock to allocate memory from high to low or from low
to high.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Reviewed-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 include/linux/memblock.h |   22 ++++++++++++++++++++++
 mm/memblock.c            |   13 +++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

Comments

Jianguo Wu Sept. 14, 2013, 2:42 a.m. UTC | #1
Hi Tang,

On 2013/9/13 17:30, Tang Chen wrote:

> The Linux kernel cannot migrate pages used by the kernel. As a result, kernel
> pages cannot be hot-removed. So we cannot allocate hotpluggable memory for
> the kernel.
> 
> ACPI SRAT (System Resource Affinity Table) contains the memory hotplug info.
> But before SRAT is parsed, memblock has already started to allocate memory
> for the kernel. So we need to prevent memblock from doing this.
> 
> In a memory hotplug system, any numa node the kernel resides in should
> be unhotpluggable. And for a modern server, each node could have at least
> 16GB memory. So memory around the kernel image is highly likely unhotpluggable.
> 
> So the basic idea is: Allocate memory from the end of the kernel image and
> to the higher memory. Since memory allocation before SRAT is parsed won't
> be too much, it could highly likely be in the same node with kernel image.
> 
> The current memblock can only allocate memory from high address to low.
> So this patch introduces the allocation direct to memblock. It could be
> used to tell memblock to allocate memory from high to low or from low
> to high.
> 
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> Reviewed-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> ---
>  include/linux/memblock.h |   22 ++++++++++++++++++++++
>  mm/memblock.c            |   13 +++++++++++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 31e95ac..a7d3436 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -19,6 +19,11 @@
>  
>  #define INIT_MEMBLOCK_REGIONS	128
>  
> +/* Allocation order. */

s/order/direction/

> +#define MEMBLOCK_DIRECTION_HIGH_TO_LOW	0
> +#define MEMBLOCK_DIRECTION_LOW_TO_HIGH	1
> +#define MEMBLOCK_DIRECTION_DEFAULT	MEMBLOCK_DIRECTION_HIGH_TO_LOW
> +
>  struct memblock_region {
>  	phys_addr_t base;
>  	phys_addr_t size;
> @@ -35,6 +40,7 @@ struct memblock_type {
>  };
>  
>  struct memblock {
> +	int current_direction;      /* allocate from higher or lower address */
>  	phys_addr_t current_limit;
>  	struct memblock_type memory;
>  	struct memblock_type reserved;
> @@ -148,6 +154,12 @@ phys_addr_t memblock_alloc_try_nid(phys_addr_t size, phys_addr_t align, int nid)
>  
>  phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t align);
>  
> +static inline bool memblock_direction_bottom_up(void)
> +{
> +	return memblock.current_direction == MEMBLOCK_DIRECTION_LOW_TO_HIGH;
> +}
> +
> +
>  /* Flags for memblock_alloc_base() amd __memblock_alloc_base() */
>  #define MEMBLOCK_ALLOC_ANYWHERE	(~(phys_addr_t)0)
>  #define MEMBLOCK_ALLOC_ACCESSIBLE	0
> @@ -175,6 +187,16 @@ static inline void memblock_dump_all(void)
>  }
>  
>  /**
> + * memblock_set_current_direction - Set current allocation direction to allow
> + *                                  allocating memory from higher to lower
> + *                                  address or from lower to higher address
> + *
> + * @direction: In which order to allocate memory. Could be

s/order/direction/

> + *             MEMBLOCK_DIRECTION_{HIGH_TO_LOW|LOW_TO_HIGH}
> + */
> +void memblock_set_current_direction(int direction);
> +
> +/**
>   * memblock_set_current_limit - Set the current allocation limit to allow
>   *                         limiting allocations to what is currently
>   *                         accessible during boot
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 0ac412a..f24ca2e 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -32,6 +32,7 @@ struct memblock memblock __initdata_memblock = {
>  	.reserved.cnt		= 1,	/* empty dummy entry */
>  	.reserved.max		= INIT_MEMBLOCK_REGIONS,
>  
> +	.current_direction	= MEMBLOCK_DIRECTION_DEFAULT,
>  	.current_limit		= MEMBLOCK_ALLOC_ANYWHERE,
>  };
>  
> @@ -995,6 +996,18 @@ void __init_memblock memblock_trim_memory(phys_addr_t align)
>  	}
>  }
>  
> +void __init_memblock memblock_set_current_direction(int direction)
> +{
> +	if (direction != MEMBLOCK_DIRECTION_HIGH_TO_LOW &&
> +	    direction != MEMBLOCK_DIRECTION_LOW_TO_HIGH) {
> +		pr_warn("memblock: Failed to set allocation order. "
> +			"Invalid order type: %d\n", direction);

s/order/direction/

> +		return;
> +	}
> +
> +	memblock.current_direction = direction;
> +}
> +
>  void __init_memblock memblock_set_current_limit(phys_addr_t limit)
>  {
>  	memblock.current_limit = limit;



--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Sept. 23, 2013, 3:38 p.m. UTC | #2
Hello,

Sorry about the delay.  Was traveling.

On Fri, Sep 13, 2013 at 05:30:51PM +0800, Tang Chen wrote:
> +/* Allocation order. */
> +#define MEMBLOCK_DIRECTION_HIGH_TO_LOW	0
> +#define MEMBLOCK_DIRECTION_LOW_TO_HIGH	1
> +#define MEMBLOCK_DIRECTION_DEFAULT	MEMBLOCK_DIRECTION_HIGH_TO_LOW

Can we please settle on either top_down/bottom_up or
high_to_low/low_to_high?  The two seem to be used interchangeably in
the patch series.  Also, it'd be more customary to use enum for things
like above, but more on the interface below.

> +static inline bool memblock_direction_bottom_up(void)
> +{
> +	return memblock.current_direction == MEMBLOCK_DIRECTION_LOW_TO_HIGH;
> +}

Maybe just memblock_bottom_up() would be enough?

Also, why not also have memblock_set_bottom_up(bool enable) as the
'set' interface?

>  /**
> + * memblock_set_current_direction - Set current allocation direction to allow
> + *                                  allocating memory from higher to lower
> + *                                  address or from lower to higher address
> + *
> + * @direction: In which order to allocate memory. Could be
> + *             MEMBLOCK_DIRECTION_{HIGH_TO_LOW|LOW_TO_HIGH}
> + */
> +void memblock_set_current_direction(int direction);

Function comments should go with the function definition.  Dunno what
happened with set_current_limit but let's please not spread it.

> +void __init_memblock memblock_set_current_direction(int direction)
> +{
> +	if (direction != MEMBLOCK_DIRECTION_HIGH_TO_LOW &&
> +	    direction != MEMBLOCK_DIRECTION_LOW_TO_HIGH) {
> +		pr_warn("memblock: Failed to set allocation order. "
> +			"Invalid order type: %d\n", direction);
> +		return;
> +	}
> +
> +	memblock.current_direction = direction;
> +}

If set_bottom_up() style interface is used, the above will be a lot
simpler, right?  Also, it's kinda weird to have two separate patches
to introduce the flag and actually implement bottom up allocation.

Thanks.
Zhang Yanfei Sept. 23, 2013, 4:36 p.m. UTC | #3
Hello tejun,

On 09/23/2013 11:38 PM, Tejun Heo wrote:
> Hello,
> 
> Sorry about the delay.  Was traveling.

hoho~ I guess you did have a good time.

> 
> On Fri, Sep 13, 2013 at 05:30:51PM +0800, Tang Chen wrote:
>> +/* Allocation order. */
>> +#define MEMBLOCK_DIRECTION_HIGH_TO_LOW	0
>> +#define MEMBLOCK_DIRECTION_LOW_TO_HIGH	1
>> +#define MEMBLOCK_DIRECTION_DEFAULT	MEMBLOCK_DIRECTION_HIGH_TO_LOW
> 
> Can we please settle on either top_down/bottom_up or
> high_to_low/low_to_high?  The two seem to be used interchangeably in
> the patch series.  Also, it'd be more customary to use enum for things
> like above, but more on the interface below.

OK. let's use top_down/bottom_up. And using enum is also ok.

> 
>> +static inline bool memblock_direction_bottom_up(void)
>> +{
>> +	return memblock.current_direction == MEMBLOCK_DIRECTION_LOW_TO_HIGH;
>> +}
> 
> Maybe just memblock_bottom_up() would be enough?

Agreed.

> 
> Also, why not also have memblock_set_bottom_up(bool enable) as the
> 'set' interface?

hmmm, ok. So we will use memblock_set_bottom_up to replace
memblock_set_current_direction below.

> 
>>  /**
>> + * memblock_set_current_direction - Set current allocation direction to allow
>> + *                                  allocating memory from higher to lower
>> + *                                  address or from lower to higher address
>> + *
>> + * @direction: In which order to allocate memory. Could be
>> + *             MEMBLOCK_DIRECTION_{HIGH_TO_LOW|LOW_TO_HIGH}
>> + */
>> +void memblock_set_current_direction(int direction);
> 
> Function comments should go with the function definition.  Dunno what
> happened with set_current_limit but let's please not spread it.
> 
>> +void __init_memblock memblock_set_current_direction(int direction)
>> +{
>> +	if (direction != MEMBLOCK_DIRECTION_HIGH_TO_LOW &&
>> +	    direction != MEMBLOCK_DIRECTION_LOW_TO_HIGH) {
>> +		pr_warn("memblock: Failed to set allocation order. "
>> +			"Invalid order type: %d\n", direction);
>> +		return;
>> +	}
>> +
>> +	memblock.current_direction = direction;
>> +}
> 
> If set_bottom_up() style interface is used, the above will be a lot
> simpler, right?  Also, it's kinda weird to have two separate patches
> to introduce the flag and actually implement bottom up allocation.

Yeah, right, that'd be much simpler. And it is ok to put the two in
one patch.

Thanks.

> 
> Thanks.
>
diff mbox

Patch

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 31e95ac..a7d3436 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -19,6 +19,11 @@ 
 
 #define INIT_MEMBLOCK_REGIONS	128
 
+/* Allocation order. */
+#define MEMBLOCK_DIRECTION_HIGH_TO_LOW	0
+#define MEMBLOCK_DIRECTION_LOW_TO_HIGH	1
+#define MEMBLOCK_DIRECTION_DEFAULT	MEMBLOCK_DIRECTION_HIGH_TO_LOW
+
 struct memblock_region {
 	phys_addr_t base;
 	phys_addr_t size;
@@ -35,6 +40,7 @@  struct memblock_type {
 };
 
 struct memblock {
+	int current_direction;      /* allocate from higher or lower address */
 	phys_addr_t current_limit;
 	struct memblock_type memory;
 	struct memblock_type reserved;
@@ -148,6 +154,12 @@  phys_addr_t memblock_alloc_try_nid(phys_addr_t size, phys_addr_t align, int nid)
 
 phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t align);
 
+static inline bool memblock_direction_bottom_up(void)
+{
+	return memblock.current_direction == MEMBLOCK_DIRECTION_LOW_TO_HIGH;
+}
+
+
 /* Flags for memblock_alloc_base() amd __memblock_alloc_base() */
 #define MEMBLOCK_ALLOC_ANYWHERE	(~(phys_addr_t)0)
 #define MEMBLOCK_ALLOC_ACCESSIBLE	0
@@ -175,6 +187,16 @@  static inline void memblock_dump_all(void)
 }
 
 /**
+ * memblock_set_current_direction - Set current allocation direction to allow
+ *                                  allocating memory from higher to lower
+ *                                  address or from lower to higher address
+ *
+ * @direction: In which order to allocate memory. Could be
+ *             MEMBLOCK_DIRECTION_{HIGH_TO_LOW|LOW_TO_HIGH}
+ */
+void memblock_set_current_direction(int direction);
+
+/**
  * memblock_set_current_limit - Set the current allocation limit to allow
  *                         limiting allocations to what is currently
  *                         accessible during boot
diff --git a/mm/memblock.c b/mm/memblock.c
index 0ac412a..f24ca2e 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -32,6 +32,7 @@  struct memblock memblock __initdata_memblock = {
 	.reserved.cnt		= 1,	/* empty dummy entry */
 	.reserved.max		= INIT_MEMBLOCK_REGIONS,
 
+	.current_direction	= MEMBLOCK_DIRECTION_DEFAULT,
 	.current_limit		= MEMBLOCK_ALLOC_ANYWHERE,
 };
 
@@ -995,6 +996,18 @@  void __init_memblock memblock_trim_memory(phys_addr_t align)
 	}
 }
 
+void __init_memblock memblock_set_current_direction(int direction)
+{
+	if (direction != MEMBLOCK_DIRECTION_HIGH_TO_LOW &&
+	    direction != MEMBLOCK_DIRECTION_LOW_TO_HIGH) {
+		pr_warn("memblock: Failed to set allocation order. "
+			"Invalid order type: %d\n", direction);
+		return;
+	}
+
+	memblock.current_direction = direction;
+}
+
 void __init_memblock memblock_set_current_limit(phys_addr_t limit)
 {
 	memblock.current_limit = limit;