All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/2] mm/memblock.c: fix potential bug and code refine
@ 2016-12-21 23:30 ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-21 23:30 UTC (permalink / raw)
  To: trivial, akpm, mhocko; +Cc: linux-mm, linux-kernel, Wei Yang

Here are two patch of mm/memblock.c.
[1]. A trivial code refine in memblock_is_region_memory(), which removes an
unnecessary check on base address.
[2]. The original code forgets to check the return value of
memblock_reserve(), which may lead to potential problem. The patch fix this.

---
v3: 
   * remove the check for base instead of comment out
   * Reform the changelog
v2: 
   * remove a trivial code refine, which is already fixed in upstream 

Wei Yang (2):
  mm/memblock.c: trivial code refine in memblock_is_region_memory()
  mm/memblock.c: check return value of memblock_reserve() in
    memblock_virt_alloc_internal()

 include/linux/memblock.h |    5 ++---
 mm/memblock.c            |    8 +++-----
 2 files changed, 5 insertions(+), 8 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3 0/2] mm/memblock.c: fix potential bug and code refine
@ 2016-12-21 23:30 ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-21 23:30 UTC (permalink / raw)
  To: trivial, akpm, mhocko; +Cc: linux-mm, linux-kernel, Wei Yang

Here are two patch of mm/memblock.c.
[1]. A trivial code refine in memblock_is_region_memory(), which removes an
unnecessary check on base address.
[2]. The original code forgets to check the return value of
memblock_reserve(), which may lead to potential problem. The patch fix this.

---
v3: 
   * remove the check for base instead of comment out
   * Reform the changelog
v2: 
   * remove a trivial code refine, which is already fixed in upstream 

Wei Yang (2):
  mm/memblock.c: trivial code refine in memblock_is_region_memory()
  mm/memblock.c: check return value of memblock_reserve() in
    memblock_virt_alloc_internal()

 include/linux/memblock.h |    5 ++---
 mm/memblock.c            |    8 +++-----
 2 files changed, 5 insertions(+), 8 deletions(-)

-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3 1/2] mm/memblock.c: trivial code refine in memblock_is_region_memory()
  2016-12-21 23:30 ` Wei Yang
@ 2016-12-21 23:30   ` Wei Yang
  -1 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-21 23:30 UTC (permalink / raw)
  To: trivial, akpm, mhocko; +Cc: linux-mm, linux-kernel, Wei Yang

memblock_is_region_memory() invoke memblock_search() to see whether the
base address is in the memory region. If it fails, idx would be -1. Then,
it returns 0.

If the memblock_search() returns a valid index, it means the base address
is guaranteed to be in the range memblock.memory.regions[idx]. Because of
this, it is not necessary to check the base again.

This patch removes the check on "base".

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/memblock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 7608bc3..4929e06 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1615,8 +1615,7 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
 
 	if (idx == -1)
 		return 0;
-	return memblock.memory.regions[idx].base <= base &&
-		(memblock.memory.regions[idx].base +
+	return (memblock.memory.regions[idx].base +
 		 memblock.memory.regions[idx].size) >= end;
 }
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 1/2] mm/memblock.c: trivial code refine in memblock_is_region_memory()
@ 2016-12-21 23:30   ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-21 23:30 UTC (permalink / raw)
  To: trivial, akpm, mhocko; +Cc: linux-mm, linux-kernel, Wei Yang

memblock_is_region_memory() invoke memblock_search() to see whether the
base address is in the memory region. If it fails, idx would be -1. Then,
it returns 0.

If the memblock_search() returns a valid index, it means the base address
is guaranteed to be in the range memblock.memory.regions[idx]. Because of
this, it is not necessary to check the base again.

This patch removes the check on "base".

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/memblock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 7608bc3..4929e06 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1615,8 +1615,7 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
 
 	if (idx == -1)
 		return 0;
-	return memblock.memory.regions[idx].base <= base &&
-		(memblock.memory.regions[idx].base +
+	return (memblock.memory.regions[idx].base +
 		 memblock.memory.regions[idx].size) >= end;
 }
 
-- 
2.5.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
  2016-12-21 23:30 ` Wei Yang
@ 2016-12-21 23:30   ` Wei Yang
  -1 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-21 23:30 UTC (permalink / raw)
  To: trivial, akpm, mhocko; +Cc: linux-mm, linux-kernel, Wei Yang

