>> For the particular bootmem node, the minimal and maximal PFN (
>> Page Frame Number) have been traced in the instance of "struct
>> bootmem_data_t". On current implementation, the maximal PFN isn't
>> checked while deallocating a bunch (BITS_PER_LONG) of page frames.
>> So the current implementation won't work if the maximal PFN isn't
>> aligned with BITS_PER_LONG.
>>
>> The patch will check the maximal PFN of the given bootmem node.
>> Also, we needn't check all the bits map when the starting PFN isn't
>> BITS_PER_LONG aligned. Actually, we should start from the offset
>> of the bits map, which indicated by the starting PFN.
>>
>> Signed-off-by: Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx>
>> ---
>> mm/bootmem.c | 11 ++++++++---
>> 1 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/bootmem.c b/mm/bootmem.c
>> index 5a04536..ebac3ba 100644
>> --- a/mm/bootmem.c
>> +++ b/mm/bootmem.c
>> @@ -194,16 +194,20 @@ static unsigned long __init free_all_bootmem_core(bootmem_data_t *bdata)
>> * BITS_PER_LONG block of pages in front of us, free
>> * it in one go.
>> */
>> - if (IS_ALIGNED(start, BITS_PER_LONG) && vec == ~0UL) {
>> + if (end - start >= BITS_PER_LONG &&
>> + IS_ALIGNED(start, BITS_PER_LONG) &&
>> + vec == ~0UL) {
>
>Did you have any actual problems with the code or was this just by
>review?
>
I just got this for review. There're no real problem against it ;-)
>vec has bits set for unreserved pages and the bitmap is aligned and
>reserved per default. So if the chunk is smaller than (end - start),
>then vec is already != ~0UL. The check you add should be redundant.
>
Yes. I'll remove the duplicate check in next revision.
>> int order = ilog2(BITS_PER_LONG);
>>
>> __free_pages_bootmem(pfn_to_page(start), order);
>> count += BITS_PER_LONG;
>> start += BITS_PER_LONG;
>> } else {
>> - unsigned long off = 0;
>> + unsigned long cursor = start;
>> + unsigned long off = cursor & (BITS_PER_LONG - 1);
>>
>> - while (vec && off < BITS_PER_LONG) {
>> + vec >>= off;
>> + while (vec && off < BITS_PER_LONG && cursor < end) {
>
>Optimization looks ok, although I doubt it makes a notable difference,
>this case should be pretty rare.
>
>Also, if you reach end, vec has no more bits set, so the cursor < end
>check should again be redundant. I think we can also remove the
>off < BITS_PER_LONG, there can hardly be more than BITS_PER_LONG
>set bits in vec.
>
Yes. I'll change that into "while (vec)" in next revision.
Thanks,
Gavin
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>