Re: isolate_freepages_block and excessive CPU usage by OSD process

[Joonsoo Kim <iamjoonsoo.kim@lge.com>]

On Wed, Dec 10, 2014 at 04:06:19PM +0100, Vlastimil Babka wrote:
> On 12/05/2014 02:07 AM, Joonsoo Kim wrote:
> >------------>8-----------------
> > From b7daa232c327a4ebbb48ca0538a2dbf9ca83ca1f Mon Sep 17 00:00:00 2001
> >From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >Date: Fri, 5 Dec 2014 09:38:30 +0900
> >Subject: [PATCH] mm/compaction: stop the compaction if there isn't enough
> >  freepage
> >
> >After compaction_suitable() passed, there is no check whether the system
> >has enough memory to compact and blindly try to find freepage through
> >iterating all memory range. This causes excessive cpu usage in low free
> >memory condition and finally compaction would be failed. It makes sense
> >that compaction would be stopped if there isn't enough freepage. So,
> >this patch adds watermark check to isolate_freepages() in order to stop
> >the compaction in this case.
> >
> >Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >---
> >  mm/compaction.c |    9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> >diff --git a/mm/compaction.c b/mm/compaction.c
> >index e005620..31c4009 100644
> >--- a/mm/compaction.c
> >+++ b/mm/compaction.c
> >@@ -828,6 +828,7 @@ static void isolate_freepages(struct compact_control *cc)
> >  	unsigned long low_pfn;	     /* lowest pfn scanner is able to scan */
> >  	int nr_freepages = cc->nr_freepages;
> >  	struct list_head *freelist = &cc->freepages;
> >+	unsigned long watermark = low_wmark_pages(zone) + (2UL << cc->order);
> 
> Given that we maybe have already isolated up to 31 free pages (if
> cc->nr_migratepages is the maximum 32), then this is somewhat
> stricter than the check in isolation_suitable() (when nothing was
> isolated yet) and may interrupt us prematurely. We should allow for
> some slack.

Okay. Will allow some slack.

> 
> >
> >  	/*
> >  	 * Initialise the free scanner. The starting point is where we last
> >@@ -903,6 +904,14 @@ static void isolate_freepages(struct compact_control *cc)
> >  		 */
> >  		if (cc->contended)
> >  			break;
> >+
> >+		/*
> >+		 * Watermarks for order-0 must be met for compaction.
> >+		 * See compaction_suitable for more detailed explanation.
> >+		 */
> >+		if (!zone_watermark_ok(zone, 0, watermark,
> >+			cc->classzone_idx, cc->alloc_flags))
> >+			break;
> >  	}
> 
> I'm a also bit concerned about the overhead of doing this in each pageblock.

Yep, we can do it whenever SWAP_CLUSTER_MAX pageblock is scanned. It
will reduce overhead somewhat. I will change it.

> 
> I wonder if there could be a mechanism where a process entering
> reclaim or compaction with the goal of meeting the watermarks to
> allocate, should increase the watermarks needed for further parallel
> allocation attempts to pass. Then it shouldn't happen that somebody
> else steals the memory.

I don't know, neither.

Thanks.

> 
> >  	/* split_free_page does not map the pages */
> >
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>