Re: [patch 1/1] [PATCH] include storage keys in hibernation image.

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Tuesday, August 16, 2011, Martin Schwidefsky wrote:
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> 
> For s390 there is one additional byte associated with each page,
> the storage key. This byte contains the referenced and changed
> bits and needs to be included into the hibernation image.
> If the storage keys are not restored to their previous state all
> original pages would appear to be dirty. This can cause
> inconsistencies e.g. with read-only filesystems.
> 
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-pm@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-s390@vger.kernel.org
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

OK, I don't have any complaints.  Do you want me to take this
patch or do you want to push it through the s390 tree?

Rafael


> ---
> 
>  arch/s390/Kconfig               |    1 
>  arch/s390/kernel/suspend.c      |  118 ++++++++++++++++++++++++++++++++++++++++
>  arch/s390/kernel/swsusp_asm64.S |    3 +
>  include/linux/suspend.h         |   34 +++++++++++
>  kernel/power/Kconfig            |    3 +
>  kernel/power/snapshot.c         |   18 ++++++
>  6 files changed, 177 insertions(+)
> 
> Index: hibernate-2.6/arch/s390/Kconfig
> ===================================================================
> --- hibernate-2.6.orig/arch/s390/Kconfig	2011-08-09 17:40:09.220010373 +0200
> +++ hibernate-2.6/arch/s390/Kconfig	2011-08-16 16:06:09.845540931 +0200
> @@ -91,6 +91,7 @@
>  	select HAVE_ARCH_MUTEX_CPU_RELAX
>  	select HAVE_ARCH_JUMP_LABEL if !MARCH_G5
>  	select HAVE_RCU_TABLE_FREE if SMP
> +	select ARCH_SAVE_PAGE_KEYS if HIBERNATION
>  	select ARCH_INLINE_SPIN_TRYLOCK
>  	select ARCH_INLINE_SPIN_TRYLOCK_BH
>  	select ARCH_INLINE_SPIN_LOCK
> Index: hibernate-2.6/arch/s390/kernel/suspend.c
> ===================================================================
> --- hibernate-2.6.orig/arch/s390/kernel/suspend.c	2009-09-24 09:06:44.000000000 +0200
> +++ hibernate-2.6/arch/s390/kernel/suspend.c	2011-08-16 16:06:09.845540931 +0200
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <linux/pfn.h>
> +#include <linux/mm.h>
>  #include <asm/system.h>
>  
>  /*
> @@ -14,6 +15,123 @@
>   */
>  extern const void __nosave_begin, __nosave_end;
>  
> +/*
> + * The restore of the saved pages in an hibernation image will set
> + * the change and referenced bits in the storage key for each page.
> + * Overindication of the referenced bits after an hibernation cycle
> + * does not cause any harm but the overindication of the change bits
> + * would cause trouble.
> + * Use the ARCH_SAVE_PAGE_KEYS hooks to save the storage key of each
> + * page to the most significant byte of the associated page frame
> + * number in the hibernation image.
> + */
> +
> +/*
> + * Key storage is allocated as a linked list of pages.
> + * The size of the keys array is (PAGE_SIZE - sizeof(long))
> + */
> +struct page_key_data {
> +	struct page_key_data *next;
> +	unsigned char data[];
> +};
> +
> +#define PAGE_KEY_DATA_SIZE	(PAGE_SIZE - sizeof(struct page_key_data *))
> +
> +static struct page_key_data *page_key_data;
> +static struct page_key_data *page_key_rp, *page_key_wp;
> +static unsigned long page_key_rx, page_key_wx;
> +
> +/*
> + * For each page in the hibernation image one additional byte is
> + * stored in the most significant byte of the page frame number.
> + * On suspend no additional memory is required but on resume the
> + * keys need to be memorized until the page data has been restored.
> + * Only then can the storage keys be set to their old state.
> + */
> +unsigned long page_key_additional_pages(unsigned long pages)
> +{
> +	return DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
> +}
> +
> +/*
> + * Free page_key_data list of arrays.
> + */
> +void page_key_free(void)
> +{
> +	struct page_key_data *pkd;
> +
> +	while (page_key_data) {
> +		pkd = page_key_data;
> +		page_key_data = pkd->next;
> +		free_page((unsigned long) pkd);
> +	}
> +}
> +
> +/*
> + * Allocate page_key_data list of arrays with enough room to store
> + * one byte for each page in the hibernation image.
> + */
> +int page_key_alloc(unsigned long pages)
> +{
> +	struct page_key_data *pk;
> +	unsigned long size;
> +
> +	size = DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
> +	while (size--) {
> +		pk = (struct page_key_data *) get_zeroed_page(GFP_KERNEL);
> +		if (!pk) {
> +			page_key_free();
> +			return -ENOMEM;
> +		}
> +		pk->next = page_key_data;
> +		page_key_data = pk;
> +	}
> +	page_key_rp = page_key_wp = page_key_data;
> +	page_key_rx = page_key_wx = 0;
> +	return 0;
> +}
> +
> +/*
> + * Save the storage key into the upper 8 bits of the page frame number.
> + */
> +void page_key_read(unsigned long *pfn)
> +{
> +	unsigned long addr;
> +
> +	addr = (unsigned long) page_address(pfn_to_page(*pfn));
> +	*(unsigned char *) pfn = (unsigned char) page_get_storage_key(addr);
> +}
> +
> +/*
> + * Extract the storage key from the upper 8 bits of the page frame number
> + * and store it in the page_key_data list of arrays.
> + */
> +void page_key_memorize(unsigned long *pfn)
> +{
> +	page_key_wp->data[page_key_wx] = *(unsigned char *) pfn;
> +	*(unsigned char *) pfn = 0;
> +	if (++page_key_wx < PAGE_KEY_DATA_SIZE)
> +		return;
> +	page_key_wp = page_key_wp->next;
> +	page_key_wx = 0;
> +}
> +
> +/*
> + * Get the next key from the page_key_data list of arrays and set the
> + * storage key of the page referred by @address. If @address refers to
> + * a "safe" page the swsusp_arch_resume code will transfer the storage
> + * key from the buffer page to the original page.
> + */
> +void page_key_write(void *address)
> +{
> +	page_set_storage_key((unsigned long) address,
> +			     page_key_rp->data[page_key_rx], 0);
> +	if (++page_key_rx >= PAGE_KEY_DATA_SIZE)
> +		return;
> +	page_key_rp = page_key_rp->next;
> +	page_key_rx = 0;
> +}
> +
>  int pfn_is_nosave(unsigned long pfn)
>  {
>  	unsigned long nosave_begin_pfn = PFN_DOWN(__pa(&__nosave_begin));
> Index: hibernate-2.6/arch/s390/kernel/swsusp_asm64.S
> ===================================================================
> --- hibernate-2.6.orig/arch/s390/kernel/swsusp_asm64.S	2011-08-09 17:40:09.228010544 +0200
> +++ hibernate-2.6/arch/s390/kernel/swsusp_asm64.S	2011-08-16 16:06:09.845540931 +0200
> @@ -136,11 +136,14 @@
>  0:
>  	lg	%r2,8(%r1)
>  	lg	%r4,0(%r1)
> +	iske	%r0,%r4
>  	lghi	%r3,PAGE_SIZE
>  	lghi	%r5,PAGE_SIZE
>  1:
>  	mvcle	%r2,%r4,0
>  	jo	1b
> +	lg	%r2,8(%r1)
> +	sske	%r0,%r2
>  	lg	%r1,16(%r1)
>  	ltgr	%r1,%r1
>  	jnz	0b
> Index: hibernate-2.6/include/linux/suspend.h
> ===================================================================
> --- hibernate-2.6.orig/include/linux/suspend.h	2011-08-09 17:40:10.164030551 +0200
> +++ hibernate-2.6/include/linux/suspend.h	2011-08-16 16:06:09.845540931 +0200
> @@ -334,4 +334,38 @@
>  }
>  #endif
>  
> +#ifdef CONFIG_ARCH_SAVE_PAGE_KEYS
> +/*
> + * The ARCH_SAVE_PAGE_KEYS functions can be used by an architecture
> + * to save/restore additional information to/from the array of page
> + * frame numbers in the hibernation image. For s390 this is used to
> + * save and restore the storage key for each page that is included
> + * in the hibernation image.
> + */
> +unsigned long page_key_additional_pages(unsigned long pages);
> +int page_key_alloc(unsigned long pages);
> +void page_key_free(void);
> +void page_key_read(unsigned long *pfn);
> +void page_key_memorize(unsigned long *pfn);
> +void page_key_write(void *address);
> +
> +#else /* !CONFIG_ARCH_SAVE_PAGE_KEYS */
> +
> +static inline unsigned long page_key_additional_pages(unsigned long pages)
> +{
> +	return 0;
> +}
> +
> +static inline int  page_key_alloc(unsigned long pages)
> +{
> +	return 0;
> +}
> +
> +static inline void page_key_free(void) {}
> +static inline void page_key_read(unsigned long *pfn) {}
> +static inline void page_key_memorize(unsigned long *pfn) {}
> +static inline void page_key_write(void *address) {}
> +
> +#endif /* !CONFIG_ARCH_SAVE_PAGE_KEYS */
> +
>  #endif /* _LINUX_SUSPEND_H */
> Index: hibernate-2.6/kernel/power/Kconfig
> ===================================================================
> --- hibernate-2.6.orig/kernel/power/Kconfig	2011-08-09 17:40:10.192031153 +0200
> +++ hibernate-2.6/kernel/power/Kconfig	2011-08-16 16:06:09.845540931 +0200
> @@ -65,6 +65,9 @@
>  
>  	  For more information take a look at <file:Documentation/power/swsusp.txt>.
>  
> +config ARCH_SAVE_PAGE_KEYS
> +	bool
> +
>  config PM_STD_PARTITION
>  	string "Default resume partition"
>  	depends on HIBERNATION
> Index: hibernate-2.6/kernel/power/snapshot.c
> ===================================================================
> --- hibernate-2.6.orig/kernel/power/snapshot.c	2011-07-08 11:25:06.293203483 +0200
> +++ hibernate-2.6/kernel/power/snapshot.c	2011-08-16 16:06:09.845540931 +0200
> @@ -1339,6 +1339,9 @@
>  	count += highmem;
>  	count -= totalreserve_pages;
>  
> +	/* Add number of pages required for page keys (s390 only). */
> +	size += page_key_additional_pages(saveable);
> +
>  	/* Compute the maximum number of saveable pages to leave in memory. */
>  	max_size = (count - (size + PAGES_FOR_IO)) / 2
>  			- 2 * DIV_ROUND_UP(reserved_size, PAGE_SIZE);
> @@ -1662,6 +1665,8 @@
>  		buf[j] = memory_bm_next_pfn(bm);
>  		if (unlikely(buf[j] == BM_END_OF_MAP))
>  			break;
> +		/* Save page key for data page (s390 only). */
> +		page_key_read(buf + j);
>  	}
>  }
>  
> @@ -1821,6 +1826,9 @@
>  		if (unlikely(buf[j] == BM_END_OF_MAP))
>  			break;
>  
> +		/* Extract and buffer page key for data page (s390 only). */
> +		page_key_memorize(buf + j);
> +
>  		if (memory_bm_pfn_present(bm, buf[j]))
>  			memory_bm_set_bit(bm, buf[j]);
>  		else
> @@ -2223,6 +2231,11 @@
>  		if (error)
>  			return error;
>  
> +		/* Allocate buffer for page keys. */
> +		error = page_key_alloc(nr_copy_pages);
> +		if (error)
> +			return error;
> +
>  	} else if (handle->cur <= nr_meta_pages + 1) {
>  		error = unpack_orig_pfns(buffer, &copy_bm);
>  		if (error)
> @@ -2243,6 +2256,8 @@
>  		}
>  	} else {
>  		copy_last_highmem_page();
> +		/* Restore page key for data page (s390 only). */
> +		page_key_write(handle->buffer);
>  		handle->buffer = get_buffer(&orig_bm, &ca);
>  		if (IS_ERR(handle->buffer))
>  			return PTR_ERR(handle->buffer);
> @@ -2264,6 +2279,9 @@
>  void snapshot_write_finalize(struct snapshot_handle *handle)
>  {
>  	copy_last_highmem_page();
> +	/* Restore page key for data page (s390 only). */
> +	page_key_write(handle->buffer);
> +	page_key_free();
>  	/* Free only if we have loaded the image entirely */
>  	if (handle->cur > 1 && handle->cur > nr_meta_pages + nr_copy_pages) {
>  		memory_bm_free(&orig_bm, PG_UNSAFE_CLEAR);
> 
> 
>