On Thu, 2008-02-28 at 18:24 -0600, Manish Ahuja wrote: > Initial patch for reserving memory in early boot, and freeing it later. > If the previous boot had ended with a crash, the reserved memory would contain > a copy of the crashed kernel data. > > Signed-off-by: Manish Ahuja <[EMAIL PROTECTED]> > Signed-off-by: Linas Vepstas <[EMAIL PROTECTED]>
Hi Manish,
A few comments inline ..
> Index: 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c 2008-02-28
> 21:57:52.000000000 -0600
> @@ -0,0 +1,105 @@
> +/*
> + * Hypervisor-assisted dump
> + *
> + * Linas Vepstas, Manish Ahuja 2008
> + * Copyright 2008 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/pfn.h>
> +#include <linux/swap.h>
> +
> +#include <asm/page.h>
> +#include <asm/phyp_dump.h>
> +#include <asm/machdep.h>
> +#include <asm/prom.h>
> +
> +/* Global, used to communicate data between early boot and late boot */
> +static struct phyp_dump phyp_dump_global;
> +struct phyp_dump *phyp_dump_info = &phyp_dump_global;
I don't see the point of this. You have a static (ie. non-global) struct
called phyp_dump_global, then you create a pointer to it and pass that
around. It could just be:
phyp_dump.h:
extern struct phyp_dump phyp_dump_info;
phyp_dump.c:
struct phyp_dump phyp_dump_info;
phyp_dump_info.foo = bar;
I also think the struct should be called phyp_dump_info, not phyp_dump -
it contains info about phyp_dump, not the dump itself.
> +
> +/**
> + * release_memory_range -- release memory previously lmb_reserved
> + * @start_pfn: starting physical frame number
> + * @nr_pages: number of pages to free.
> + *
> + * This routine will release memory that had been previously
> + * lmb_reserved in early boot. The released memory becomes
> + * available for genreal use.
> + */
> +static void
> +release_memory_range(unsigned long start_pfn, unsigned long nr_pages)
> +{
> + struct page *rpage;
> + unsigned long end_pfn;
> + long i;
> +
> + end_pfn = start_pfn + nr_pages;
> +
> + for (i = start_pfn; i <= end_pfn; i++) {
> + rpage = pfn_to_page(i);
> + if (PageReserved(rpage)) {
> + ClearPageReserved(rpage);
> + init_page_count(rpage);
> + __free_page(rpage);
> + totalram_pages++;
> + }
> + }
> +}
> +
> +static int __init phyp_dump_setup(void)
> +{
> + unsigned long start_pfn, nr_pages;
> +
> + /* If no memory was reserved in early boot, there is nothing to do */
> + if (phyp_dump_info->init_reserve_size == 0)
> + return 0;
> +
> + /* Release memory that was reserved in early boot */
> + start_pfn = PFN_DOWN(phyp_dump_info->init_reserve_start);
> + nr_pages = PFN_DOWN(phyp_dump_info->init_reserve_size);
> + release_memory_range(start_pfn, nr_pages);
> +
> + return 0;
> +}
> +machine_subsys_initcall(pseries, phyp_dump_setup);
> +
> +int __init early_init_dt_scan_phyp_dump(unsigned long node,
> + const char *uname, int depth, void *data)
> +{
> +#ifdef CONFIG_PHYP_DUMP
> + const unsigned int *sizes;
> +
> + phyp_dump_info->phyp_dump_configured = 0;
> + phyp_dump_info->phyp_dump_is_active = 0;
> +
> + if (depth != 1 || strcmp(uname, "rtas") != 0)
> + return 0;
> +
> + if (of_get_flat_dt_prop(node, "ibm,configure-kernel-dump", NULL))
> + phyp_dump_info->phyp_dump_configured++;
> +
> + if (of_get_flat_dt_prop(node, "ibm,dump-kernel", NULL))
> + phyp_dump_info->phyp_dump_is_active++;
> +
> + sizes = of_get_flat_dt_prop(node, "ibm,configure-kernel-dump-sizes",
> + NULL);
> + if (!sizes)
> + return 0;
> +
> + if (sizes[0] == 1)
> + phyp_dump_info->cpu_state_size = *((unsigned long *)&sizes[1]);
> +
> + if (sizes[3] == 2)
> + phyp_dump_info->hpte_region_size =
> + *((unsigned long *)&sizes[4]);
> +#endif
This doesn't need to be inside #ifdef, you have a dummy version already
defined in the header file.
> Index: 2.6.25-rc1/arch/powerpc/kernel/prom.c
> ===================================================================
> --- 2.6.25-rc1.orig/arch/powerpc/kernel/prom.c 2008-02-28
> 21:54:57.000000000 -0600
> +++ 2.6.25-rc1/arch/powerpc/kernel/prom.c 2008-02-28 21:55:27.000000000
> -0600
> @@ -1039,6 +1040,51 @@ static void __init early_reserve_mem(voi
> #endif
> }
>
> +#ifdef CONFIG_PHYP_DUMP
> +/**
> + * reserve_crashed_mem() - reserve all not-yet-dumped mmemory
> + *
> + * This routine may reserve memory regions in the kernel only
> + * if the system is supported and a dump was taken in last
> + * boot instance or if the hardware is supported and the
> + * scratch area needs to be setup. In other instances it returns
> + * without reserving anything. The memory in case of dump being
> + * active is freed when the dump is collected (by userland tools).
> + */
> +static void __init reserve_crashed_mem(void)
This could do with a name change IMO, eg. phyp_dump_reserve_mem() or
something.
> +{
> + unsigned long base, size;
> + if (!phyp_dump_info->phyp_dump_configured) {
> + printk(KERN_ERR "Phyp-dump not supported on this hardware\n");
> + return;
> + }
> +
> + if (phyp_dump_info->phyp_dump_is_active) {
> + /* Reserve *everything* above RMR.Area freed by userland tools*/
> + base = PHYP_DUMP_RMR_END;
> + size = lmb_end_of_DRAM() - base;
> +
> + /* XXX crashed_ram_end is wrong, since it may be beyond
> + * the memory_limit, it will need to be adjusted. */
> + lmb_reserve(base, size);
> +
> + phyp_dump_info->init_reserve_start = base;
> + phyp_dump_info->init_reserve_size = size;
> + } else {
> + size = phyp_dump_info->cpu_state_size +
> + phyp_dump_info->hpte_region_size +
> + PHYP_DUMP_RMR_END;
> + base = lmb_end_of_DRAM() - size;
> + lmb_reserve(base, size);
> + phyp_dump_info->init_reserve_start = base;
> + phyp_dump_info->init_reserve_size = size;
> + }
> +}
> +#else
> +static inline void __init reserve_crashed_mem(void) {}
> +#endif /* CONFIG_PHYP_DUMP && CONFIG_PPC_RTAS */
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
