Re: [Xen-devel] [PATCH v2, part1 01/29] mm: introduce common help functions to deal with reserved/managed pages

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

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)?

> +/*
> + * 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.

> +                                 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)"?

> +                       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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel