Re: [RFC PATCH] rework memory hotplug onlining

[Michal Hocko]

On Wed 15-03-17 23:08:14, Kani, Toshimitsu wrote:
> On Wed, 2017-03-15 at 10:13 +0100, Michal Hocko wrote:
>  :
> > @@ -388,39 +389,44 @@ static ssize_t show_valid_zones(struct device
> > *dev,
> >  				struct device_attribute *attr, char
> > *buf)
> >  {
> >  	struct memory_block *mem = to_memory_block(dev);
> > -	unsigned long start_pfn, end_pfn;
> > -	unsigned long valid_start, valid_end, valid_pages;
> > -	unsigned long nr_pages = PAGES_PER_SECTION *
> > sections_per_block;
> > +	unsigned long start_pfn, nr_pages;
> > +	bool append = false;
> >  	struct zone *zone;
> > -	int zone_shift = 0;
> > +	int nid;
> >  
> >  	start_pfn = section_nr_to_pfn(mem->start_section_nr);
> > -	end_pfn = start_pfn + nr_pages;
> > +	zone = page_zone(pfn_to_page(start_pfn));
> > +	nr_pages = PAGES_PER_SECTION * sections_per_block;
> >  
> > -	/* The block contains more than one zone can not be
> > offlined. */
> > -	if (!test_pages_in_a_zone(start_pfn, end_pfn, &valid_start,
> > &valid_end))
> > +	/*
> > +	 * The block contains more than one zone can not be
> > offlined.
> > +	 * This can happen e.g. for ZONE_DMA and ZONE_DMA32
> > +	 */
> > +	if (!test_pages_in_a_zone(start_pfn, start_pfn + nr_pages,
> > NULL, NULL))
> >  		return sprintf(buf, "none\n");
> >  
> > -	zone = page_zone(pfn_to_page(valid_start));
> 
> Please do not remove the fix made in a96dfddbcc043. zone needs to be
> set from valid_start, not from start_pfn.

Thanks for pointing this out. I was scratching my head about this part
but was too tired from previous git archeology so I didn't check the
history of this particular part.

I will restore the original behavior but before I do that I am really
curious whether partial memblocks are even supported for onlining. Maybe
I am missing something but I do not see any explicit checks for NULL
struct page when we set zone boundaries or online a memblock. Is it
possible those memblocks are just never hotplugable?
-- 
Michal Hocko
SUSE Labs