Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1524011 > unrolled thread

[patch 1/2] mm, zone: track number of pages in free area by migratetype

Back to article view | Back to linux.kernel

Contents

  [patch 1/2] mm, zone: track number of pages in free area by  migratetype David Rientjes <rientjes@google.com> - 2016-11-17 02:40 +0100
    Re: [patch 1/2] mm, zone: track number of pages in free area by  migratetype Vlastimil Babka <vbabka@suse.cz> - 2016-11-17 18:30 +0100
      Re: [patch 1/2] mm, zone: track number of pages in free area by  migratetype David Rientjes <rientjes@google.com> - 2016-11-17 23:20 +0100
        Re: [patch 1/2] mm, zone: track number of pages in free area by  migratetype Vlastimil Babka <vbabka@suse.cz> - 2016-11-18 22:00 +0100

#1524011 — [patch 1/2] mm, zone: track number of pages in free area by migratetype

Each zone's free_area tracks the number of free pages for all free lists.
This does not allow the number of free pages for a specific migratetype
to be determined without iterating its free list.

An upcoming change will use this information to preclude doing async
memory compaction when the number of MIGRATE_UNMOVABLE pageblocks is
below a certain threshold.

The total number of free pages is still tracked, however, to not make
zone_watermark_ok() more expensive.  Reading /proc/pagetypeinfo, however,
is faster.

This patch introduces no functional change and increases the amount of
per-zone metadata at worst by 48 bytes per memory zone (when CONFIG_CMA
and CONFIG_MEMORY_ISOLATION are enabled).

Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/mmzone.h |  3 ++-
 mm/compaction.c        |  4 ++--
 mm/page_alloc.c        | 47 ++++++++++++++++++++++++++++-------------------
 mm/vmstat.c            | 18 +++++-------------
 4 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -89,7 +89,8 @@ extern int page_group_by_mobility_disabled;
 
 struct free_area {
 	struct list_head	free_list[MIGRATE_TYPES];
-	unsigned long		nr_free;
+	unsigned long		nr_free[MIGRATE_TYPES];
+	unsigned long		total_free;
 };
 
 struct pglist_data;
diff --git a/mm/compaction.c b/mm/compaction.c
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1320,13 +1320,13 @@ static enum compact_result __compact_finished(struct zone *zone, struct compact_
 		bool can_steal;
 
 		/* Job done if page is free of the right migratetype */
-		if (!list_empty(&area->free_list[migratetype]))
+		if (area->nr_free[migratetype])
 			return COMPACT_SUCCESS;
 
 #ifdef CONFIG_CMA
 		/* MIGRATE_MOVABLE can fallback on MIGRATE_CMA */
 		if (migratetype == MIGRATE_MOVABLE &&
-			!list_empty(&area->free_list[MIGRATE_CMA]))
+						area->nr_free[MIGRATE_CMA])
 			return COMPACT_SUCCESS;
 #endif
 		/*
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -821,7 +821,8 @@ static inline void __free_one_page(struct page *page,
 			clear_page_guard(zone, buddy, order, migratetype);
 		} else {
 			list_del(&buddy->lru);
-			zone->free_area[order].nr_free--;
+			zone->free_area[order].nr_free[migratetype]--;
+			zone->free_area[order].total_free--;
 			rmv_page_order(buddy);
 		}
 		combined_idx = buddy_idx & page_idx;
@@ -880,7 +881,8 @@ static inline void __free_one_page(struct page *page,
 
 	list_add(&page->lru, &zone->free_area[order].free_list[migratetype]);
 out:
-	zone->free_area[order].nr_free++;
+	zone->free_area[order].nr_free[migratetype]++;
+	zone->free_area[order].total_free++;
 }
 
 /*
@@ -1648,7 +1650,8 @@ static inline void expand(struct zone *zone, struct page *page,
 			continue;
 
 		list_add(&page[size].lru, &area->free_list[migratetype]);
-		area->nr_free++;
+		area->nr_free[migratetype]++;
+		area->total_free++;
 		set_page_order(&page[size], high);
 	}
 }
@@ -1802,7 +1805,8 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
 			continue;
 		list_del(&page->lru);
 		rmv_page_order(page);
-		area->nr_free--;
+		area->nr_free[migratetype]--;
+		area->total_free--;
 		expand(zone, page, order, current_order, area, migratetype);
 		set_pcppage_migratetype(page, migratetype);
 		return page;
@@ -1991,7 +1995,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order,
 	int i;
 	int fallback_mt;
 
-	if (area->nr_free == 0)
+	if (!area->total_free)
 		return -1;
 
 	*can_steal = false;
@@ -2000,7 +2004,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order,
 		if (fallback_mt == MIGRATE_TYPES)
 			break;
 
-		if (list_empty(&area->free_list[fallback_mt]))
+		if (!area->nr_free[fallback_mt])
 			continue;
 
 		if (can_steal_fallback(order, migratetype))
@@ -2163,7 +2167,8 @@ __rmqueue_fallback(struct zone *zone, unsigned int order, int start_migratetype)
 			steal_suitable_fallback(zone, page, start_migratetype);
 
 		/* Remove the page from the freelists */
