Re: [PATCH][2/6]Memory preserving reboot using kexec

[Dave Hansen <haveblue@us.ibm.com>]

On Tue, 2004-08-17 at 05:07, Hariprasad Nellitheertha wrote:
> Regards, Hari


> +void __relocate_base_mem(unsigned long backup_addr, unsigned long backup_size)
> +{
> +       unsigned long pfn, pfn_max;
> +       void *src_addr, *dest_addr;
> +       struct page *page;
> +
> +       pfn_max = backup_size >> PAGE_SHIFT;
> +       for (pfn = 0; pfn < pfn_max; pfn++) {
> +               src_addr = phys_to_virt(pfn << PAGE_SHIFT);
> +               dest_addr = backup_addr + src_addr;
> +               if (!pfn_valid(pfn))
> +                       continue;
> +               page = pfn_to_page(pfn);
> +               if (PageReserved(page))
> +                       copy_page(dest_addr, src_addr);
> +       }
> +}

You're getting a little sloppy with your types in there.  I know you
probably aren't getting warnings for passing unsigned longs to
copy_page(), but you should probably still be passing pointers to it.  

I think the general convention is to keep physical addresses in unsigned
longs and virtual addresses in pointers.  Just keep that in mind.  

> +#define CRASH_BACKUP_BASE 0x08000000
> +#define CRASH_BACKUP_SIZE 0x01000000

What are these numbers?  Why do you need to define them when your config
option is off?

> +/*
> + * If we have booted due to a crash, max_pfn will be a very low value. We need
> + * to know the amount of memory that the previous kernel used.
> + */
> +unsigned long saved_max_pfn;

I'd probably put that comment next to the place where saved_max_pfn is
used instead of where it is declared.  


-- Dave