WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and

To: Shriram Rajagopalan <rshriram@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH v3 4/5] PM: Add visible HIBERNATION_INTERFACE and hide HIBERNATION
From: "Rafael J. Wysocki" <rjw@xxxxxxx>
Date: Tue, 15 Mar 2011 22:38:29 +0100
Cc: linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 15 Mar 2011 14:40:00 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1299906471-31011-1-git-send-email-rshriram@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1299906471-31011-1-git-send-email-rshriram@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.13.6 (Linux/2.6.38+; KDE/4.6.0; x86_64; ; )
Hi,

Sorry, I didn't have the time to review the patch in detail before.

On Saturday, March 12, 2011, Shriram Rajagopalan wrote:
> HIBERNATION covers the main hibernation control code and freeze-thaw
> pm events, that xen's save/restore also uses. Explicitly enabling
> an independant hibernation functionality to enable xen's save/restore
> is a bit ugly. Define a new user visible symbol HIBERNATION_INTERFACE
> that "selects" HIBERNATION and covers the main hibernation control code
> instead of HIBERNATION. This way, we can also make XEN_SAVE_RESTORE
> "select" HIBERNATION, enabling only the freeze-thaw code.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
> ---
>  kernel/power/Kconfig     |    9 +++++++--
>  kernel/power/hibernate.c |    4 ++++
>  kernel/power/main.c      |    2 +-
>  kernel/power/user.c      |    2 ++
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 4603f08..493c678 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -19,10 +19,15 @@ config SUSPEND_FREEZER
>         Turning OFF this setting is NOT recommended! If in doubt, say Y.
>  
>  config HIBERNATION
> -     bool "Hibernation (aka 'suspend to disk')"
> -     depends on SWAP && ARCH_HIBERNATION_POSSIBLE
> +     def_bool n

You don't need that.  Simply use "bool"

> +     depends on ARCH_HIBERNATION_POSSIBLE

That need not depend on it, HIBERNATION_INTERFACE should instead.

>       select LZO_COMPRESS
>       select LZO_DECOMPRESS

These two selects may be moved to HIBERNATION_INTERFACE too.

> +
> +config HIBERNATION_INTERFACE
> +     bool "Hibernation (aka 'suspend to disk')"
> +     depends on SWAP
> +     select HIBERNATION
>       ---help---
>         Enable the suspend to disk (STD) functionality, which is usually
>         called "hibernation" in user interfaces.  STD checkpoints the
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 1832bd2..13bcf69 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -592,6 +592,7 @@ static int prepare_processes(void)
>   *   hibernate - The granpappy of the built-in hibernation management
>   */
>  
> +#ifdef CONFIG_HIBERNATION_INTERFACE

Please don't do that.  Please make the whole hibernate.c depend on
CONFIG_HIBERNATION_INTERFACE.

>  int hibernate(void)
>  {
>       int error;
> @@ -667,6 +668,8 @@ int hibernate(void)
>       return error;
>  }
>  
> +#else /* !CONFIG_HIBERNATION_INTERFACE */
> +int hibernate(void) { return -ENOSYS; }

And move that to a header.

>  /**
>   *   software_resume - Resume from a saved image.
> @@ -1029,3 +1032,4 @@ __setup("noresume", noresume_setup);
>  __setup("resume_offset=", resume_offset_setup);
>  __setup("resume=", resume_setup);
>  __setup("hibernate=", hibernate_setup);
> +#endif /* !CONFIG_HIBERNATION_INTERFACE */
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 8eaba5f..686a130 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -156,7 +156,7 @@ static ssize_t state_show(struct kobject *kobj, struct 
> kobj_attribute *attr,
>                       s += sprintf(s,"%s ", pm_states[i]);
>       }
>  #endif
> -#ifdef CONFIG_HIBERNATION
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>       s += sprintf(s, "%s\n", "disk");
>  #else
>       if (s != buf)
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index c36c3b9..5f36ee7 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -458,6 +458,7 @@ static long snapshot_ioctl(struct file *filp, unsigned 
> int cmd,
>       return error;
>  }
>  
> +#ifdef CONFIG_HIBERNATION_INTERFACE
>  static const struct file_operations snapshot_fops = {
>       .open = snapshot_open,
>       .release = snapshot_release,
> @@ -479,3 +480,4 @@ static int __init snapshot_device_init(void)
>  };
>  
>  device_initcall(snapshot_device_init);
> +#endif /* CONFIG_HIBERNATION_INTERFACE */
> 

Again, please make the entire user.c depend on CONFIG_HIBERNATION_INTERFACE
instead of adding #ifdefs to the file.

Thanks,
Rafael

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel