RFC: Implement hwpoison on free for soft offlining

RFC: Implement hwpoison on free for soft offlining

[Andi Kleen]

Here's a somewhat experimental patch to improve soft offlining
in hwpoison, but allowing hwpoison on free for not directly
freeable page types. It should work for nearly all
left over page types that get eventually freed, so this makes
soft offlining nearly universal. The only non handleable page
types are now pages that never get freed.

Drawback: It needs an additional page flag. Cannot set hwpoison
directly because that would not be "soft" and cause errors.

Since the flags are scarce on 32bit I only enabled it on 64bit.

Comments?

-Andi

[PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Hard offlining was always able to offline most kernel pages by setting the
HWPoison flag on it and waiting for the next free. This works on all
pages that get eventually freed.

This didn't work for soft offlining unfortunately, because it cannot
safely set the HWPoison flag on a still alive page. Handle this
by introducing a second page flag HWPoisonOnFree that is handled
by page free and enables HWPoison then. This does not add any
overhead to page free because it can be handled as a bad flag
which are already checked for.

Since page flags are scarce on 32bit architectures this is only done on
64bit .

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 include/linux/mm.h         |    1 +
 include/linux/page-flags.h |   15 +++++++++++++-
 mm/Kconfig                 |    4 +++
 mm/memory-failure.c        |   46 +++++++++++++++++++++++++++++++++++++++++++-
 mm/page_alloc.c            |    2 +
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 74949fb..f7da94d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1481,6 +1481,7 @@ enum mf_flags {
 extern void memory_failure(unsigned long pfn, int trapno);
 extern int __memory_failure(unsigned long pfn, int trapno, int flags);
 extern int unpoison_memory(unsigned long pfn);
+extern void hwpoison_page_on_free(struct page *p);
 extern int sysctl_memory_failure_early_kill;
 extern int sysctl_memory_failure_recovery;
 extern void shake_page(struct page *p, int access);
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 6fa3178..2d4196b 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -107,6 +107,9 @@ enum pageflags {
 #endif
 #ifdef CONFIG_MEMORY_FAILURE
 	PG_hwpoison,		/* hardware poisoned page. Don't touch */
+#ifdef CONFIG_HWPOISON_ON_FREE
+	PG_hwpoison_on_free,	/* hwpoison on free */
+#endif
 #endif
 	__NR_PAGEFLAGS,
 
@@ -280,6 +283,15 @@ PAGEFLAG_FALSE(HWPoison)
 #define __PG_HWPOISON 0
 #endif
 
+#if defined(CONFIG_MEMORY_FAILURE) && BITS_PER_LONG == 64
+PAGEFLAG(HWPoisonOnFree, hwpoison_on_free)
+TESTSCFLAG(HWPoisonOnFree, hwpoison_on_free)
+#define __PG_HWPOISON_ON_FREE (1UL << PG_hwpoison_on_free)
+#else
+PAGEFLAG_FALSE(HWPoisonOnFree)
+#define __PG_HWPOISON_ON_FREE 0
+#endif
+
 u64 stable_page_flags(struct page *page);
 
 static inline int PageUptodate(struct page *page)
@@ -406,7 +418,8 @@ static inline void __ClearPageTail(struct page *page)
 	 1 << PG_private | 1 << PG_private_2 | \
 	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
 	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
-	 1 << PG_unevictable | __PG_MLOCKED | __PG_HWPOISON)
+	 1 << PG_unevictable | __PG_MLOCKED | __PG_HWPOISON | \
+	 __PG_HWPOISON_ON_FREE)
 
 /*
  * Flags checked when a page is prepped for return by the page allocator.
diff --git a/mm/Kconfig b/mm/Kconfig
index f0fb912..ede5444 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -269,6 +269,10 @@ config MEMORY_FAILURE
 	  even when some of its memory has uncorrected errors. This requires
 	  special hardware support and typically ECC memory.
 
+config HWPOISON_ON_FREE
+	depends on MEMORY_FAILURE && 64BIT
+	def_bool y	
+
 config HWPOISON_INJECT
 	tristate "HWPoison pages injector"
 	depends on MEMORY_FAILURE && DEBUG_KERNEL && PROC_FS
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 9c26eec..34901f6 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1268,6 +1268,14 @@ int soft_offline_page(struct page *page, int flags)
 	if (ret == 0)
 		goto done;
 
+#ifdef CONFIG_HWPOISON_ON_FREE
+	/*
+	 * When the page gets freed make sure to poison
+	 * it immediately.
+	 */
+	SetPageHWPoisonOnFree(page);
+#endif
+
 	/*
 	 * Page cache page we can handle?
 	 */
@@ -1279,7 +1287,15 @@ int soft_offline_page(struct page *page, int flags)
 		shake_page(page, 1);
 
 		/*
-		 * Did it turn free?
+		 * Did it get poisoned on free?
+		 */
+		if (PageHWPoison(page)) {
+			pr_info("soft_offline: %#lx: Page freed through shaking\n", pfn);
+			return 0;
+		}
+
+		/* 
+		 * Try to grab it as a free page again.
 		 */
 		ret = get_any_page(page, pfn, 0);
 		if (ret < 0)
@@ -1287,12 +1303,23 @@ int soft_offline_page(struct page *page, int flags)
 		if (ret == 0)
 			goto done;
 	}