memblock_reserve() would add a new range to memblock.reserved in case the
new range is not totally covered by any of the current memblock.reserved
range. If the memblock.reserved is full and can't resize,
memblock_reserve() would fail.

This doesn't happen in real world now, I observed this during code review.
While theoretically, it has the chance to happen. And if it happens, others
would think this range of memory is still available and may corrupt the
memory.

This patch checks the return value and goto "done" after it succeeds.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/memblock.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 4929e06..d0f2c96 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1274,18 +1274,17 @@ static void * __init memblock_virt_alloc_internal(
 
 	if (max_addr > memblock.current_limit)
 		max_addr = memblock.current_limit;
-
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid, flags);
-	if (alloc)
+	if (alloc && !memblock_reserve(alloc, size))
 		goto done;
 
 	if (nid != NUMA_NO_NODE) {
 		alloc = memblock_find_in_range_node(size, align, min_addr,
 						    max_addr, NUMA_NO_NODE,
 						    flags);
-		if (alloc)
+		if (alloc && !memblock_reserve(alloc, size))
 			goto done;
 	}
 
@@ -1303,7 +1302,6 @@ static void * __init memblock_virt_alloc_internal(
 
 	return NULL;
 done:
-	memblock_reserve(alloc, size);
 	ptr = phys_to_virt(alloc);
 	memset(ptr, 0, size);
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
@ 2016-12-21 23:30   ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-21 23:30 UTC (permalink / raw)
  To: trivial, akpm, mhocko; +Cc: linux-mm, linux-kernel, Wei Yang

memblock_reserve() would add a new range to memblock.reserved in case the
new range is not totally covered by any of the current memblock.reserved
range. If the memblock.reserved is full and can't resize,
memblock_reserve() would fail.

This doesn't happen in real world now, I observed this during code review.
While theoretically, it has the chance to happen. And if it happens, others
would think this range of memory is still available and may corrupt the
memory.

This patch checks the return value and goto "done" after it succeeds.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/memblock.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 4929e06..d0f2c96 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1274,18 +1274,17 @@ static void * __init memblock_virt_alloc_internal(
 
 	if (max_addr > memblock.current_limit)
 		max_addr = memblock.current_limit;
-
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid, flags);
-	if (alloc)
+	if (alloc && !memblock_reserve(alloc, size))
 		goto done;
 
 	if (nid != NUMA_NO_NODE) {
 		alloc = memblock_find_in_range_node(size, align, min_addr,
 						    max_addr, NUMA_NO_NODE,
 						    flags);
-		if (alloc)
+		if (alloc && !memblock_reserve(alloc, size))
 			goto done;
 	}
 
@@ -1303,7 +1302,6 @@ static void * __init memblock_virt_alloc_internal(
 
 	return NULL;
 done:
-	memblock_reserve(alloc, size);
 	ptr = phys_to_virt(alloc);
 	memset(ptr, 0, size);
 
-- 
2.5.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/2] mm/memblock.c: trivial code refine in memblock_is_region_memory()
  2016-12-21 23:30   ` Wei Yang
@ 2016-12-22  9:06     ` Michal Hocko
  -1 siblings, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2016-12-22  9:06 UTC (permalink / raw)
  To: Wei Yang; +Cc: trivial, akpm, linux-mm, linux-kernel

On Wed 21-12-16 23:30:32, Wei Yang wrote:
> memblock_is_region_memory() invoke memblock_search() to see whether the
> base address is in the memory region. If it fails, idx would be -1. Then,
> it returns 0.
> 
> If the memblock_search() returns a valid index, it means the base address
> is guaranteed to be in the range memblock.memory.regions[idx]. Because of
> this, it is not necessary to check the base again.
> 
> This patch removes the check on "base".

OK, the patch looks correct. I doubt it makes any real difference but I
do not see it being harmful.

> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/memblock.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..4929e06 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1615,8 +1615,7 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
>  
>  	if (idx == -1)
>  		return 0;
> -	return memblock.memory.regions[idx].base <= base &&
> -		(memblock.memory.regions[idx].base +
> +	return (memblock.memory.regions[idx].base +
>  		 memblock.memory.regions[idx].size) >= end;
>  }
>  
> -- 
> 2.5.0

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/2] mm/memblock.c: trivial code refine in memblock_is_region_memory()
@ 2016-12-22  9:06     ` Michal Hocko
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2016-12-22  9:06 UTC (permalink / raw)
  To: Wei Yang; +Cc: trivial, akpm, linux-mm, linux-kernel

On Wed 21-12-16 23:30:32, Wei Yang wrote:
> memblock_is_region_memory() invoke memblock_search() to see whether the
> base address is in the memory region. If it fails, idx would be -1. Then,
> it returns 0.
> 
> If the memblock_search() returns a valid index, it means the base address
> is guaranteed to be in the range memblock.memory.regions[idx]. Because of
> this, it is not necessary to check the base again.
> 
> This patch removes the check on "base".

OK, the patch looks correct. I doubt it makes any real difference but I
do not see it being harmful.

> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/memblock.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..4929e06 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1615,8 +1615,7 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
>  
>  	if (idx == -1)
>  		return 0;
> -	return memblock.memory.regions[idx].base <= base &&
> -		(memblock.memory.regions[idx].base +
> +	return (memblock.memory.regions[idx].base +
>  		 memblock.memory.regions[idx].size) >= end;
>  }
>  
> -- 
> 2.5.0

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
  2016-12-21 23:30   ` Wei Yang
