2013-08-15 03:24:46

by Tang Chen

[permalink] [raw]
Subject: [PATCH 1/1] memblock cleanup: Remove unnecessary check in memblock_find_in_range_node()

In memblock_find_in_range_node(), it has the following check at line 117 and 118:

113 for_each_free_mem_range_reverse(i, nid, &this_start, &this_end, NULL) {
114 this_start = clamp(this_start, start, end);
115 this_end = clamp(this_end, start, end);
116
117 if (this_end < size)
118 continue;
119
120 cand = round_down(this_end - size, align);
121 if (cand >= this_start)
122 return cand;
123 }

Since it finds memory from higher memory downwards, if this_end < size,
we can break because the rest memory will all under size. It won't satisfy
us ang more.

Furthermore, we don't need to check "if (this_end < size)" actually. Without
this confusing check, we only waste some loops. So this patch removes the
check.

Signed-off-by: Tang Chen <[email protected]>
---
mm/memblock.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index a847bfe..e0c626e 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -114,9 +114,6 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
this_start = clamp(this_start, start, end);
this_end = clamp(this_end, start, end);

- if (this_end < size)
- continue;
-
cand = round_down(this_end - size, align);
if (cand >= this_start)
return cand;
--
1.7.1


2013-08-15 03:27:52

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 1/1] memblock cleanup: Remove unnecessary check in memblock_find_in_range_node()

Hello, Tang.

On Thu, Aug 15, 2013 at 11:23:19AM +0800, Tang Chen wrote:
> Furthermore, we don't need to check "if (this_end < size)" actually. Without
> this confusing check, we only waste some loops. So this patch removes the
> check.
>
> Signed-off-by: Tang Chen <[email protected]>
> ---
> mm/memblock.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memblock.c b/mm/memblock.c
> index a847bfe..e0c626e 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -114,9 +114,6 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
> this_start = clamp(this_start, start, end);
> this_end = clamp(this_end, start, end);
>
> - if (this_end < size)
> - continue;
> -
> cand = round_down(this_end - size, align);
> if (cand >= this_start)
> return cand;

Hmmm... maybe I'm missing something but are you sure? "this_end -
size" can underflow and "cand >= this_start" will be true incorrectly.

Thanks.

--
tejun

2013-08-15 03:39:37

by Tang Chen

[permalink] [raw]
Subject: Re: [PATCH 1/1] memblock cleanup: Remove unnecessary check in memblock_find_in_range_node()

On 08/15/2013 11:27 AM, Tejun Heo wrote:
> Hello, Tang.
>
> On Thu, Aug 15, 2013 at 11:23:19AM +0800, Tang Chen wrote:
>> Furthermore, we don't need to check "if (this_end< size)" actually. Without
>> this confusing check, we only waste some loops. So this patch removes the
>> check.
>>
>> Signed-off-by: Tang Chen<[email protected]>
>> ---
>> mm/memblock.c | 3 ---
>> 1 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index a847bfe..e0c626e 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -114,9 +114,6 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
>> this_start = clamp(this_start, start, end);
>> this_end = clamp(this_end, start, end);
>>
>> - if (this_end< size)
>> - continue;
>> -
>> cand = round_down(this_end - size, align);
>> if (cand>= this_start)
>> return cand;
>
> Hmmm... maybe I'm missing something but are you sure? "this_end -
> size" can underflow and "cand>= this_start" will be true incorrectly.
>

Oh, you are right... Please ignore this. I didn't read it carefully.

Thanks.