+
+	if (PageHWPoisonOnFree(page)) {
+		pr_info("soft_offline: %#lx: Delaying poision of unknown page %lx to free\n",
+			pfn, page->flags);
+		return -EIO; /* or 0? */
+	}
+
 	if (!PageLRU(page)) {
 		pr_debug("soft_offline: %#lx: unknown non LRU page type %lx\n",
 				pfn, page->flags);
 		return -EIO;
 	}
 
+	/* 
+	 * Normal page cache page here.
+	 */
+
 	lock_page(page);
 	wait_on_page_writeback(page);
 
@@ -1389,3 +1416,20 @@ int is_hwpoison_address(unsigned long addr)
 	return is_hwpoison_entry(entry);
 }
 EXPORT_SYMBOL_GPL(is_hwpoison_address);
+
+#ifdef CONFIG_HWPOISON_ON_FREE
+
+/* 
+ * HWPoison a page after freeing. 
+ * This is the fallback path for most pages we cannot free early.
+ */
+void hwpoison_page_on_free(struct page *p)
+{
+	get_page(p);
+	SetPageHWPoison(p);
+	ClearPageHWPoisonOnFree(p);
+	pr_info("MCE: %#lx: Delayed poisioning of page after free\n",
+		page_to_pfn(p));
+	atomic_long_inc(&mce_bad_pages);
+}
+#endif
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a8cfa9c..519c24c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -564,6 +564,8 @@ static inline int free_pages_check(struct page *page)
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |
 		(page->flags & PAGE_FLAGS_CHECK_AT_FREE))) {
+		if (PageHWPoisonOnFree(page))
+			hwpoison_page_on_free(page);
 		bad_page(page);
 		return 1;
 	}
-- 
1.7.1

Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Wu Fengguang]

On Thu, Oct 07, 2010 at 06:09:11AM +0800, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Hard offlining was always able to offline most kernel pages by setting the
> HWPoison flag on it and waiting for the next free. This works on all
> pages that get eventually freed.
> 
> This didn't work for soft offlining unfortunately, because it cannot
> safely set the HWPoison flag on a still alive page. Handle this
> by introducing a second page flag HWPoisonOnFree that is handled
> by page free and enables HWPoison then. This does not add any
> overhead to page free because it can be handled as a bad flag
> which are already checked for.
> 
> Since page flags are scarce on 32bit architectures this is only done on
> 64bit .

I have no problem with the 64bit only page flag.

> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  include/linux/mm.h         |    1 +
>  include/linux/page-flags.h |   15 +++++++++++++-
>  mm/Kconfig                 |    4 +++
>  mm/memory-failure.c        |   46 +++++++++++++++++++++++++++++++++++++++++++-
>  mm/page_alloc.c            |    2 +
>  5 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 74949fb..f7da94d 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1481,6 +1481,7 @@ enum mf_flags {
>  extern void memory_failure(unsigned long pfn, int trapno);
>  extern int __memory_failure(unsigned long pfn, int trapno, int flags);
>  extern int unpoison_memory(unsigned long pfn);
> +extern void hwpoison_page_on_free(struct page *p);
>  extern int sysctl_memory_failure_early_kill;
>  extern int sysctl_memory_failure_recovery;
>  extern void shake_page(struct page *p, int access);
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 6fa3178..2d4196b 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -107,6 +107,9 @@ enum pageflags {
>  #endif
>  #ifdef CONFIG_MEMORY_FAILURE
>  	PG_hwpoison,		/* hardware poisoned page. Don't touch */
> +#ifdef CONFIG_HWPOISON_ON_FREE
> +	PG_hwpoison_on_free,	/* hwpoison on free */
> +#endif
>  #endif
>  	__NR_PAGEFLAGS,
>  
> @@ -280,6 +283,15 @@ PAGEFLAG_FALSE(HWPoison)
>  #define __PG_HWPOISON 0
>  #endif
>  
> +#if defined(CONFIG_MEMORY_FAILURE) && BITS_PER_LONG == 64

We have the simpler CONFIG_HWPOISON_ON_FREE :)

> +PAGEFLAG(HWPoisonOnFree, hwpoison_on_free)
> +TESTSCFLAG(HWPoisonOnFree, hwpoison_on_free)
> +#define __PG_HWPOISON_ON_FREE (1UL << PG_hwpoison_on_free)
> +#else
> +PAGEFLAG_FALSE(HWPoisonOnFree)

Could define SETPAGEFLAG_NOOP(HWPoisonOnFree) too, for eliminating an
#ifdef in the .c file.

> +#define __PG_HWPOISON_ON_FREE 0
> +#endif
> +
>  u64 stable_page_flags(struct page *page);
>  
>  static inline int PageUptodate(struct page *page)
> @@ -406,7 +418,8 @@ static inline void __ClearPageTail(struct page *page)
>  	 1 << PG_private | 1 << PG_private_2 | \
>  	 1 << PG_buddy	 | 1 << PG_writeback | 1 << PG_reserved | \
>  	 1 << PG_slab	 | 1 << PG_swapcache | 1 << PG_active | \
> -	 1 << PG_unevictable | __PG_MLOCKED | __PG_HWPOISON)
> +	 1 << PG_unevictable | __PG_MLOCKED | __PG_HWPOISON | \
> +	 __PG_HWPOISON_ON_FREE)
>  
>  /*
>   * Flags checked when a page is prepped for return by the page allocator.
> diff --git a/mm/Kconfig b/mm/Kconfig
> index f0fb912..ede5444 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -269,6 +269,10 @@ config MEMORY_FAILURE
>  	  even when some of its memory has uncorrected errors. This requires
>  	  special hardware support and typically ECC memory.
>  
> +config HWPOISON_ON_FREE
> +	depends on MEMORY_FAILURE && 64BIT
> +	def_bool y	
> +
>  config HWPOISON_INJECT
>  	tristate "HWPoison pages injector"
>  	depends on MEMORY_FAILURE && DEBUG_KERNEL && PROC_FS
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 9c26eec..34901f6 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1268,6 +1268,14 @@ int soft_offline_page(struct page *page, int flags)
>  	if (ret == 0)
>  		goto done;
>  
> +#ifdef CONFIG_HWPOISON_ON_FREE
> +	/*
> +	 * When the page gets freed make sure to poison
> +	 * it immediately.
> +	 */
> +	SetPageHWPoisonOnFree(page);
> +#endif
> +
>  	/*
>  	 * Page cache page we can handle?
>  	 */
> @@ -1279,7 +1287,15 @@ int soft_offline_page(struct page *page, int flags)
>  		shake_page(page, 1);
>  
>  		/*
> -		 * Did it turn free?
> +		 * Did it get poisoned on free?
> +		 */
> +		if (PageHWPoison(page)) {
> +			pr_info("soft_offline: %#lx: Page freed through shaking\n", pfn);
> +			return 0;
> +		}
> +
> +		/* 
> +		 * Try to grab it as a free page again.
>  		 */
>  		ret = get_any_page(page, pfn, 0);
>  		if (ret < 0)
> @@ -1287,12 +1303,23 @@ int soft_offline_page(struct page *page, int flags)
>  		if (ret == 0)
>  			goto done;
>  	}
> +
> +	if (PageHWPoisonOnFree(page)) {
> +		pr_info("soft_offline: %#lx: Delaying poision of unknown page %lx to free\n",
> +			pfn, page->flags);
> +		return -EIO; /* or 0? */

-EIO looks safer because HWPoisonOnFree does not guarantee success.

> +	}
> +
>  	if (!PageLRU(page)) {
>  		pr_debug("soft_offline: %#lx: unknown non LRU page type %lx\n",
>  				pfn, page->flags);
>  		return -EIO;
>  	}
>  
> +	/* 
> +	 * Normal page cache page here.
> +	 */
> +
>  	lock_page(page);
>  	wait_on_page_writeback(page);
>  
> @@ -1389,3 +1416,20 @@ int is_hwpoison_address(unsigned long addr)
>  	return is_hwpoison_entry(entry);
>  }
>  EXPORT_SYMBOL_GPL(is_hwpoison_address);
> +
> +#ifdef CONFIG_HWPOISON_ON_FREE
> +
> +/* 
> + * HWPoison a page after freeing. 
> + * This is the fallback path for most pages we cannot free early.
> + */
> +void hwpoison_page_on_free(struct page *p)
> +{
> +	get_page(p);
> +	SetPageHWPoison(p);
> +	ClearPageHWPoisonOnFree(p);
> +	pr_info("MCE: %#lx: Delayed poisioning of page after free\n",
> +		page_to_pfn(p));
> +	atomic_long_inc(&mce_bad_pages);
> +}
> +#endif
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a8cfa9c..519c24c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -564,6 +564,8 @@ static inline int free_pages_check(struct page *page)
>  		(page->mapping != NULL)  |
>  		(atomic_read(&page->_count) != 0) |
>  		(page->flags & PAGE_FLAGS_CHECK_AT_FREE))) {
> +		if (PageHWPoisonOnFree(page))
> +			hwpoison_page_on_free(page);

hwpoison_page_on_free() seems to be undefined when
CONFIG_HWPOISON_ON_FREE is not defined.

>  		bad_page(page);
>  		return 1;
>  	}

