Re: [PATCH v28 3/4] mm/page_poison: add a function to expose page poison val to kernel modules

From: Michael S. Tsirkin
Date: Thu Feb 08 2018 - 14:29:49 EST


On Thu, Feb 08, 2018 at 05:50:19PM +0800, Wei Wang wrote:
> Move the PAGE_POISON value to page_poison.c and add a function to enable
> callers from a kernel module to get the poison value if the page poisoning
> feature is in use. This also avoids callers directly checking PAGE_POISON
> regardless of whether the feature is enabled.
>
> Signed-off-by: Wei Wang <wei.w.wang@xxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
> Cc: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
> include/linux/mm.h | 2 ++
> include/linux/poison.h | 7 -------
> mm/page_poison.c | 24 ++++++++++++++++++++++++
> 3 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1c77d88..d95e5d3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2469,11 +2469,13 @@ extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
> extern bool page_poisoning_enabled(void);
> extern void kernel_poison_pages(struct page *page, int numpages, int enable);
> extern bool page_is_poisoned(struct page *page);
> +extern bool page_poison_val_get(u8 *val);
> #else
> static inline bool page_poisoning_enabled(void) { return false; }
> static inline void kernel_poison_pages(struct page *page, int numpages,
> int enable) { }
> static inline bool page_is_poisoned(struct page *page) { return false; }
> +static inline bool page_poison_val_get(u8 *val) { return false; };
> #endif
>
> #ifdef CONFIG_DEBUG_PAGEALLOC

Thinking more about it, page_poisoning_enabled already exists.
I would just add "u8 page_poison_val_get(void)" and don't add
a stub since compiler will drop the call with poisoning off.

> diff --git a/include/linux/poison.h b/include/linux/poison.h
> index 15927eb..348bf67 100644
> --- a/include/linux/poison.h
> +++ b/include/linux/poison.h
> @@ -30,13 +30,6 @@
> */
> #define TIMER_ENTRY_STATIC ((void *) 0x300 + POISON_POINTER_DELTA)
>
> -/********** mm/debug-pagealloc.c **********/
> -#ifdef CONFIG_PAGE_POISONING_ZERO
> -#define PAGE_POISON 0x00
> -#else
> -#define PAGE_POISON 0xaa
> -#endif
> -
> /********** mm/page_alloc.c ************/
>

I'd split this chunk out, this patchset is at v28 as it is.

> #define TAIL_MAPPING ((void *) 0x400 + POISON_POINTER_DELTA)
> diff --git a/mm/page_poison.c b/mm/page_poison.c
> index e83fd44..9a07973 100644
> --- a/mm/page_poison.c
> +++ b/mm/page_poison.c
> @@ -7,6 +7,13 @@
> #include <linux/poison.h>
> #include <linux/ratelimit.h>
>
> +/********** mm/debug-pagealloc.c **********/

This file no longer exists.

> +#ifdef CONFIG_PAGE_POISONING_ZERO
> +#define PAGE_POISON 0x00
> +#else
> +#define PAGE_POISON 0xaa
> +#endif
> +
> static bool want_page_poisoning __read_mostly;
>
> static int early_page_poison_param(char *buf)
> @@ -30,6 +37,23 @@ bool page_poisoning_enabled(void)
> debug_pagealloc_enabled()));
> }
>
> +/**
> + * page_poison_val_get - get the page poison value if page poisoning is enabled
> + * @val: the caller's memory to get the page poison value
> + *
> + * Return true with @val stores the poison value if page poisoning is enabled.
> + * Otherwise, return false with @val unchanged.
> + */
> +bool page_poison_val_get(u8 *val)
> +{
> + if (!page_poisoning_enabled())
> + return false;
> +
> + *val = PAGE_POISON;
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(page_poison_val_get);
> +
> static void poison_page(struct page *page)
> {
> void *addr = kmap_atomic(page);
> --
> 2.7.4