mm: Avoid putting a bad page back on the LRU v3

PATCH In review Russ Anderson 2 participants 3 messages May 9, 2008

Share & tools

Russ Anderson

author

2008-05-09 15:10 UTC #0 lore raw

PATCH 2/3 mm: Avoid putting a bad page back on the LRU v3

Prevent a page with a physical memory error from being placed back on the LRU. A new page flag (PG_memerror) is added on 64 bit architectures.

Signed-off-by Russ Anderson <[email protected]>

---
 include/linux/page-flags.h |   11 ++++++++++-
 include/linux/swap.h       |    1 +
 mm/migrate.c               |    2 ++
 mm/page_alloc.c            |    9 +++++----
 mm/swap.c                  |    4 ++++
 5 files changed, 22 insertions(+), 5 deletions(-)

Index: linus/mm/page_alloc.c
=================================--- linus.orig/mm/page_alloc.c 2008-05-09 09:20:04.389938802 -0500
+++ linus/mm/page_alloc.c 2008-05-09 09:20:11.302788361 -0500
@@ -71,6 +71,7 @@ unsigned long totalram_pages __read_most
unsigned long totalreserve_pages __read_mostly; long nr_swap_pages;
int percpu_pagelist_fraction;
+unsigned int totalbad_pages;

#ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
int pageblock_order __read_mostly;
@@ -602,10 +603,10 @@ static int prep_new_page(struct page *pa
bad_page(page);

/*
- * For now, we report if PG_reserved was found set, but do not
- * clear it, and do not allocate the page: as a safety net.
+ * For now, we report if PG_reserved or PG_memerror was found set, but
+ * do not clear it, and do not allocate the page: as a safety net.
*/
- if (PageReserved(page))
+ if (PageReserved(page) || PageMemError(page))
return 1;

page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_reclaim |
@@ -2474,7 +2475,7 @@ static void setup_zone_migrate_reserve(s
page = pfn_to_page(pfn);

/* Blocks with reserved pages will never free, skip them. */
- if (PageReserved(page))
+ if (PageReserved(page) || PageMemError(page))
continue;

block_migratetype = get_pageblock_migratetype(page);
Index: linus/mm/swap.c
=================================--- linus.orig/mm/swap.c 2008-05-09 09:19:40.466984064 -0500
+++ linus/mm/swap.c 2008-05-09 09:20:11.330791803 -0500
@@ -195,6 +195,8 @@ void lru_cache_add(struct page *page)
struct pagevec *pvec = &get_cpu_var(lru_add_pvecs);

page_cache_get(page);
+ if (unlikely(PageMemError(page)))
+ return; /* Don't add bad pages to the page list */
if (!pagevec_add(pvec, page))
__pagevec_lru_add(pvec);
put_cpu_var(lru_add_pvecs);
@@ -205,6 +207,8 @@ void lru_cache_add_active(struct page *p
struct pagevec *pvec = &get_cpu_var(lru_add_active_pvecs);

page_cache_get(page);
+ if (unlikely(PageMemError(page)))
+ return; /* Don't add bad pages to the page list */
if (!pagevec_add(pvec, page))
__pagevec_lru_add_active(pvec);
put_cpu_var(lru_add_active_pvecs);
Index: linus/include/linux/swap.h
=================================--- linus.orig/include/linux/swap.h 2008-05-09 09:19:40.470984561 -0500
+++ linus/include/linux/swap.h 2008-05-09 09:20:11.346793770 -0500
@@ -165,6 +165,7 @@ extern unsigned long totalreserve_pages;
extern long nr_swap_pages;
extern unsigned int nr_free_buffer_pages(void); extern unsigned int nr_free_pagecache_pages(void);
+extern unsigned int totalbad_pages;

/* Definition of global_page_state not available yet */ #define nr_free_pages() global_page_state(NR_FREE_PAGES)
Index: linus/mm/migrate.c
=================================--- linus.orig/mm/migrate.c 2008-05-09 09:19:40.470984561 -0500
+++ linus/mm/migrate.c 2008-05-09 09:20:11.366796228 -0500
@@ -721,6 +721,8 @@ unlock:
*/
list_del(&page->lru); move_to_lru(page);
+ if (PageMemError(page))
+ totalbad_pages++;
}

move_newpage:
Index: linus/include/linux/page-flags.h
=================================--- linus.orig/include/linux/page-flags.h 2008-05-09 09:20:04.401940277 -0500
+++ linus/include/linux/page-flags.h 2008-05-09 09:20:11.382798194 -0500
@@ -84,6 +84,7 @@ enum pageflags {
PG_private, /* If pagecache, has fs-private data */
PG_writeback, /* Page is under writeback */
#ifdef CONFIG_PAGEFLAGS_EXTENDED
+ PG_memerror, /* Page has a physical memory error */
PG_head, /* A head page */
PG_tail, /* A tail page */
#else
@@ -307,9 +308,17 @@ static inline void __ClearPageTail(struc

#endif /* !PAGEFLAGS_EXTENDED */

 			 1 << PG_buddy | 1 << PG_writeback | \
 			 1 << PG_slab  | 1 << PG_swapcache | 1 << PG_active)

+
+#ifdef CONFIG_PAGEFLAGS_EXTENDED
+PAGEFLAG(MemError, memerror)
+#define PAGE_FLAGS (PAGE_FLAGS_BASE | 1UL << PG_memerror)
+#else
+PAGEFLAG_FALSE(MemError)
+#define PAGE_FLAGS (PAGE_FLAGS_BASE)
+#endif

#define PAGE_FLAGS_RECLAIM (PAGE_FLAGS | 1 << PG_reclaim | 1 << PG_dirty)
#define PAGE_FLAGS_RESERVE (PAGE_FLAGS | 1 << PG_reserved)

Signature · 2 lines

Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          [email protected]

Andrew Morton

2008-05-09 20:11 UTC #1 lore raw

Quoted 4 lines · On Fri, 9 May 2008 10:10:58 -0500 Russ Anderson <[email protected]> wrote:

Prevent a page with a physical memory error from being placed back on the LRU. A new page flag (PG_memerror) is added on 64 bit architectures.

A little thing:

Quoted 48 lines

Index: linus/mm/page_alloc.c
=================================> --- linus.orig/mm/page_alloc.c 2008-05-09 09:20:04.389938802 -0500
+++ linus/mm/page_alloc.c 2008-05-09 09:20:11.302788361 -0500

(quoted diff) +5 -4

@@ -71,6 +71,7 @@ unsigned long totalram_pages __read_most
 unsigned long totalreserve_pages __read_mostly;
 long nr_swap_pages;
 int percpu_pagelist_fraction;
+unsigned int totalbad_pages;
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
 int pageblock_order __read_mostly;

@@ -602,10 +603,10 @@ static int prep_new_page(struct page *pa
 		bad_page(page);
 
 	/*
-	 * For now, we report if PG_reserved was found set, but do not
-	 * clear it, and do not allocate the page: as a safety net.
+	 * For now, we report if PG_reserved or PG_memerror was found set, but
+	 * do not clear it, and do not allocate the page: as a safety net.
 	 */
-	if (PageReserved(page))
+	if (PageReserved(page) || PageMemError(page))
 		return 1;
 
 	page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_reclaim |

@@ -2474,7 +2475,7 @@ static void setup_zone_migrate_reserve(s
 		page = pfn_to_page(pfn);
 
 		/* Blocks with reserved pages will never free, skip them. */
-		if (PageReserved(page))
+		if (PageReserved(page) || PageMemError(page))
 			continue;
 
 		block_migratetype = get_pageblock_migratetype(page);

Index: linus/mm/swap.c
=================================> --- linus.orig/mm/swap.c 2008-05-09 09:19:40.466984064 -0500
+++ linus/mm/swap.c 2008-05-09 09:20:11.330791803 -0500

(quoted diff) +2 -0

@@ -195,6 +195,8 @@ void lru_cache_add(struct page *page)
 	struct pagevec *pvec = &get_cpu_var(lru_add_pvecs);
 
 	page_cache_get(page);
+	if (unlikely(PageMemError(page)))
+		return;		/* Don't add bad pages to the page list */
 	if (!pagevec_add(pvec, page))
 		__pagevec_lru_add(pvec);
 	put_cpu_var(lru_add_pvecs);

Review void lru_cache_add_active(struct page *p · line 214

Quoted 8 lines

(quoted diff) +2 -0

@@ -205,6 +207,8 @@ void lru_cache_add_active(struct page *p
 	struct pagevec *pvec = &get_cpu_var(lru_add_active_pvecs);
 
 	page_cache_get(page);
+	if (unlikely(PageMemError(page)))
+		return;		/* Don't add bad pages to the page list */
 	if (!pagevec_add(pvec, page))
 		__pagevec_lru_add_active(pvec);
 	put_cpu_var(lru_add_active_pvecs);

These PageMemError() tests are happening in some pretty darn hot paths. But we've gone and added this overhead to a lot of architectures and configs which don't need it.

Should we tighten that up? Arrange for PageMemError() to evaluate to constant zero for all builds which don't actually implement "Migrate data off physical pages with correctable errors"?

Probably the way to implement that would be to add a new CONFIG_NEED_PAGE_MEM_ERROR and `select' that from the appropriate place in ia64 Kconfig. Which is pretty nasty, but a) we're nasty that way rather often and b) this time it _is_ a hot-path, so some nastiness is justifiable.

Russ Anderson

author

2008-05-09 21:51 UTC #2 lore raw

Quoted 29 lines · Andrew Morton

Quoted 13 lines · On Fri, 9 May 2008 10:10:58 -0500 Russ Anderson <[email protected]> wrote:
Index: linus/mm/swap.c
=================================> > --- linus.orig/mm/swap.c 2008-05-09 09:19:40.466984064 -0500
+++ linus/mm/swap.c 2008-05-09 09:20:11.330791803 -0500
(quoted diff) +2 -0
@@ -195,6 +195,8 @@ void lru_cache_add(struct page *page)
 	struct pagevec *pvec = &get_cpu_var(lru_add_pvecs);
 
 	page_cache_get(page);
+	if (unlikely(PageMemError(page)))
+		return;		/* Don't add bad pages to the page list */
 	if (!pagevec_add(pvec, page))
 		__pagevec_lru_add(pvec);
 	put_cpu_var(lru_add_pvecs);
Review void lru_cache_add_active(struct page *p · line 214
Quoted 8 lines
(quoted diff) +2 -0
@@ -205,6 +207,8 @@ void lru_cache_add_active(struct page *p
 	struct pagevec *pvec = &get_cpu_var(lru_add_active_pvecs);
 
 	page_cache_get(page);
+	if (unlikely(PageMemError(page)))
+		return;		/* Don't add bad pages to the page list */
 	if (!pagevec_add(pvec, page))
 		__pagevec_lru_add_active(pvec);
 	put_cpu_var(lru_add_active_pvecs);
These PageMemError() tests are happening in some pretty darn hot paths. But we've gone and added this overhead to a lot of architectures and configs which don't need it.

Quoted 3 lines

Should we tighten that up? Arrange for PageMemError() to evaluate to constant zero for all builds which don't actually implement "Migrate data off physical pages with correctable errors"?

Yes.

Quoted 5 lines

Probably the way to implement that would be to add a new CONFIG_NEED_PAGE_MEM_ERROR and `select' that from the appropriate place in ia64 Kconfig. Which is pretty nasty, but a) we're nasty that way rather often and b) this time it _is_ a hot-path, so some nastiness is justifiable.

Will do.

Signature · 2 lines

Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          [email protected]