Thanks,
Fengguang

Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Andi Kleen]

  On 10/12/2010 2:26 PM, Wu Fengguang wrote:
>
>>
>> +#if defined(CONFIG_MEMORY_FAILURE)&&  BITS_PER_LONG == 64
> We have the simpler CONFIG_HWPOISON_ON_FREE :)
>

Leftover from when I didn't have that. Fixed.

>> +PAGEFLAG(HWPoisonOnFree, hwpoison_on_free)
>> +TESTSCFLAG(HWPoisonOnFree, hwpoison_on_free)
>> +#define __PG_HWPOISON_ON_FREE (1UL<<  PG_hwpoison_on_free)
>> +#else
>> +PAGEFLAG_FALSE(HWPoisonOnFree)
> Could define SETPAGEFLAG_NOOP(HWPoisonOnFree) too, for eliminating an
> #ifdef in the .c file.

Ok.

>
>>   	}
>> +
>> +	if (PageHWPoisonOnFree(page)) {
>> +		pr_info("soft_offline: %#lx: Delaying poision of unknown page %lx to free\n",
>> +			pfn, page->flags);
>> +		return -EIO; /* or 0? */
> -EIO looks safer because HWPoisonOnFree does not guarantee success.
>
Ok.

>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index a8cfa9c..519c24c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -564,6 +564,8 @@ static inline int free_pages_check(struct page *page)
>>   		(page->mapping != NULL)  |
>>   		(atomic_read(&page->_count) != 0) |
>>   		(page->flags&  PAGE_FLAGS_CHECK_AT_FREE))) {
>> +		if (PageHWPoisonOnFree(page))
>> +			hwpoison_page_on_free(page);
> hwpoison_page_on_free() seems to be undefined when
> CONFIG_HWPOISON_ON_FREE is not defined.

Yes, but I rely on the compiler never generating the call in this case 
because
the test is zero.

It would fail on a unoptimized build, but the kernel doesn't support 
that anyways.

Thanks for the review.

-Andi

Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Wu Fengguang]