@ 2016-12-22  9:15     ` Michal Hocko
  -1 siblings, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2016-12-22  9:15 UTC (permalink / raw)
  To: Wei Yang; +Cc: trivial, akpm, linux-mm, linux-kernel

On Wed 21-12-16 23:30:33, Wei Yang wrote:
> memblock_reserve() would add a new range to memblock.reserved in case the
> new range is not totally covered by any of the current memblock.reserved
> range. If the memblock.reserved is full and can't resize,
> memblock_reserve() would fail.
> 
> This doesn't happen in real world now, I observed this during code review.
> While theoretically, it has the chance to happen. And if it happens, others
> would think this range of memory is still available and may corrupt the
> memory.

OK, this explains it much better than the previous version! The silent
memory corruption is indeed too hard to debug to have this open even
when the issue is theoretical.

> This patch checks the return value and goto "done" after it succeeds.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
>  mm/memblock.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 4929e06..d0f2c96 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1274,18 +1274,17 @@ static void * __init memblock_virt_alloc_internal(
>  
>  	if (max_addr > memblock.current_limit)
>  		max_addr = memblock.current_limit;
> -
>  again:
>  	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
>  					    nid, flags);
> -	if (alloc)
> +	if (alloc && !memblock_reserve(alloc, size))
>  		goto done;
>  
>  	if (nid != NUMA_NO_NODE) {
>  		alloc = memblock_find_in_range_node(size, align, min_addr,
>  						    max_addr, NUMA_NO_NODE,
>  						    flags);
> -		if (alloc)
> +		if (alloc && !memblock_reserve(alloc, size))
>  			goto done;
>  	}
>  
> @@ -1303,7 +1302,6 @@ static void * __init memblock_virt_alloc_internal(
>  
>  	return NULL;
>  done:
> -	memblock_reserve(alloc, size);
>  	ptr = phys_to_virt(alloc);
>  	memset(ptr, 0, size);
>  
> -- 
> 2.5.0

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
@ 2016-12-22  9:15     ` Michal Hocko
  0 siblings, 0 replies; 14+ messages in thread
From: Michal Hocko @ 2016-12-22  9:15 UTC (permalink / raw)
  To: Wei Yang; +Cc: trivial, akpm, linux-mm, linux-kernel

On Wed 21-12-16 23:30:33, Wei Yang wrote:
> memblock_reserve() would add a new range to memblock.reserved in case the
> new range is not totally covered by any of the current memblock.reserved
> range. If the memblock.reserved is full and can't resize,
> memblock_reserve() would fail.
> 
> This doesn't happen in real world now, I observed this during code review.
> While theoretically, it has the chance to happen. And if it happens, others
> would think this range of memory is still available and may corrupt the
> memory.

OK, this explains it much better than the previous version! The silent
memory corruption is indeed too hard to debug to have this open even
when the issue is theoretical.

