Hi Geert,
Thanks for review!
On 03/10/2013 05:20 PM, Geert Uytterhoeven wrote:
> On Sun, Mar 10, 2013 at 7:26 AM, Jiang Liu <liuj97@xxxxxxxxx> wrote:
>> Code to deal with reserved/managed pages are duplicated by many
>> architectures, so introduce common help functions to reduce duplicated
>> code. These common help functions will also be used to concentrate code
>> to modify totalram_pages and zone->managed_pages, which makes the code
>> much more clear.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxx>
>
> I have a few minor comments (see below), but apart from that:
> Acked-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
>
>> ---
>> include/linux/mm.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>> mm/page_alloc.c | 20 ++++++++++++++++++++
>> 2 files changed, 68 insertions(+)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 7acc9dc..d75c14b 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -1295,6 +1295,54 @@ extern void free_area_init_node(int nid, unsigned long * zones_size,
>> unsigned long zone_start_pfn, unsigned long *zholes_size);
>> extern void free_initmem(void);
>>
>> +/*
>> + * Free reserved pages within range [PAGE_ALIGN(start), end & PAGE_MASK)
>> + * into the buddy system. The freed pages will be poisoned with pattern
>> + * "poison" if it's non-zero.
>
> What if you want to poison with zero?
> As poison is a full int, but memset only uses the least-significant
> byte, you can
> change it to poison if it's positive (i.e. >= 0)?
Good point, ARM64 does poison memory with 0. Will implement that way in next version.
>
>> +/*
>> + * Default method to free all the __init memory into the buddy system.
>> + * The freed pages will be poisoned with pattern "poison" if it is
>> + * non-zero. Return pages freed into the buddy system.
>> + */
>> +static inline unsigned long free_initmem_default(int poison)
>> +{
>> + extern char __init_begin[], __init_end[];
>> +
>> + return free_reserved_area(PAGE_ALIGN((unsigned long)&__init_begin) ,
>> + ((unsigned long)&__init_end) & PAGE_MASK,
>
> The "PAGE_ALIGN(...)" and "& PAGE_MASK" are superfluous, as
> free_reserved_area() already does that.
Will remove the redundant ops next version.
>
>> + poison, "unused kernel");
>> +}
>> +
>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 8fcced7..0fadb09 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5113,6 +5113,26 @@ early_param("movablecore", cmdline_parse_movablecore);
>>
>> #endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>>
>> +unsigned long free_reserved_area(unsigned long start, unsigned long end,
>> + int poison, char *s)
>> +{
>
>> + if (pages && s)
>> + pr_info("Freeing %s memory: %ldK (%lx - %lx)\n",
>
> "%luKiB (0x%lx - 0x%lx)"?
Sure.
Regards!
Gerry
>
>> + s, pages << (PAGE_SHIFT - 10), start, end);
>> +
>> + return pages;
>> +}
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
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/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>