> >> +		if (PageHWPoisonOnFree(page))
> >> +			hwpoison_page_on_free(page);
> > hwpoison_page_on_free() seems to be undefined when
> > CONFIG_HWPOISON_ON_FREE is not defined.
> 
> Yes, but I rely on the compiler never generating the call in this case 
> because
> the test is zero.
> 
> It would fail on a unoptimized build, but the kernel doesn't support 
> that anyways.

Fair enough.

Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>

> Thanks for the review.

:)

Thanks,
Fengguang

Re: RFC: Implement hwpoison on free for soft offlining

[Andi Kleen]

Andi Kleen <andi@firstfloor.org> writes:

> Here's a somewhat experimental patch to improve soft offlining
> in hwpoison, but allowing hwpoison on free for not directly
> freeable page types. It should work for nearly all
> left over page types that get eventually freed, so this makes
> soft offlining nearly universal. The only non handleable page
> types are now pages that never get freed.
>
> Drawback: It needs an additional page flag. Cannot set hwpoison
> directly because that would not be "soft" and cause errors.

Ping? Any comments on this patch?

Except for the page flag use I think it's nearly a no brainer. 
A lot of new soft hwpoison capability for very little additional code.

Has anyone a problem using up a 64bit page flag for that?

Thanks,

-Andi 
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: RFC: Implement hwpoison on free for soft offlining

[KOSAKI Motohiro]

> Andi Kleen <andi@firstfloor.org> writes:
> 
> > Here's a somewhat experimental patch to improve soft offlining
> > in hwpoison, but allowing hwpoison on free for not directly
> > freeable page types. It should work for nearly all
> > left over page types that get eventually freed, so this makes
> > soft offlining nearly universal. The only non handleable page
> > types are now pages that never get freed.
> >
> > Drawback: It needs an additional page flag. Cannot set hwpoison
> > directly because that would not be "soft" and cause errors.
> 
> Ping? Any comments on this patch?
> 
> Except for the page flag use I think it's nearly a no brainer. 
> A lot of new soft hwpoison capability for very little additional code.
> 
> Has anyone a problem using up a 64bit page flag for that?

To me, it's no problem if this keep 64bit only. IOW, I only dislike to
add 32bit page flags.

Yeah, memory corruption is very crap and i think your effort has a lot
of worth :)


offtopic, I don't think CONFIG_MEMORY_FAILURE and CONFIG_HWPOISON_ON_FREE
are symmetric nor easy understandable. can you please consider naming change?
(example, CONFIG_HWPOISON/CONFIG_HWPOISON_ON_FREE, 
CONFIG_MEMORY_FAILURE/CONFIG_MEMORY_FAILURE_SOFT_OFFLINE)

Re: RFC: Implement hwpoison on free for soft offlining

[Andi Kleen]

> To me, it's no problem if this keep 64bit only. IOW, I only dislike to
> add 32bit page flags.
> 
> Yeah, memory corruption is very crap and i think your effort has a lot
> of worth :)

Thanks.

> 
> 
> offtopic, I don't think CONFIG_MEMORY_FAILURE and CONFIG_HWPOISON_ON_FREE
> are symmetric nor easy understandable. can you please consider naming change?
> (example, CONFIG_HWPOISON/CONFIG_HWPOISON_ON_FREE, 
> CONFIG_MEMORY_FAILURE/CONFIG_MEMORY_FAILURE_SOFT_OFFLINE)

memory-failure was the old name before hwpoison as a term was invented
by Andrew.

In theory it would make sense to rename everything to "hwpoison" now.
But I decided so far the disadvantages from breaking user configurations
and the impact from renaming files far outweight the small benefits
in clarity.

So right now I prefer to keep the status quo, but name everything
new hwpoison.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: RFC: Implement hwpoison on free for soft offlining

[KOSAKI Motohiro]

