On Mon, 2013-05-20 at 13:55 -0600, Toshi Kani wrote:
> On Mon, 2013-05-20 at 21:47 +0200, Rafael J. Wysocki wrote:
> > On Monday, May 20, 2013 11:27:56 AM Toshi Kani wrote:
> > > On Sun, 2013-05-19 at 01:34 +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
>
> :
>
> > > > - lock_memory_hotplug();
> > > > -
> > > > - /*
> > > > - * we have offlined all memory blocks like this:
> > > > - * 1. lock memory hotplug
> > > > - * 2. offline a memory block
> > > > - * 3. unlock memory hotplug
> > > > - *
> > > > - * repeat step1-3 to offline the memory block. All memory blocks
> > > > - * must be offlined before removing memory. But we don't hold
> > > > the
> > > > - * lock in the whole operation. So we should check whether all
> > > > - * memory blocks are offlined.
> > > > - */
> > > > -
> > > > - ret = walk_memory_range(start_pfn, end_pfn, NULL,
> > > > - is_memblock_offlined_cb);
> > > > - if (ret) {
> > > > - unlock_memory_hotplug();
> > > > - return ret;
> > > > - }
> > > > -
> > >
> > > I think the above procedure is still useful for safe guard.
> >
> > But then it shoud to BUG_ON() instead of returning an error (which isn't
> > very
> > useful for anything now).
>
> Right since we cannot fail at that state.
>
> > > > - /* remove memmap entry */
> > > > - firmware_map_remove(start, start + size, "System RAM");
> > > > -
> > > > - arch_remove_memory(start, size);
> > > > -
> > > > - try_offline_node(nid);
> > >
> > > The above procedure performs memory hot-delete specific operations and
> > > is necessary.
> >
> > OK, I see. I'll replace this patch with something simpler, then.
>
> Thanks.
>
> > What about the other patches in the series?
>
> I will send you my comments later (a bit interrupted for other thing
> now).
Other patches look good. For patch 1/5 to 4/5:
Acked-by: Toshi Kani <toshi.k...@hp.com>
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
