Re: [patch 2/3] Add flags parameter to reserve_bootmem_generic()

[Vivek Goyal <vgoyal@redhat.com>]

On Mon, Jun 09, 2008 at 12:01:15AM +0200, Johannes Weiner wrote:
> Hi,
> 
> Bernhard Walle <bwalle@suse.de> writes:
> 
> > --- a/arch/x86/mm/init_64.c
> > +++ b/arch/x86/mm/init_64.c
> > @@ -798,12 +798,13 @@ void free_initrd_mem(unsigned long start
> >  }
> >  #endif
> >  
> > -void __init reserve_bootmem_generic(unsigned long phys, unsigned len)
> > +int __init reserve_bootmem_generic(unsigned long phys, unsigned len, int flags)
> >  {
> >  #ifdef CONFIG_NUMA
> >  	int nid, next_nid;
> >  #endif
> >  	unsigned long pfn = phys >> PAGE_SHIFT;
> > +	int ret;
> >  
> >  	if (pfn >= end_pfn) {
> >  		/*
> > @@ -811,11 +812,11 @@ void __init reserve_bootmem_generic(unsi
> >  		 * firmware tables:
> >  		 */
> >  		if (pfn < max_pfn_mapped)
> > -			return;
> > +			return -EFAULT;
> 
> This seemed to be `just do nothing' behaviour.  Wouldn't 0 be more
> correct here?  Or something else so there is a difference between the
> path that does not print a warning (the one below) and the path that
> does?

Bernard,

This is interesting. IIUC, end_pfn represents end of physical RAM and
max_pfn_mapped represents, end of other tables like ACPI which are
mapped in higher regions.

Kdump first kernel always tries to reserve just physical RAM and nothing
else. So I am not sure what does above code do. Try to reserve a memory
which is not RAM but is in the region less than highest mapped entity and
in that case return silently without any warning. In what case do we
exercise this path?

Thanks
Vivek