> > To me, it's no problem if this keep 64bit only. IOW, I only dislike to
> > add 32bit page flags.
> > 
> > Yeah, memory corruption is very crap and i think your effort has a lot
> > of worth :)
> 
> Thanks.
> 
> > 
> > 
> > offtopic, I don't think CONFIG_MEMORY_FAILURE and CONFIG_HWPOISON_ON_FREE
> > are symmetric nor easy understandable. can you please consider naming change?
> > (example, CONFIG_HWPOISON/CONFIG_HWPOISON_ON_FREE, 
> > CONFIG_MEMORY_FAILURE/CONFIG_MEMORY_FAILURE_SOFT_OFFLINE)
> 
> memory-failure was the old name before hwpoison as a term was invented
> by Andrew.
> 
> In theory it would make sense to rename everything to "hwpoison" now.
> But I decided so far the disadvantages from breaking user configurations
> and the impact from renaming files far outweight the small benefits
> in clarity.
> 
> So right now I prefer to keep the status quo, but name everything
> new hwpoison.

ok. I've got it. thanks :)

Re: RFC: Implement hwpoison on free for soft offlining

[Andi Kleen]

Andi Kleen <andi@firstfloor.org> writes:

> Here's a somewhat experimental patch to improve soft offlining
> in hwpoison, but allowing hwpoison on free for not directly
> freeable page types. It should work for nearly all
> left over page types that get eventually freed, so this makes
> soft offlining nearly universal. The only non handleable page
> types are now pages that never get freed.
>
> Drawback: It needs an additional page flag. Cannot set hwpoison
> directly because that would not be "soft" and cause errors.
>
> Since the flags are scarce on 32bit I only enabled it on 64bit.
>
> Comments?

I got a couple of positive comments and reviews and no negative comments.

So I assume noone has a problem with using up a 64bit page flag
for this. I plan to push this into linux-next after some delay
and then prepare it for merge later.

If there are any objections please speak up now.

Thanks,
-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Penttilä Mika]

> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -564,6 +564,8 @@ static inline int free_pages_check(struct page *page)
>  		(page->mapping != NULL)  |
>  		(atomic_read(&page->_count) != 0) |
>  		(page->flags & PAGE_FLAGS_CHECK_AT_FREE))) {
> +		if (PageHWPoisonOnFree(page))
> +			hwpoison_page_on_free(page);

hwpoison_page_on_free() seems to be undefined when
CONFIG_HWPOISON_ON_FREE is not defined.

>  		bad_page(page);
>  		return 1;
>  	}

And of course PAGE_FLAGS_CHECK_AT_FREE should include pahehwpoisononfree this to be effective

--Mika

Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Andi Kleen]

Penttilä Mika <mika.penttila@ixonos.com> writes:

Please always cc people when answering list emails.
Missed this earlier.

>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -564,6 +564,8 @@ static inline int free_pages_check(struct page *page)
>>  		(page->mapping != NULL)  |
>>  		(atomic_read(&page->_count) != 0) |
>>  		(page->flags & PAGE_FLAGS_CHECK_AT_FREE))) {
>> +		if (PageHWPoisonOnFree(page))
>> +			hwpoison_page_on_free(page);
>
> hwpoison_page_on_free() seems to be undefined when
> CONFIG_HWPOISON_ON_FREE is not defined.

The optimizer takes care of that.

>
>>  		bad_page(page);
>>  		return 1;
>>  	}
>
> And of course PAGE_FLAGS_CHECK_AT_FREE should include
> pahehwpoisononfree this to be effective

It is of course (__PG_HWPOISON_ON_FREE)

Thanks for the review.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

RE: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Penttilä Mika]

> 
> >> --- a/mm/page_alloc.c
> >> +++ b/mm/page_alloc.c
> >> @@ -564,6 +564,8 @@ static inline int free_pages_check(struct page
> *page)
> >>  		(page->mapping != NULL)  |
> >>  		(atomic_read(&page->_count) != 0) |
> >>  		(page->flags & PAGE_FLAGS_CHECK_AT_FREE))) {
> >> +		if (PageHWPoisonOnFree(page))
> >> +			hwpoison_page_on_free(page);
> >
> > hwpoison_page_on_free() seems to be undefined when
> > CONFIG_HWPOISON_ON_FREE is not defined.
> 
> The optimizer takes care of that.
> 
> >
> >>  		bad_page(page);
> >>  		return 1;
> >>  	}
> >
> > And of course PAGE_FLAGS_CHECK_AT_FREE should include
> > pahehwpoisononfree this to be effective
> 
> It is of course (__PG_HWPOISON_ON_FREE)
> 
Yes but it's not included in PAGE_FLAGS_CHECK_AT_FREE, if  I'm not missing something