-		area->nr_free--;
+		area->nr_free[fallback_mt]--;
+		area->total_free--;
 		list_del(&page->lru);
 		rmv_page_order(page);
 
@@ -2549,7 +2554,8 @@ int __isolate_free_page(struct page *page, unsigned int order)
 
 	/* Remove page from free list */
 	list_del(&page->lru);
-	zone->free_area[order].nr_free--;
+	zone->free_area[order].nr_free[mt]--;
+	zone->free_area[order].total_free--;
 	rmv_page_order(page);
 
 	/*
@@ -2808,22 +2814,19 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
 		struct free_area *area = &z->free_area[o];
 		int mt;
 
-		if (!area->nr_free)
+		if (!area->total_free)
 			continue;
 
 		if (alloc_harder)
 			return true;
 
-		for (mt = 0; mt < MIGRATE_PCPTYPES; mt++) {
-			if (!list_empty(&area->free_list[mt]))
+		for (mt = 0; mt < MIGRATE_PCPTYPES; mt++)
+			if (area->nr_free[mt])
 				return true;
-		}
 
 #ifdef CONFIG_CMA
-		if ((alloc_flags & ALLOC_CMA) &&
-		    !list_empty(&area->free_list[MIGRATE_CMA])) {
+		if ((alloc_flags & ALLOC_CMA) && area->nr_free[MIGRATE_CMA])
 			return true;
-		}
 #endif
 	}
 	return false;
@@ -4431,12 +4434,12 @@ void show_free_areas(unsigned int filter)
 			struct free_area *area = &zone->free_area[order];
 			int type;
 
-			nr[order] = area->nr_free;
+			nr[order] = area->total_free;
 			total += nr[order] << order;
 
 			types[order] = 0;
 			for (type = 0; type < MIGRATE_TYPES; type++) {
-				if (!list_empty(&area->free_list[type]))
+				if (area->nr_free[type])
 					types[order] |= 1 << type;
 			}
 		}
@@ -5100,8 +5103,10 @@ static void __meminit zone_init_free_lists(struct zone *zone)
 	unsigned int order, t;
 	for_each_migratetype_order(order, t) {
 		INIT_LIST_HEAD(&zone->free_area[order].free_list[t]);
-		zone->free_area[order].nr_free = 0;
+		zone->free_area[order].nr_free[t] = 0;
 	}
+	for (order = 0; order < MAX_ORDER; order++)
+		zone->free_area[order].total_free = 0;
 }
 
 #ifndef __HAVE_ARCH_MEMMAP_INIT
@@ -7416,6 +7421,8 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
 	spin_lock_irqsave(&zone->lock, flags);
 	pfn = start_pfn;
 	while (pfn < end_pfn) {
+		int migratetype;
+
 		if (!pfn_valid(pfn)) {
 			pfn++;
 			continue;
@@ -7438,9 +7445,11 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
 		pr_info("remove from free list %lx %d %lx\n",
 			pfn, 1 << order, end_pfn);
 #endif
+		migratetype = get_pageblock_migratetype(page);
 		list_del(&page->lru);
 		rmv_page_order(page);
-		zone->free_area[order].nr_free--;
+		zone->free_area[order].nr_free[migratetype]--;
+		zone->free_area[order].total_free--;
 		for (i = 0; i < (1 << order); i++)
 			SetPageReserved((page+i));
 		pfn += (1 << order);
diff --git a/mm/vmstat.c b/mm/vmstat.c
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -846,7 +846,7 @@ static void fill_contig_page_info(struct zone *zone,
 		unsigned long blocks;
 
 		/* Count number of free blocks */
