On Mon, Oct 25, 2010 at 01:15:19PM +0800, KAMEZAWA Hiroyuki wrote:
> On Mon, 25 Oct 2010 13:12:02 +0800
> Wu Fengguang <fengguang.wu@xxxxxxxxx> wrote:
>
> > do_migrate_range() offlines 1MB pages at one time and hence might be
> > called up to 16000 times when trying to offline 16GB memory.
>
> But size of memory section is not such big.
It's NR_OFFLINE_AT_ONCE_PAGES=256 (1MB).
> > It makes sense to avoid sending the costly IPIs to drain pages on all LRU for
> > the 99% cases that do_migrate_range() succeeds offlining some pages.
> >
>
> did you test ? I think this patch should be tested by IBM guys.
Only compile tested..
> > Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
>
> Sorry, this will HUNK. Could you wait until the end of merge window ?
OK, I'll resend.
>
> > ---
> > mm/memory_hotplug.c | 24 ++++++++++--------------
> > 1 file changed, 10 insertions(+), 14 deletions(-)
> >
> > --- linux-next.orig/mm/memory_hotplug.c 2010-10-25 11:20:47.000000000 +0800
> > +++ linux-next/mm/memory_hotplug.c 2010-10-25 13:07:10.000000000 +0800
> > @@ -788,7 +788,7 @@ static int offline_pages(unsigned long s
> > {
> > unsigned long pfn, nr_pages, expire;
> > long offlined_pages;
> > - int ret, drain, retry_max, node;
> > + int ret, retry_max, node;
> > struct zone *zone;
> > struct memory_notify arg;
> >
> > @@ -827,7 +827,6 @@ static int offline_pages(unsigned long s
> >
> > pfn = start_pfn;
> > expire = jiffies + timeout;
> > - drain = 0;
> > retry_max = 5;
> > repeat:
> > /* start memory hot removal */
> > @@ -838,34 +837,31 @@ repeat:
> > if (signal_pending(current))
> > goto failed_removal;
> > ret = 0;
> > - if (drain) {
> > - lru_add_drain_all();
> > - flush_scheduled_work();
>
> this flush_scheduled_work() is removed in recent work of Tejun Heo.
Ah yes.
> > - cond_resched();
> > - drain_all_pages();
> > - }
> > -
> > pfn = scan_lru_pages(start_pfn, end_pfn);
> > if (pfn) { /* We have page on LRU */
> > ret = do_migrate_range(pfn, end_pfn);
> > if (!ret) {
> > - drain = 1;
> > goto repeat;
> > } else {
> > if (ret < 0)
> > if (--retry_max == 0)
> > goto failed_removal;
> > yield();
> > - drain = 1;
> > + lru_add_drain_all();
> > + flush_scheduled_work();
> This flush is unnecessary.
OK.
> > + cond_resched();
> > + drain_all_pages();
>
> I think followin is better order.
>
> drain_all_pages(); # SEND IPI and asynchronous.
> lru_add_drain_pages(); # call schedule_work ony by one and it's synchronous.
> cond_resched(); # may not be unnecessary (lru_add_drain_pages() will sleep.)
That looks better. I'll remove cond_resched() too.
> > goto repeat;
> > }
> > }
> > - /* drain all zone's lru pagevec, this is asyncronous... */
> > +
> > + /* drain all zone's lru pagevec, this is asynchronous... */
> > lru_add_drain_all();
> > flush_scheduled_work();
> This flush() is dropped by recent works of Tejun Heo's workqueue updates.
OK.
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>