--Mika

Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Andi Kleen]

On Tue, Oct 12, 2010 at 02:23:15PM +0000, Penttilä Mika wrote:
> > >>  		bad_page(page);
> > >>  		return 1;
> > >>  	}
> > >
> > > And of course PAGE_FLAGS_CHECK_AT_FREE should include
> > > pahehwpoisononfree this to be effective
> > 
> > It is of course (__PG_HWPOISON_ON_FREE)
> > 
> Yes but it's not included in PAGE_FLAGS_CHECK_AT_FREE, if  I'm not missing something

#ifdef CONFIG_HWPOISON_ON_FREE
PAGEFLAG(HWPoisonOnFree, hwpoison_on_free)
TESTSCFLAG(HWPoisonOnFree, hwpoison_on_free)
#define __PG_HWPOISON_ON_FREE (1UL << PG_hwpoison_on_free)
#else
PAGEFLAG_FALSE(HWPoisonOnFree)
SETPAGEFLAG_NOOP(HWPoisonOnFree)
#define __PG_HWPOISON_ON_FREE 0
#endif

...

#define PAGE_FLAGS_CHECK_AT_FREE \
        (1 << PG_lru     | 1 << PG_locked    | \
         1 << PG_private | 1 << PG_private_2 | \
         1 << PG_buddy   | 1 << PG_writeback | 1 << PG_reserved | \
         1 << PG_slab    | 1 << PG_swapcache | 1 << PG_active | \
         1 << PG_unevictable | __PG_MLOCKED | __PG_HWPOISON | \
         __PG_HWPOISON_ON_FREE)


-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

RE: [PATCH] HWPOISON: Implement hwpoison-on-free for soft offlining

[Penttilä Mika]

> From: Andi Kleen [mailto:andi@firstfloor.org]
> Sent: 12. lokakuuta 2010 17:30
> To: Penttilä Mika
> Cc: Andi Kleen; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] HWPOISON: Implement hwpoison-on-free for soft
> offlining
> 
> On Tue, Oct 12, 2010 at 02:23:15PM +0000, Penttilä Mika wrote:
> > > >>  		bad_page(page);
> > > >>  		return 1;
> > > >>  	}
> > > >
> > > > And of course PAGE_FLAGS_CHECK_AT_FREE should include
> > > > pahehwpoisononfree this to be effective
> > >
> > > It is of course (__PG_HWPOISON_ON_FREE)
> > >
> > Yes but it's not included in PAGE_FLAGS_CHECK_AT_FREE, if  I'm not
> missing something
> 
> #ifdef CONFIG_HWPOISON_ON_FREE
> PAGEFLAG(HWPoisonOnFree, hwpoison_on_free)
> TESTSCFLAG(HWPoisonOnFree, hwpoison_on_free)
> #define __PG_HWPOISON_ON_FREE (1UL << PG_hwpoison_on_free)
> #else
> PAGEFLAG_FALSE(HWPoisonOnFree)
> SETPAGEFLAG_NOOP(HWPoisonOnFree)
> #define __PG_HWPOISON_ON_FREE 0
> #endif
> 
> ...
> 
> #define PAGE_FLAGS_CHECK_AT_FREE \
>         (1 << PG_lru     | 1 << PG_locked    | \
>          1 << PG_private | 1 << PG_private_2 | \
>          1 << PG_buddy   | 1 << PG_writeback | 1 << PG_reserved | \
>          1 << PG_slab    | 1 << PG_swapcache | 1 << PG_active | \
>          1 << PG_unevictable | __PG_MLOCKED | __PG_HWPOISON | \
>          __PG_HWPOISON_ON_FREE)
> 
> 
> -Andi
> --
> ak@linux.intel.com -- Speaking for myself only.



Sure, sorry for that, read the patch badly..

--Mika