-		blocks = zone->free_area[order].nr_free;
+		blocks = zone->free_area[order].total_free;
 		info->free_blocks_total += blocks;
 
 		/* Count free base pages */
@@ -1146,7 +1146,7 @@ static void frag_show_print(struct seq_file *m, pg_data_t *pgdat,
 
 	seq_printf(m, "Node %d, zone %8s ", pgdat->node_id, zone->name);
 	for (order = 0; order < MAX_ORDER; ++order)
-		seq_printf(m, "%6lu ", zone->free_area[order].nr_free);
+		seq_printf(m, "%6lu ", zone->free_area[order].total_free);
 	seq_putc(m, '\n');
 }
 
@@ -1170,17 +1170,9 @@ static void pagetypeinfo_showfree_print(struct seq_file *m,
 					pgdat->node_id,
 					zone->name,
 					migratetype_names[mtype]);
-		for (order = 0; order < MAX_ORDER; ++order) {
-			unsigned long freecount = 0;
-			struct free_area *area;
-			struct list_head *curr;
-
-			area = &(zone->free_area[order]);
-
-			list_for_each(curr, &area->free_list[mtype])
-				freecount++;
-			seq_printf(m, "%6lu ", freecount);
-		}
+		for (order = 0; order < MAX_ORDER; ++order)
+			seq_printf(m, "%6lu ",
+				   zone->free_area[order].nr_free[mtype]);
 		seq_putc(m, '\n');
 	}
 }

[toc] | [next] | [standalone]

#1524565

On 11/17/2016 02:32 AM, David Rientjes wrote:
> Each zone's free_area tracks the number of free pages for all free lists.
> This does not allow the number of free pages for a specific migratetype
> to be determined without iterating its free list.
> 
> An upcoming change will use this information to preclude doing async
> memory compaction when the number of MIGRATE_UNMOVABLE pageblocks is
> below a certain threshold.
> 
> The total number of free pages is still tracked, however, to not make
> zone_watermark_ok() more expensive.  Reading /proc/pagetypeinfo, however,
> is faster.

Yeah I've already seen a case with /proc/pagetypeinfo causing soft
lockups due to high number of iterations...

> This patch introduces no functional change and increases the amount of
> per-zone metadata at worst by 48 bytes per memory zone (when CONFIG_CMA
> and CONFIG_MEMORY_ISOLATION are enabled).

Isn't it 48 bytes per zone and order?

> Signed-off-by: David Rientjes <rientjes@google.com>

I'd be for this if there are no performance regressions. It affects hot
paths and increases cache footprint. I think at least some allocator
intensive microbenchmark should be used.

Vlastimil

[toc] | [prev] | [next] | [standalone]

#1524848

On Thu, 17 Nov 2016, Vlastimil Babka wrote:

> > The total number of free pages is still tracked, however, to not make
> > zone_watermark_ok() more expensive.  Reading /proc/pagetypeinfo, however,
> > is faster.
> 
> Yeah I've already seen a case with /proc/pagetypeinfo causing soft
> lockups due to high number of iterations...
> 

Thanks for taking a look at the patchset!

