Re: [-mm PATCH] register_memory/unregister_memory clean ups

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

On Tue, 2008-02-12 at 13:56 -0800, Badari Pulavarty wrote:
> > > +   /*
> > > +    * Its ugly, but this is the best I can do - HELP !!
> > > +    * We don't know where the allocations for section memmap and usemap
> > > +    * came from. If they are allocated at the boot time, they would come
> > > +    * from bootmem. If they are added through hot-memory-add they could be
> > > +    * from sla or vmalloc. If they are allocated as part of hot-mem-add
> > > +    * free them up properly. If they are allocated at boot, no easy way
> > > +    * to correctly free them :(
> > > +    */
> > > +   if (usemap) {
> > > +           if (PageSlab(virt_to_page(usemap))) {
> > > +                   kfree(usemap);
> > > +                   if (memmap)
> > > +                           __kfree_section_memmap(memmap, nr_pages);
> > > +           }
> > > +   }
> > > +}
> > 
> > Do what we did with the memmap and store some of its origination
> > information in the low bits.
> 
> Hmm. my understand of memmap is limited. Can you help me out here ?

Never mind.  That was a bad suggestion.  I do think it would be a good
idea to mark the 'struct page' of ever page we use as bootmem in some
way.  Perhaps page->private?  Otherwise, you can simply try all of the
possibilities and consider the remainder bootmem.  Did you ever find out
if we properly initialize the bootmem 'struct page's?

Please have mercy and put this in a helper, first of all.

static void free_usemap(unsigned long *usemap)
{
	if (!usemap_
		return;

	if (PageSlab(virt_to_page(usemap))) {
		kfree(usemap)
	} else if (is_vmalloc_addr(usemap)) {
		vfree(usemap);
	} else {
		int nid = page_to_nid(virt_to_page(usemap));
		bootmem_fun_here(NODE_DATA(nid), usemap);
	}
}

right?

> I was trying to use free_bootmem_node() to free up the allocations,
> but I need nodeid from which allocation came from :(

How is this any different from pfn_to_nid() on the thing?  Or, can you
not use that because we never init'd the bootmem 'struct page's?

If so, I think the *CORRECT* fix is to give the bootmem areas real
struct pages, probably at boot-time.

-- Dave