> This patch checks the return value and goto "done" after it succeeds.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
>  mm/memblock.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 4929e06..d0f2c96 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1274,18 +1274,17 @@ static void * __init memblock_virt_alloc_internal(
>  
>  	if (max_addr > memblock.current_limit)
>  		max_addr = memblock.current_limit;
> -
>  again:
>  	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
>  					    nid, flags);
> -	if (alloc)
> +	if (alloc && !memblock_reserve(alloc, size))
>  		goto done;
>  
>  	if (nid != NUMA_NO_NODE) {
>  		alloc = memblock_find_in_range_node(size, align, min_addr,
>  						    max_addr, NUMA_NO_NODE,
>  						    flags);
> -		if (alloc)
> +		if (alloc && !memblock_reserve(alloc, size))
>  			goto done;
>  	}
>  
> @@ -1303,7 +1302,6 @@ static void * __init memblock_virt_alloc_internal(
>  
>  	return NULL;
>  done:
> -	memblock_reserve(alloc, size);
>  	ptr = phys_to_virt(alloc);
>  	memset(ptr, 0, size);
>  
> -- 
> 2.5.0

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
  2016-12-22  9:15     ` Michal Hocko
@ 2016-12-22 22:37       ` Wei Yang
  -1 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-22 22:37 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Wei Yang, trivial, akpm, linux-mm, linux-kernel

On Thu, Dec 22, 2016 at 10:15:20AM +0100, Michal Hocko wrote:
>On Wed 21-12-16 23:30:33, Wei Yang wrote:
>> memblock_reserve() would add a new range to memblock.reserved in case the
>> new range is not totally covered by any of the current memblock.reserved
>> range. If the memblock.reserved is full and can't resize,
>> memblock_reserve() would fail.
>> 
>> This doesn't happen in real world now, I observed this during code review.
>> While theoretically, it has the chance to happen. And if it happens, others
>> would think this range of memory is still available and may corrupt the
>> memory.
>
>OK, this explains it much better than the previous version! The silent
>memory corruption is indeed too hard to debug to have this open even
>when the issue is theoretical.
>

Thanks~ Have a nice day:-)

>> This patch checks the return value and goto "done" after it succeeds.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>Acked-by: Michal Hocko <mhocko@suse.com>
>
>Thanks!
>
>> ---
>>  mm/memblock.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 4929e06..d0f2c96 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1274,18 +1274,17 @@ static void * __init memblock_virt_alloc_internal(
>>  
>>  	if (max_addr > memblock.current_limit)
>>  		max_addr = memblock.current_limit;
>> -
>>  again:
>>  	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
>>  					    nid, flags);
>> -	if (alloc)
>> +	if (alloc && !memblock_reserve(alloc, size))
>>  		goto done;
>>  
>>  	if (nid != NUMA_NO_NODE) {
>>  		alloc = memblock_find_in_range_node(size, align, min_addr,
>>  						    max_addr, NUMA_NO_NODE,
>>  						    flags);
>> -		if (alloc)
>> +		if (alloc && !memblock_reserve(alloc, size))
>>  			goto done;
>>  	}
>>  
>> @@ -1303,7 +1302,6 @@ static void * __init memblock_virt_alloc_internal(
>>  
>>  	return NULL;
>>  done:
>> -	memblock_reserve(alloc, size);
>>  	ptr = phys_to_virt(alloc);
>>  	memset(ptr, 0, size);
>>  
>> -- 
>> 2.5.0
>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
@ 2016-12-22 22:37       ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-22 22:37 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Wei Yang, trivial, akpm, linux-mm, linux-kernel

On Thu, Dec 22, 2016 at 10:15:20AM +0100, Michal Hocko wrote:
>On Wed 21-12-16 23:30:33, Wei Yang wrote:
>> memblock_reserve() would add a new range to memblock.reserved in case the
>> new range is not totally covered by any of the current memblock.reserved
>> range. If the memblock.reserved is full and can't resize,
>> memblock_reserve() would fail.
>> 
>> This doesn't happen in real world now, I observed this during code review.
>> While theoretically, it has the chance to happen. And if it happens, others
>> would think this range of memory is still available and may corrupt the
>> memory.
>
>OK, this explains it much better than the previous version! The silent
>memory corruption is indeed too hard to debug to have this open even
>when the issue is theoretical.
>

Thanks~ Have a nice day:-)

>> This patch checks the return value and goto "done" after it succeeds.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>Acked-by: Michal Hocko <mhocko@suse.com>
>
>Thanks!
>
>> ---
>>  mm/memblock.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 4929e06..d0f2c96 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1274,18 +1274,17 @@ static void * __init memblock_virt_alloc_internal(
>>  
>>  	if (max_addr > memblock.current_limit)
>>  		max_addr = memblock.current_limit;
>> -
>>  again:
>>  	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
>>  					    nid, flags);
>> -	if (alloc)
>> +	if (alloc && !memblock_reserve(alloc, size))
>>  		goto done;
>>  
>>  	if (nid != NUMA_NO_NODE) {
>>  		alloc = memblock_find_in_range_node(size, align, min_addr,
>>  						    max_addr, NUMA_NO_NODE,
>>  						    flags);
>> -		if (alloc)
>> +		if (alloc && !memblock_reserve(alloc, size))
>>  			goto done;
>>  	}
>>  
>> @@ -1303,7 +1302,6 @@ static void * __init memblock_virt_alloc_internal(
>>  
>>  	return NULL;
>>  done:
>> -	memblock_reserve(alloc, size);
>>  	ptr = phys_to_virt(alloc);
>>  	memset(ptr, 0, size);
>>  
>> -- 
>> 2.5.0
>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
  2016-12-11 12:59 [PATCH 0/2] mm/memblock.c: fix potential bug and code refine Wei Yang
@ 2016-12-11 12:59   ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-11 12:59 UTC (permalink / raw)
  To: trivial, akpm; +Cc: linux-mm, linux-kernel, Wei Yang

memblock_reserve() may fail in case there is not enough regions.

This patch checks the return value.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/memblock.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 9d402d05..83ad703 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1268,18 +1268,17 @@ static void * __init memblock_virt_alloc_internal(
 
 	if (max_addr > memblock.current_limit)
 		max_addr = memblock.current_limit;
-
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid, flags);
-	if (alloc)
+	if (alloc && !memblock_reserve(alloc, size))
 		goto done;
 
 	if (nid != NUMA_NO_NODE) {
 		alloc = memblock_find_in_range_node(size, align, min_addr,
 						    max_addr, NUMA_NO_NODE,
 						    flags);
-		if (alloc)
+		if (alloc && !memblock_reserve(alloc, size))
 			goto done;
 	}
 
@@ -1297,7 +1296,6 @@ again:
 
 	return NULL;
 done:
-	memblock_reserve(alloc, size);
 	ptr = phys_to_virt(alloc);
 	memset(ptr, 0, size);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal()
@ 2016-12-11 12:59   ` Wei Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Yang @ 2016-12-11 12:59 UTC (permalink / raw)
  To: trivial, akpm; +Cc: linux-mm, linux-kernel, Wei Yang

memblock_reserve() may fail in case there is not enough regions.

This patch checks the return value.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/memblock.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 9d402d05..83ad703 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1268,18 +1268,17 @@ static void * __init memblock_virt_alloc_internal(
 
 	if (max_addr > memblock.current_limit)
 		max_addr = memblock.current_limit;
-
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid, flags);
-	if (alloc)
+	if (alloc && !memblock_reserve(alloc, size))
 		goto done;
 
 	if (nid != NUMA_NO_NODE) {
 		alloc = memblock_find_in_range_node(size, align, min_addr,
 						    max_addr, NUMA_NO_NODE,
 						    flags);
-		if (alloc)
+		if (alloc && !memblock_reserve(alloc, size))
 			goto done;
 	}
 
@@ -1297,7 +1296,6 @@ again:
 
 	return NULL;
 done:
-	memblock_reserve(alloc, size);
 	ptr = phys_to_virt(alloc);
 	memset(ptr, 0, size);
 
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-12-22 22:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-21 23:30 [PATCH V3 0/2] mm/memblock.c: fix potential bug and code refine Wei Yang
2016-12-21 23:30 ` Wei Yang
2016-12-21 23:30 ` [PATCH V3 1/2] mm/memblock.c: trivial code refine in memblock_is_region_memory() Wei Yang
2016-12-21 23:30   ` Wei Yang
2016-12-22  9:06   ` Michal Hocko
2016-12-22  9:06     ` Michal Hocko
2016-12-21 23:30 ` [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal() Wei Yang
2016-12-21 23:30   ` Wei Yang
2016-12-22  9:15   ` Michal Hocko
2016-12-22  9:15     ` Michal Hocko
2016-12-22 22:37     ` Wei Yang
2016-12-22 22:37       ` Wei Yang
  -- strict thread matches above, loose matches on Subject: below --
2016-12-11 12:59 [PATCH 0/2] mm/memblock.c: fix potential bug and code refine Wei Yang
2016-12-11 12:59 ` [PATCH 2/2] mm/memblock.c: check return value of memblock_reserve() in memblock_virt_alloc_internal() Wei Yang
2016-12-11 12:59   ` Wei Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.