Wow, I haven't seen /proc/pagetypeinfo soft lockups yet, I thought this 
was a relatively minor point :)  But it looks like we need some 
improvement in this behavior independent of memory compaction anyway.

> > This patch introduces no functional change and increases the amount of
> > per-zone metadata at worst by 48 bytes per memory zone (when CONFIG_CMA
> > and CONFIG_MEMORY_ISOLATION are enabled).
> 
> Isn't it 48 bytes per zone and order?
> 

Yes, sorry, I'll fix that in v2.  I think less than half a kilobyte for 
each memory zone is satisfactory for extra tracking, compaction 
improvements, and optimized /proc/pagetypeinfo, though.

> > Signed-off-by: David Rientjes <rientjes@google.com>
> 
> I'd be for this if there are no performance regressions. It affects hot
> paths and increases cache footprint. I think at least some allocator
> intensive microbenchmark should be used.
> 

I can easily implement a test to stress movable page allocations from 
fallback MIGRATE_UNMOVABLE pageblocks and freeing back to the same 
pageblocks.  I assume we're not interested in memory offline benchmarks.

What do you think about the logic presented in patch 2/2?  Are you 
comfortable with a hard-coded ratio such as 1/64th of free memory or would 
you prefer to look at the zone's watermark with the number of free pages 
from MIGRATE_MOVABLE pageblocks rather than NR_FREE_PAGES?  I was split 
between the two options.

[toc] | [prev] | [next] | [standalone]

#1525711

On 11/17/2016 11:11 PM, David Rientjes wrote:
> On Thu, 17 Nov 2016, Vlastimil Babka wrote:
> 
>>> The total number of free pages is still tracked, however, to not make
>>> zone_watermark_ok() more expensive.  Reading /proc/pagetypeinfo, however,
>>> is faster.
>>
>> Yeah I've already seen a case with /proc/pagetypeinfo causing soft
>> lockups due to high number of iterations...
>>
> 
> Thanks for taking a look at the patchset!
> 
> Wow, I haven't seen /proc/pagetypeinfo soft lockups yet, I thought this 
> was a relatively minor point :)

Well to be honest, it was a system misconfigured with numa=off which
made the lists both longer and more numa-distant. But nevertheless, we
might get there. It's not nice when userspace can so easily trigger long
iterations under the zone/node lock...

> But it looks like we need some 
> improvement in this behavior independent of memory compaction anyway.

Yeah.

>>> This patch introduces no functional change and increases the amount of
>>> per-zone metadata at worst by 48 bytes per memory zone (when CONFIG_CMA
>>> and CONFIG_MEMORY_ISOLATION are enabled).
>>
>> Isn't it 48 bytes per zone and order?
>>
> 
> Yes, sorry, I'll fix that in v2.  I think less than half a kilobyte for 
> each memory zone is satisfactory for extra tracking, compaction 
> improvements, and optimized /proc/pagetypeinfo, though.

I'm not worried about memory usage, but perhaps cache usage.

>>> Signed-off-by: David Rientjes <rientjes@google.com>
>>
>> I'd be for this if there are no performance regressions. It affects hot
>> paths and increases cache footprint. I think at least some allocator
>> intensive microbenchmark should be used.
>>
> 
> I can easily implement a test to stress movable page allocations from 
> fallback MIGRATE_UNMOVABLE pageblocks and freeing back to the same 
> pageblocks.  I assume we're not interested in memory offline benchmarks.

I meant just allocation benchmarks to see how much the extra operations
and cache footprint matters.

> What do you think about the logic presented in patch 2/2?  Are you 
> comfortable with a hard-coded ratio such as 1/64th of free memory or would 
> you prefer to look at the zone's watermark with the number of free pages 
> from MIGRATE_MOVABLE pageblocks rather than NR_FREE_PAGES?  I was split 
> between the two options.

The second options makes more sense to me intuitively as it resembles
what we've been doing until now. Maybe just don't require such a large
gap as compaction_suitable does?

> --
> 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>
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web