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
